[Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Mel Gorman]

Cyrill,

Gimp is broken due to a kernel bug included in 3.12. It cannot open
large files without failing memory allocations due to exceeding
vm.max_map_count. The relevant bugzilla entries are

https://bugzilla.kernel.org/show_bug.cgi?id=67651
https://bugzilla.gnome.org/show_bug.cgi?id=719619#c0

They include details on how to reproduce the issue. In my case, a
failure shows messages like this

	(gimp:11768): GLib-ERROR **: gmem.c:110: failed to allocate 4096 bytes

	(file-tiff-load:12038): LibGimpBase-WARNING **: file-tiff-load: gimp_wire_read(): error
	xinit: connection to X server lost

	waiting for X server to shut down
	/usr/lib64/gimp/2.0/plug-ins/file-tiff-load terminated: Hangup
	/usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
	/usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup

X-related junk is there was because I was using a headless server and
xinit directly to launch gimp to reproduce the bug.

Automated bisection using mmtests (https://github.com/gormanm/mmtests)
and the configuration file configs/config-global-dhp__gimp-simple (needs
local web server with a copy of the image file) identified the following
commit. Test case was simple -- try and open the large file described in
the bug. I did not investigate the patch itself as I'm just reporting
the results of the bisection. If I had to guess, I'd say that VMA
merging has been affected.

d9104d1ca9662498339c0de975b4666c30485f4e is the first bad commit
commit d9104d1ca9662498339c0de975b4666c30485f4e
Author: Cyrill Gorcunov <gorcunov@gmail.com>
Date:   Wed Sep 11 14:22:24 2013 -0700

    mm: track vma changes with VM_SOFTDIRTY bit
    
    Pavel reported that in case if vma area get unmapped and then mapped (or
    expanded) in-place, the soft dirty tracker won't be able to recognize this
    situation since it works on pte level and ptes are get zapped on unmap,
    loosing soft dirty bit of course.
    
    So to resolve this situation we need to track actions on vma level, there
    VM_SOFTDIRTY flag comes in.  When new vma area created (or old expanded)
    we set this bit, and keep it here until application calls for clearing
    soft dirty bit.
    
    Thus when user space application track memory changes now it can detect if
    vma area is renewed.
    
    Reported-by: Pavel Emelyanov <xemul@parallels.com>
    Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
    Cc: Andy Lutomirski <luto@amacapital.net>
    Cc: Matt Mackall <mpm@selenic.com>
    Cc: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
    Cc: Marcelo Tosatti <mtosatti@redhat.com>
    Cc: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
    Cc: Stephen Rothwell <sfr@canb.auug.org.au>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
    Cc: Rob Landley <rob@landley.net>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Cyrill Gorcunov]

On Wed, Jan 22, 2014 at 07:08:16PM +0000, Mel Gorman wrote:
> Cyrill,
> 
> Gimp is broken due to a kernel bug included in 3.12. It cannot open
> large files without failing memory allocations due to exceeding
> vm.max_map_count. The relevant bugzilla entries are
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=67651
> https://bugzilla.gnome.org/show_bug.cgi?id=719619#c0
> 
> They include details on how to reproduce the issue. In my case, a
> failure shows messages like this
> 
> 	(gimp:11768): GLib-ERROR **: gmem.c:110: failed to allocate 4096 bytes
> 
> 	(file-tiff-load:12038): LibGimpBase-WARNING **: file-tiff-load: gimp_wire_read(): error
> 	xinit: connection to X server lost
> 
> 	waiting for X server to shut down
> 	/usr/lib64/gimp/2.0/plug-ins/file-tiff-load terminated: Hangup
> 	/usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
> 	/usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
> 
> X-related junk is there was because I was using a headless server and
> xinit directly to launch gimp to reproduce the bug.
> 
> Automated bisection using mmtests (https://github.com/gormanm/mmtests)
> and the configuration file configs/config-global-dhp__gimp-simple (needs
> local web server with a copy of the image file) identified the following
> commit. Test case was simple -- try and open the large file described in
> the bug. I did not investigate the patch itself as I'm just reporting
> the results of the bisection. If I had to guess, I'd say that VMA
> merging has been affected.

Thanks a lot for report, Mel! I'm investigating...

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Cyrill Gorcunov]

On Wed, Jan 22, 2014 at 11:19:28PM +0400, Cyrill Gorcunov wrote:
> > commit. Test case was simple -- try and open the large file described in
> > the bug. I did not investigate the patch itself as I'm just reporting
> > the results of the bisection. If I had to guess, I'd say that VMA
> > merging has been affected.
> 
> Thanks a lot for report, Mel! I'm investigating...

Mel, here is a quick fix for bring merging back (just in case if you
have a minute to test it and confirm the merging were affected). It
seems I've lost setting up vma-softdirty bit somewhere and procedure
which tests vma flags mathcing fails, will continue investigating/testing
tomorrow.
---
 mm/mmap.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

Index: linux-2.6.git/mm/mmap.c
===================================================================
--- linux-2.6.git.orig/mm/mmap.c
+++ linux-2.6.git/mm/mmap.c
@@ -893,8 +893,18 @@ again:			remove_next = 1 + (end > next->
 static inline int is_mergeable_vma(struct vm_area_struct *vma,
 			struct file *file, unsigned long vm_flags)
 {
+	/*
+	 * VM_SOFTDIRTY should not prevent from VMA merging, if we
+	 * match the flags but dirty bit -- just mark merged one as
+	 * a dirty then.
+	 */
+#ifdef CONFIG_MEM_SOFT_DIRTY
+	if ((vma->vm_flags ^ vm_flags) & ~VM_SOFTDIRTY)
+		return 0;
+#else
 	if (vma->vm_flags ^ vm_flags)
 		return 0;
+#endif
 	if (vma->vm_file != file)
 		return 0;
 	if (vma->vm_ops && vma->vm_ops->close)
@@ -1082,7 +1092,11 @@ static int anon_vma_compatible(struct vm
 	return a->vm_end == b->vm_start &&
 		mpol_equal(vma_policy(a), vma_policy(b)) &&
 		a->vm_file == b->vm_file &&
+#ifdef CONFIG_MEM_SOFT_DIRTY
+		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
+#else
 		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC)) &&
+#endif
 		b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
 }
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Mel Gorman]

On Thu, Jan 23, 2014 at 02:33:25AM +0400, Cyrill Gorcunov wrote:
> On Wed, Jan 22, 2014 at 11:19:28PM +0400, Cyrill Gorcunov wrote:
> > > commit. Test case was simple -- try and open the large file described in
> > > the bug. I did not investigate the patch itself as I'm just reporting
> > > the results of the bisection. If I had to guess, I'd say that VMA
> > > merging has been affected.
> > 
> > Thanks a lot for report, Mel! I'm investigating...
> 
> Mel, here is a quick fix for bring merging back (just in case if you
> have a minute to test it and confirm the merging were affected). It
> seems I've lost setting up vma-softdirty bit somewhere and procedure
> which tests vma flags mathcing fails, will continue investigating/testing
> tomorrow.

The test case passes with this patch applied to 3.13 so that appears
to confirm that this is related to VM_SOFTDIRTY preventing merges.
Unfortunately I did not have slabinfo tracking enabled to double check
the number of vm_area_structs in teh system.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Cyrill Gorcunov]

On Thu, Jan 23, 2014 at 09:55:41AM +0000, Mel Gorman wrote:
> On Thu, Jan 23, 2014 at 02:33:25AM +0400, Cyrill Gorcunov wrote:
> > On Wed, Jan 22, 2014 at 11:19:28PM +0400, Cyrill Gorcunov wrote:
> > > > commit. Test case was simple -- try and open the large file described in
> > > > the bug. I did not investigate the patch itself as I'm just reporting
> > > > the results of the bisection. If I had to guess, I'd say that VMA
> > > > merging has been affected.
> > > 
> > > Thanks a lot for report, Mel! I'm investigating...
> > 
> > Mel, here is a quick fix for bring merging back (just in case if you
> > have a minute to test it and confirm the merging were affected). It
> > seems I've lost setting up vma-softdirty bit somewhere and procedure
> > which tests vma flags mathcing fails, will continue investigating/testing
> > tomorrow.
> 
> The test case passes with this patch applied to 3.13 so that appears
> to confirm that this is related to VM_SOFTDIRTY preventing merges.
> Unfortunately I did not have slabinfo tracking enabled to double check
> the number of vm_area_structs in teh system.

Thanks a lot, Mel! I'm testing the patch as well (manually though :).
I'll send the final fix today.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Cyrill Gorcunov]

On Thu, Jan 23, 2014 at 02:36:06PM +0400, Cyrill Gorcunov wrote:
> > The test case passes with this patch applied to 3.13 so that appears
> > to confirm that this is related to VM_SOFTDIRTY preventing merges.
> > Unfortunately I did not have slabinfo tracking enabled to double check
> > the number of vm_area_structs in teh system.
> 
> Thanks a lot, Mel! I'm testing the patch as well (manually though :).
> I'll send the final fix today.

The patch below should fix the problem. I would really appreaciate
some additional testing.
---
From: Cyrill Gorcunov <gorcunov@gmail.com>
Subject: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging

VM_SOFTDIRTY bit affects vma merge routine: if two VMAs has all
bits in vm_flags matched except dirty bit the kernel can't longer
merge them and this forces the kernel to generate new VMAs instead.

It finally may lead to the situation when userspace application
reaches vm.max_map_count limit and get crashed in worse case

 | (gimp:11768): GLib-ERROR **: gmem.c:110: failed to allocate 4096 bytes
 |
 | (file-tiff-load:12038): LibGimpBase-WARNING **: file-tiff-load: gimp_wire_read(): error
 | xinit: connection to X server lost
 |
 | waiting for X server to shut down
 | /usr/lib64/gimp/2.0/plug-ins/file-tiff-load terminated: Hangup
 | /usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
 | /usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup

https://bugzilla.kernel.org/show_bug.cgi?id=67651
https://bugzilla.gnome.org/show_bug.cgi?id=719619#c0

Initial problem came from missed VM_SOFTDIRTY in do_brk() routine
but even if we would set up VM_SOFTDIRTY here, there is still a way to
prevent VMAs from merging: one can call

 | echo 4 > /proc/$PID/clear_refs

and clear all VM_SOFTDIRTY over all VMAs presented in memory map,
then new do_brk() will try to extend old VMA and finds that dirty
bit doesn't match thus new VMA will be generated.

As discussed to Pavel, the right approach should be to ignore
VM_SOFTDIRTY bit when we're trying to merge VMAs and if merged
successed we mark extended VMA with dirty bit.

Reported-by: Mel Gorman <mgorman@suse.de>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Andrew Morton <akpm@linux-foundation.org>
---
 mm/mmap.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Index: linux-2.6.git/mm/mmap.c
===================================================================
--- linux-2.6.git.orig/mm/mmap.c
+++ linux-2.6.git/mm/mmap.c
@@ -893,7 +893,15 @@ again:			remove_next = 1 + (end > next->
 static inline int is_mergeable_vma(struct vm_area_struct *vma,
 			struct file *file, unsigned long vm_flags)
 {
-	if (vma->vm_flags ^ vm_flags)
+	/*
+	 * VM_SOFTDIRTY should not prevent from VMA merging, if we
+	 * match the flags but dirty bit -- the caller should mark
+	 * merged VMA as dirty. If dirty bit won't be excluded from
+	 * comparison, we increase pressue on the memory system forcing
+	 * the kernel to generate new VMAs when old one could be extended
+	 * instead.
+	 */
+	if ((vma->vm_flags ^ vm_flags) & VM_SOFTDIRTY)
 		return 0;
 	if (vma->vm_file != file)
 		return 0;
@@ -1038,6 +1046,8 @@ struct vm_area_struct *vma_merge(struct
 				end, prev->vm_pgoff, NULL);
 		if (err)
 			return NULL;
+		else
+			prev->vm_flags |= VM_SOFTDIRTY;
 		khugepaged_enter_vma_merge(prev);
 		return prev;
 	}
@@ -1057,6 +1067,8 @@ struct vm_area_struct *vma_merge(struct
 				next->vm_pgoff - pglen, NULL);
 		if (err)
 			return NULL;
+		else
+			area->vm_flags |= VM_SOFTDIRTY;
 		khugepaged_enter_vma_merge(area);
 		return area;
 	}
@@ -1082,7 +1094,7 @@ static int anon_vma_compatible(struct vm
 	return a->vm_end == b->vm_start &&
 		mpol_equal(vma_policy(a), vma_policy(b)) &&
 		a->vm_file == b->vm_file &&
-		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC)) &&
+		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
 		b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
 }
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Cyrill Gorcunov]

On Thu, Jan 23, 2014 at 04:15:55PM +0400, Cyrill Gorcunov wrote:
> On Thu, Jan 23, 2014 at 02:36:06PM +0400, Cyrill Gorcunov wrote:
> > > The test case passes with this patch applied to 3.13 so that appears
> > > to confirm that this is related to VM_SOFTDIRTY preventing merges.
> > > Unfortunately I did not have slabinfo tracking enabled to double check
> > > the number of vm_area_structs in teh system.
> > 
> > Thanks a lot, Mel! I'm testing the patch as well (manually though :).
> > I'll send the final fix today.
> 
> The patch below should fix the problem. I would really appreaciate
> some additional testing.

Forgot to refresh the patch, sorry.
---
From: Cyrill Gorcunov <gorcunov@gmail.com>
Subject: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging

VM_SOFTDIRTY bit affects vma merge routine: if two VMAs has all
bits in vm_flags matched except dirty bit the kernel can't longer
merge them and this forces the kernel to generate new VMAs instead.

It finally may lead to the situation when userspace application
reaches vm.max_map_count limit and get crashed in worse case

 | (gimp:11768): GLib-ERROR **: gmem.c:110: failed to allocate 4096 bytes
 |
 | (file-tiff-load:12038): LibGimpBase-WARNING **: file-tiff-load: gimp_wire_read(): error
 | xinit: connection to X server lost
 |
 | waiting for X server to shut down
 | /usr/lib64/gimp/2.0/plug-ins/file-tiff-load terminated: Hangup
 | /usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
 | /usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup

https://bugzilla.kernel.org/show_bug.cgi?id=67651
https://bugzilla.gnome.org/show_bug.cgi?id=719619#c0

Initial problem came from missed VM_SOFTDIRTY in do_brk() routine
but even if we would set up VM_SOFTDIRTY here, there is still a way to
prevent VMAs from merging: one can call

 | echo 4 > /proc/$PID/clear_refs

and clear all VM_SOFTDIRTY over all VMAs presented in memory map,
then new do_brk() will try to extend old VMA and finds that dirty
bit doesn't match thus new VMA will be generated.

As discussed to Pavel, the right approach should be to ignore
VM_SOFTDIRTY bit when we're trying to merge VMAs and if merged
successed we mark extended VMA with dirty bit.

Reported-by: Mel Gorman <mgorman@suse.de>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Andrew Morton <akpm@linux-foundation.org>
---
 mm/mmap.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Index: linux-2.6.git/mm/mmap.c
===================================================================
--- linux-2.6.git.orig/mm/mmap.c
+++ linux-2.6.git/mm/mmap.c
@@ -893,7 +893,15 @@ again:			remove_next = 1 + (end > next->
 static inline int is_mergeable_vma(struct vm_area_struct *vma,
 			struct file *file, unsigned long vm_flags)
 {
-	if (vma->vm_flags ^ vm_flags)
+	/*
+	 * VM_SOFTDIRTY should not prevent from VMA merging, if we
+	 * match the flags but dirty bit -- the caller should mark
+	 * merged VMA as dirty. If dirty bit won't be excluded from
+	 * comparison, we increase pressue on the memory system forcing
+	 * the kernel to generate new VMAs when old one could be extended
+	 * instead.
+	 */
+	if ((vma->vm_flags ^ vm_flags) & ~VM_SOFTDIRTY)
 		return 0;
 	if (vma->vm_file != file)
 		return 0;
@@ -1038,6 +1046,8 @@ struct vm_area_struct *vma_merge(struct
 				end, prev->vm_pgoff, NULL);
 		if (err)
 			return NULL;
+		else
+			prev->vm_flags |= VM_SOFTDIRTY;
 		khugepaged_enter_vma_merge(prev);
 		return prev;
 	}
@@ -1057,6 +1067,8 @@ struct vm_area_struct *vma_merge(struct
 				next->vm_pgoff - pglen, NULL);
 		if (err)
 			return NULL;
+		else
+			area->vm_flags |= VM_SOFTDIRTY;
 		khugepaged_enter_vma_merge(area);
 		return area;
 	}
@@ -1082,7 +1094,7 @@ static int anon_vma_compatible(struct vm
 	return a->vm_end == b->vm_start &&
 		mpol_equal(vma_policy(a), vma_policy(b)) &&
 		a->vm_file == b->vm_file &&
-		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC)) &&
+		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
 		b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
 }
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

[Cyrill Gorcunov]

On Thu, Jan 23, 2014 at 04:55:43PM +0400, Cyrill Gorcunov wrote:
> On Thu, Jan 23, 2014 at 04:15:55PM +0400, Cyrill Gorcunov wrote:
> > > 
> > > Thanks a lot, Mel! I'm testing the patch as well (manually though :).
> > > I'll send the final fix today.
> > 
> > The patch below should fix the problem. I would really appreaciate
> > some additional testing.
> 
> Forgot to refresh the patch, sorry.
> ---

I think setting up dirty bit inside vma_merge() body is a big hammer
which should not be used, but it's up to caller of vma_merge() to figure
out if dirty bit should be set or not if merge successed. Thus softdirty
vma bit should be (and it already is) set at the end of mmap_region and do_brk
routines. So patch could be simplified (below). Pavel, what do you think?
---
From: Cyrill Gorcunov <gorcunov@gmail.com>
Subject: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

VM_SOFTDIRTY bit affects vma merge routine: if two VMAs has all
bits in vm_flags matched except dirty bit the kernel can't longer
merge them and this forces the kernel to generate new VMAs instead.

It finally may lead to the situation when userspace application
reaches vm.max_map_count limit and get crashed in worse case

 | (gimp:11768): GLib-ERROR **: gmem.c:110: failed to allocate 4096 bytes
 |
 | (file-tiff-load:12038): LibGimpBase-WARNING **: file-tiff-load: gimp_wire_read(): error
 | xinit: connection to X server lost
 |
 | waiting for X server to shut down
 | /usr/lib64/gimp/2.0/plug-ins/file-tiff-load terminated: Hangup
 | /usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
 | /usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup

https://bugzilla.kernel.org/show_bug.cgi?id=67651
https://bugzilla.gnome.org/show_bug.cgi?id=719619#c0

Initial problem came from missed VM_SOFTDIRTY in do_brk() routine
but even if we would set up VM_SOFTDIRTY here, there is still a way to
prevent VMAs from merging: one can call

 | echo 4 > /proc/$PID/clear_refs

and clear all VM_SOFTDIRTY over all VMAs presented in memory map,
then new do_brk() will try to extend old VMA and finds that dirty
bit doesn't match thus new VMA will be generated.

As discussed to Pavel, the right approach should be to ignore
VM_SOFTDIRTY bit when we're trying to merge VMAs and if merge
successed we mark extended VMA with dirty bit where needed.

v2: Don't mark VMA as dirty inside vma_merge() body, it's up
    to calling code to set up dirty bit where needed.

Reported-by: Mel Gorman <mgorman@suse.de>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Andrew Morton <akpm@linux-foundation.org>
---
 mm/mmap.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Index: linux-2.6.git/mm/mmap.c
===================================================================
--- linux-2.6.git.orig/mm/mmap.c
+++ linux-2.6.git/mm/mmap.c
@@ -893,7 +893,15 @@ again:			remove_next = 1 + (end > next->
 static inline int is_mergeable_vma(struct vm_area_struct *vma,
 			struct file *file, unsigned long vm_flags)
 {
-	if (vma->vm_flags ^ vm_flags)
+	/*
+	 * VM_SOFTDIRTY should not prevent from VMA merging, if we
+	 * match the flags but dirty bit -- the caller should mark
+	 * merged VMA as dirty. If dirty bit won't be excluded from
+	 * comparison, we increase pressue on the memory system forcing
+	 * the kernel to generate new VMAs when old one could be
+	 * extended instead.
+	 */
+	if ((vma->vm_flags ^ vm_flags) & ~VM_SOFTDIRTY)
 		return 0;
 	if (vma->vm_file != file)
 		return 0;
@@ -1082,7 +1090,7 @@ static int anon_vma_compatible(struct vm
 	return a->vm_end == b->vm_start &&
 		mpol_equal(vma_policy(a), vma_policy(b)) &&
 		a->vm_file == b->vm_file &&
-		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC)) &&
+		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
 		b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
 }
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

[Pavel Emelyanov]

On 01/23/2014 07:14 PM, Cyrill Gorcunov wrote:

> I think setting up dirty bit inside vma_merge() body is a big hammer
> which should not be used, but it's up to caller of vma_merge() to figure
> out if dirty bit should be set or not if merge successed. Thus softdirty
> vma bit should be (and it already is) set at the end of mmap_region and do_brk
> routines. So patch could be simplified (below). Pavel, what do you think?

Looks correct, thank you!

Acked-by: Pavel Emelyanov <xemul@parallels,com>

> ---
> From: Cyrill Gorcunov <gorcunov@gmail.com>
> Subject: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2
> 
> VM_SOFTDIRTY bit affects vma merge routine: if two VMAs has all
> bits in vm_flags matched except dirty bit the kernel can't longer
> merge them and this forces the kernel to generate new VMAs instead.
> 
> It finally may lead to the situation when userspace application
> reaches vm.max_map_count limit and get crashed in worse case
> 
>  | (gimp:11768): GLib-ERROR **: gmem.c:110: failed to allocate 4096 bytes
>  |
>  | (file-tiff-load:12038): LibGimpBase-WARNING **: file-tiff-load: gimp_wire_read(): error
>  | xinit: connection to X server lost
>  |
>  | waiting for X server to shut down
>  | /usr/lib64/gimp/2.0/plug-ins/file-tiff-load terminated: Hangup
>  | /usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
>  | /usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=67651
> https://bugzilla.gnome.org/show_bug.cgi?id=719619#c0
> 
> Initial problem came from missed VM_SOFTDIRTY in do_brk() routine
> but even if we would set up VM_SOFTDIRTY here, there is still a way to
> prevent VMAs from merging: one can call
> 
>  | echo 4 > /proc/$PID/clear_refs
> 
> and clear all VM_SOFTDIRTY over all VMAs presented in memory map,
> then new do_brk() will try to extend old VMA and finds that dirty
> bit doesn't match thus new VMA will be generated.
> 
> As discussed to Pavel, the right approach should be to ignore
> VM_SOFTDIRTY bit when we're trying to merge VMAs and if merge
> successed we mark extended VMA with dirty bit where needed.
> 
> v2: Don't mark VMA as dirty inside vma_merge() body, it's up
>     to calling code to set up dirty bit where needed.
> 
> Reported-by: Mel Gorman <mgorman@suse.de>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> CC: Pavel Emelyanov <xemul@parallels.com>
> CC: Andrew Morton <akpm@linux-foundation.org>
> ---
>  mm/mmap.c |   12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.git/mm/mmap.c
> ===================================================================
> --- linux-2.6.git.orig/mm/mmap.c
> +++ linux-2.6.git/mm/mmap.c
> @@ -893,7 +893,15 @@ again:			remove_next = 1 + (end > next->
>  static inline int is_mergeable_vma(struct vm_area_struct *vma,
>  			struct file *file, unsigned long vm_flags)
>  {
> -	if (vma->vm_flags ^ vm_flags)
> +	/*
> +	 * VM_SOFTDIRTY should not prevent from VMA merging, if we
> +	 * match the flags but dirty bit -- the caller should mark
> +	 * merged VMA as dirty. If dirty bit won't be excluded from
> +	 * comparison, we increase pressue on the memory system forcing
> +	 * the kernel to generate new VMAs when old one could be
> +	 * extended instead.
> +	 */
> +	if ((vma->vm_flags ^ vm_flags) & ~VM_SOFTDIRTY)
>  		return 0;
>  	if (vma->vm_file != file)
>  		return 0;
> @@ -1082,7 +1090,7 @@ static int anon_vma_compatible(struct vm
>  	return a->vm_end == b->vm_start &&
>  		mpol_equal(vma_policy(a), vma_policy(b)) &&
>  		a->vm_file == b->vm_file &&
> -		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC)) &&
> +		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
>  		b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
>  }
>  
> .
> 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

[Andrew Morton]

On Thu, 23 Jan 2014 19:14:45 +0400 Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> VM_SOFTDIRTY bit affects vma merge routine: if two VMAs has all
> bits in vm_flags matched except dirty bit the kernel can't longer
> merge them and this forces the kernel to generate new VMAs instead.

Do you intend to alter the brk() and binprm code to set VM_SOFTDIRTY?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

[Cyrill Gorcunov]

On Thu, Jan 23, 2014 at 01:02:35PM -0800, Andrew Morton wrote:
> On Thu, 23 Jan 2014 19:14:45 +0400 Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> 
> > VM_SOFTDIRTY bit affects vma merge routine: if two VMAs has all
> > bits in vm_flags matched except dirty bit the kernel can't longer
> > merge them and this forces the kernel to generate new VMAs instead.
> 
> Do you intend to alter the brk() and binprm code to set VM_SOFTDIRTY?

brk() will be "dirtified" now with this merge fix.
brk
  do_brk
    out:
	...
	vma->vm_flags |= VM_SOFTDIRTY;

this will work even if vma get merged, the problem was that earlier
we tried to merge without VM_SOFTDIRTY flag. And matcher failed.

do_brk
  flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
	vma = vma_merge(mm, prev, addr, addr + len, flags,
					NULL, NULL, pgoff, NULL);
	if (vma)
		goto out;
...
out:
	...
	vma->vm_flags |= VM_SOFTDIRTY;

That said I'm not really sure now if I should alert @flags in code above.
Should I add VM_SOFTDIRTY into @flags for clarity?

Same for binprm -- the vma allocated for bprm->vma is dirtified
__bprm_mm_init
  vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;

then setup_arg_pages calls mprotect_fixup with @vm_flags having dirty bit
set thus it'll be propagated to vma

mprotect_fixup
  ...
  vma->vm_flags = newflags;

the @newflags will have dirty bit set from caller code.

Or you mean something else which I'm missing?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

[Mel Gorman]

On Thu, Jan 23, 2014 at 07:14:45PM +0400, Cyrill Gorcunov wrote:
> On Thu, Jan 23, 2014 at 04:55:43PM +0400, Cyrill Gorcunov wrote:
> > On Thu, Jan 23, 2014 at 04:15:55PM +0400, Cyrill Gorcunov wrote:
> > > > 
> > > > Thanks a lot, Mel! I'm testing the patch as well (manually though :).
> > > > I'll send the final fix today.
> > > 
> > > The patch below should fix the problem. I would really appreaciate
> > > some additional testing.
> > 
> > Forgot to refresh the patch, sorry.
> > ---
> 
> I think setting up dirty bit inside vma_merge() body is a big hammer
> which should not be used, but it's up to caller of vma_merge() to figure
> out if dirty bit should be set or not if merge successed. Thus softdirty
> vma bit should be (and it already is) set at the end of mmap_region and do_brk
> routines. So patch could be simplified (below). Pavel, what do you think?
> ---
> From: Cyrill Gorcunov <gorcunov@gmail.com>
> Subject: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2
> 

It passed the gimp launching test. Patch looks sane but I confess I did
not put a whole lot of thought into it because I see that Andrew is
already reviewing it so

Tested-by: Mel Gorman <mgorman@suse.de>

If this is merged then remember that it should be tagged for 3.12-stable
as 3.12.7 and 3.12.8 are affected by this bug.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

[Cyrill Gorcunov]

On Fri, Jan 24, 2014 at 10:14:16AM +0000, Mel Gorman wrote:
> > From: Cyrill Gorcunov <gorcunov@gmail.com>
> > Subject: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2
> > 
> 
> It passed the gimp launching test. Patch looks sane but I confess I did
> not put a whole lot of thought into it because I see that Andrew is
> already reviewing it so
> 
> Tested-by: Mel Gorman <mgorman@suse.de>
> 
> If this is merged then remember that it should be tagged for 3.12-stable
> as 3.12.7 and 3.12.8 are affected by this bug.

Thanks a huge, Mel! Andrew has picked it up and CC'ed stable@ team.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

[Mel Gorman]

On Fri, Jan 24, 2014 at 03:56:29PM +0400, Cyrill Gorcunov wrote:
> On Fri, Jan 24, 2014 at 10:14:16AM +0000, Mel Gorman wrote:
> > > From: Cyrill Gorcunov <gorcunov@gmail.com>
> > > Subject: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2
> > > 
> > 
> > It passed the gimp launching test. Patch looks sane but I confess I did
> > not put a whole lot of thought into it because I see that Andrew is
> > already reviewing it so
> > 
> > Tested-by: Mel Gorman <mgorman@suse.de>
> > 
> > If this is merged then remember that it should be tagged for 3.12-stable
> > as 3.12.7 and 3.12.8 are affected by this bug.
> 
> Thanks a huge, Mel! Andrew has picked it up and CC'ed stable@ team.

Big thanks to the gimp developers that actually pinned this down as a
kernel bug and the people who shoved it through the kernel bugzilla. I
just did a light bit of legwork shuffling the paperwork around :P

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Ignore VM_SOFTDIRTY on VMA merging, v2

[Cyrill Gorcunov]

On Fri, Jan 24, 2014 at 01:41:35PM +0000, Mel Gorman wrote:
> > 
> > Thanks a huge, Mel! Andrew has picked it up and CC'ed stable@ team.
> 
> Big thanks to the gimp developers that actually pinned this down as a
> kernel bug and the people who shoved it through the kernel bugzilla. I
> just did a light bit of legwork shuffling the paperwork around :P

Sure! The help in testing and finding kernel problem is always invaluable!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Mel Gorman]

On Thu, Jan 23, 2014 at 02:33:25AM +0400, Cyrill Gorcunov wrote:
> On Wed, Jan 22, 2014 at 11:19:28PM +0400, Cyrill Gorcunov wrote:
> > > commit. Test case was simple -- try and open the large file described in
> > > the bug. I did not investigate the patch itself as I'm just reporting
> > > the results of the bisection. If I had to guess, I'd say that VMA
> > > merging has been affected.
> > 
> > Thanks a lot for report, Mel! I'm investigating...
> 
> Mel, here is a quick fix for bring merging back (just in case if you
> have a minute to test it and confirm the merging were affected). It
> seems I've lost setting up vma-softdirty bit somewhere and procedure
> which tests vma flags mathcing fails, will continue investigating/testing
> tomorrow.

The test case passes with this patch applied to 3.13 so that appears
to confirm that this is related to VM_SOFTDIRTY preventing merges.
Unfortunately I did not have slabinfo tracking enabled to double check
the number of vm_area_structs in teh system.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Andrew Morton]

On Wed, 22 Jan 2014 19:08:16 +0000 Mel Gorman <mgorman@suse.de> wrote:

> X-related junk is there was because I was using a headless server and
> xinit directly to launch gimp to reproduce the bug.

I've never done this.  Can you share the magic recipe for running an X
app in this way?

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Mel Gorman]

On Wed, Jan 22, 2014 at 11:52:15AM -0800, Andrew Morton wrote:
> On Wed, 22 Jan 2014 19:08:16 +0000 Mel Gorman <mgorman@suse.de> wrote:
> 
> > X-related junk is there was because I was using a headless server and
> > xinit directly to launch gimp to reproduce the bug.
> 
> I've never done this.  Can you share the magic recipe for running an X
> app in this way?
> 

The relevant part of the test script is

# Build a wrapper script to launch gimp
cat > gimp-launch.sh << EOF
/usr/bin/gimp -i -b "(mmtests-open-image \"$FILENAME\")" -b "(gimp-quit 0)" > $LOGDIR_RESULTS/gimp-out.1 2>&1
echo \$? > gimp-exit-code
EOF
chmod u+x gimp-launch.sh

$TIME_CMD xinit ./gimp-launch.sh 2> $LOGDIR_RESULTS/time.1
RETVAL=`cat gimp-exit-code`

It's clumsy because the application would start with no window manager
and looking at it again, it probably was not even necessary because of
the -i switch in gimp.

Previously when I needed to automate an X app I configured the machine to
login automatically, exported the DISPLAY variable in the test script and
used wmctrl to detect if an application had a window displayed yet.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Andy Lutomirski]

On 01/22/2014 11:08 AM, Mel Gorman wrote:
> Cyrill,
> 
> Gimp is broken due to a kernel bug included in 3.12. It cannot open
> large files without failing memory allocations due to exceeding
> vm.max_map_count. The relevant bugzilla entries are
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=67651
> https://bugzilla.gnome.org/show_bug.cgi?id=719619#c0
> 
> They include details on how to reproduce the issue. In my case, a
> failure shows messages like this
> 
> 	(gimp:11768): GLib-ERROR **: gmem.c:110: failed to allocate 4096 bytes
> 
> 	(file-tiff-load:12038): LibGimpBase-WARNING **: file-tiff-load: gimp_wire_read(): error
> 	xinit: connection to X server lost
> 
> 	waiting for X server to shut down
> 	/usr/lib64/gimp/2.0/plug-ins/file-tiff-load terminated: Hangup
> 	/usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
> 	/usr/lib64/gimp/2.0/plug-ins/script-fu terminated: Hangup
> 
> X-related junk is there was because I was using a headless server and
> xinit directly to launch gimp to reproduce the bug.
> 
> Automated bisection using mmtests (https://github.com/gormanm/mmtests)
> and the configuration file configs/config-global-dhp__gimp-simple (needs
> local web server with a copy of the image file) identified the following
> commit. Test case was simple -- try and open the large file described in
> the bug. I did not investigate the patch itself as I'm just reporting
> the results of the bisection. If I had to guess, I'd say that VMA
> merging has been affected.
> 
> d9104d1ca9662498339c0de975b4666c30485f4e is the first bad commit
> commit d9104d1ca9662498339c0de975b4666c30485f4e
> Author: Cyrill Gorcunov <gorcunov@gmail.com>
> Date:   Wed Sep 11 14:22:24 2013 -0700
> 
>     mm: track vma changes with VM_SOFTDIRTY bit
>     
>     Pavel reported that in case if vma area get unmapped and then mapped (or
>     expanded) in-place, the soft dirty tracker won't be able to recognize this
>     situation since it works on pte level and ptes are get zapped on unmap,
>     loosing soft dirty bit of course.
>     
>     So to resolve this situation we need to track actions on vma level, there
>     VM_SOFTDIRTY flag comes in.  When new vma area created (or old expanded)
>     we set this bit, and keep it here until application calls for clearing
>     soft dirty bit.
>     
>     Thus when user space application track memory changes now it can detect if
>     vma area is renewed.

Presumably some path is failing to set VM_SOFTDIRTY, thus preventing mms
from being merged.

That being said, this could cause vma blowups for programs that are
actually using this thing.

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Cyrill Gorcunov]

On Wed, Jan 22, 2014 at 02:45:53PM -0800, Andy Lutomirski wrote:
> >     
> >     Thus when user space application track memory changes now it can detect if
> >     vma area is renewed.
> 
> Presumably some path is failing to set VM_SOFTDIRTY, thus preventing mms
> from being merged.
> 
> That being said, this could cause vma blowups for programs that are
> actually using this thing.

Hi Andy, indeed, this could happen. The easiest way is to ignore softdirty bit
when we're trying to merge vmas and set it one new merged. I think this should
be correct. Once I finish I'll send the patch.

	Cyrill

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Andrew Morton]

On Thu, 23 Jan 2014 09:59:06 +0400 Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> On Wed, Jan 22, 2014 at 02:45:53PM -0800, Andy Lutomirski wrote:
> > >     
> > >     Thus when user space application track memory changes now it can detect if
> > >     vma area is renewed.
> > 
> > Presumably some path is failing to set VM_SOFTDIRTY, thus preventing mms
> > from being merged.
> > 
> > That being said, this could cause vma blowups for programs that are
> > actually using this thing.
> 
> Hi Andy, indeed, this could happen. The easiest way is to ignore softdirty bit
> when we're trying to merge vmas and set it one new merged. I think this should
> be correct. Once I finish I'll send the patch.

Hang on.  We think the problem is that gimp is generating vmas which
*should* be merged, but for unknown reasons they differ in
VM_SOFTDIRTY, yes?

Shouldn't we work out where we're forgetting to set VM_SOFTDIRTY? 
Putting bandaids over this error when we come to trying to merge the
vmas sounds very wrong?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Cyrill Gorcunov]

On Wed, Jan 22, 2014 at 10:09:10PM -0800, Andrew Morton wrote:
> > > 
> > > That being said, this could cause vma blowups for programs that are
> > > actually using this thing.
> > 
> > Hi Andy, indeed, this could happen. The easiest way is to ignore softdirty bit
> > when we're trying to merge vmas and set it one new merged. I think this should
> > be correct. Once I finish I'll send the patch.
> 
> Hang on.  We think the problem is that gimp is generating vmas which
> *should* be merged, but for unknown reasons they differ in
> VM_SOFTDIRTY, yes?

Yes. One place where I forgot to set softdirty bit is setup_arg_pages. But
it called once on elf load, so it can't cause such effect (but should be
fixed too). Also there is do_brk where vmasoftdirty is missed too :/

Another problem is the potential scenario when we have a bunch of vmas
and clear vma-softdirty bit on them, then we try to map new one, flags
won't match and instead of extending old vma the new one will be created.
I think (if only I'm not missing something) that vma-softdirty should
be ignored in such case (ie inside is_mergeable_vma) and once vma extended
it should be marked as dirty one. Again, I need to think and test more.

> Shouldn't we work out where we're forgetting to set VM_SOFTDIRTY? 
> Putting bandaids over this error when we come to trying to merge the
> vmas sounds very wrong?

I'm looking into this as well.

	Cyrill

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Bug 67651] Bisected: Lots of fragmented mmaps cause gimp to fail in 3.12 after exceeding vm_max_map_count

[Muhammad Usama Anjum]

Sorry for replying to so much older thread. The original thread:
https://lore.kernel.org/all/20140123151445.GX1574@moon/

On 1/23/14 11:27 AM, Cyrill Gorcunov wrote:
> On Wed, Jan 22, 2014 at 10:09:10PM -0800, Andrew Morton wrote:
>>>>
>>>> That being said, this could cause vma blowups for programs that are
>>>> actually using this thing.
>>>
>>> Hi Andy, indeed, this could happen. The easiest way is to ignore softdirty bit
>>> when we're trying to merge vmas and set it one new merged. I think this should
>>> be correct. Once I finish I'll send the patch.
>>
>> Hang on.  We think the problem is that gimp is generating vmas which
>> *should* be merged, but for unknown reasons they differ in
>> VM_SOFTDIRTY, yes?
> 
> Yes. One place where I forgot to set softdirty bit is setup_arg_pages. But
> it called once on elf load, so it can't cause such effect (but should be
> fixed too). Also there is do_brk where vmasoftdirty is missed too :/
> 
> Another problem is the potential scenario when we have a bunch of vmas
> and clear vma-softdirty bit on them, then we try to map new one, flags
> won't match and instead of extending old vma the new one will be created.
> I think (if only I'm not missing something) that vma-softdirty should
> be ignored in such case (ie inside is_mergeable_vma) and once vma extended
> it should be marked as dirty one. Again, I need to think and test more.
> 
>> Shouldn't we work out where we're forgetting to set VM_SOFTDIRTY? 
>> Putting bandaids over this error when we come to trying to merge the
>> vmas sounds very wrong?
> 
> I'm looking into this as well.
I've looked at it while working on adding an IOCTL to get and/or clear the
soft dirty bit for a particular region only [1]. The VM_SOFTDIRTY should be
set in the vm_flags to be tested if new allocation should be merged in
previous vma or not. With the following patch, the new allocations are
merged in the previous VMAs.

I've tested it by reverting the 34228d473efe ("mm: ignore VM_SOFTDIRTY on
VMA merging") commit and after adding the following patch, I'm seeing that
all the new allocations done through mmap() (in my testing) are merged in
the previous VMAs. The number of VMAs doesn't increase drastically which
had contributed to the crash of gimp. If I run the same test after
reverting and not including the following, the number of VMAs keep on
increasing with every mmap() syscall which proves this patch.

diff --git a/mm/mmap.c b/mm/mmap.c
index f9b96b387a6f..b132d52f6fe1 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1708,6 +1708,15 @@ unsigned long mmap_region(struct file *file,
unsigned long addr,
 		vm_flags |= VM_ACCOUNT;
 	}

+	/*
+	 * New (or expanded) vma always get soft dirty status.
+	 * Otherwise user-space soft-dirty page tracker won't
+	 * be able to distinguish situation when vma area unmapped,
+	 * then new mapped in-place (which must be aimed as
+	 * a completely new data area).
+	 */
+	vm_flags |= VM_SOFTDIRTY;
+
 	/*
 	 * Can we just expand an old mapping?
 	 */
@@ -1823,15 +1832,6 @@ unsigned long mmap_region(struct file *file,
unsigned long addr,
 	if (file)
 		uprobe_mmap(vma);

-	/*
-	 * New (or expanded) vma always get soft dirty status.
-	 * Otherwise user-space soft-dirty page tracker won't
-	 * be able to distinguish situation when vma area unmapped,
-	 * then new mapped in-place (which must be aimed as
-	 * a completely new data area).
-	 */
-	vma->vm_flags |= VM_SOFTDIRTY;
-
 	vma_set_page_prot(vma);

 	return addr;
@@ -2998,6 +2998,8 @@ static int do_brk_flags(unsigned long addr, unsigned
long len, unsigned long fla
 	if (security_vm_enough_memory_mm(mm, len >> PAGE_SHIFT))
 		return -ENOMEM;

+	flags |= VM_SOFTDIRTY;
+
 	/* Can we just expand an old private anonymous mapping? */
 	vma = vma_merge(mm, prev, addr, addr + len, flags,
 			NULL, NULL, pgoff, NULL, NULL_VM_UFFD_CTX, NULL);
@@ -3026,7 +3028,7 @@ static int do_brk_flags(unsigned long addr, unsigned
long len, unsigned long fla
 	mm->data_vm += len >> PAGE_SHIFT;
 	if (flags & VM_LOCKED)
 		mm->locked_vm += (len >> PAGE_SHIFT);
-	vma->vm_flags |= VM_SOFTDIRTY;
+
 	return 0;
 }

This patch should be included in the kernel regardless if we revert
34228d473efe or not. Thoughts?


Cyrill is correct about the following:
> Another problem is the potential scenario when we have a bunch of vmas
> and clear vma-softdirty bit on them, then we try to map new one, flags
> won't match and instead of extending old vma the new one will be created.
> I think (if only I'm not missing something) that vma-softdirty should
> be ignored in such case (ie inside is_mergeable_vma) and once vma
> extended
> it should be marked as dirty one. Again, I need to think and test more.
While implementing clear soft-dirty bit for a range of address space, I'm
facing an issue. The non-soft dirty VMA gets merged sometimes with the soft
dirty VMA. Thus the non-soft dirty VMA become dirty which is undesirable.
When discussed with the some other developers they consider it the
regression. Why the non-soft dirty page should appear as soft dirty when it
isn't soft dirty in reality? I agree with them. Should we revert
34228d473efe or find a workaround in the IOCTL?

* Revert may cause the VMAs to expand in uncontrollable situation where the
soft dirty bit of a lot of memory regions or the whole address space is
being cleared again and again. AFAIK normal process must either be only
clearing a few memory regions. So the applications should be okay. There is
still chance of regressions if some applications are already using the
soft-dirty bit. I'm not sure how to test it.

* Add a flag in the IOCTL to ignore the dirtiness of VMA. The user will
surely lose the functionality to detect reused memory regions. But the
extraneous soft-dirty pages would not appear. I'm trying to do this in the
patch series [1]. Some discussion is going on that this fails with some
mprotect use case [2]. I still need to have a look at the mprotect selftest
to see how and why this fails. I think this can be implemented after some
more work.

What are your thoughts?

> 
> 	Cyrill
> 

[1]
https://lore.kernel.org/all/20221109102303.851281-1-usama.anjum@collabora.com/
[2]
https://lore.kernel.org/all/bfcae708-db21-04b4-0bbe-712badd03071@redhat.com/

-- 
BR,
Muhammad Usama Anjum