[PATCH] mm: avoid dirtying shared mappings on mlock

[PATCH] mm: avoid dirtying shared mappings on mlock

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

Subject: mm: avoid dirtying shared mappings on mlock

Suleiman noticed that shared mappings get dirtied when mlocked.
Avoid this by teaching make_pages_present about this case.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: Suleiman Souhlal <suleiman@google.com>
---
 mm/memory.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-2.6/mm/memory.c
===================================================================
--- linux-2.6.orig/mm/memory.c
+++ linux-2.6/mm/memory.c
@@ -2719,7 +2719,12 @@ int make_pages_present(unsigned long add
 	vma = find_vma(current->mm, addr);
 	if (!vma)
 		return -1;
-	write = (vma->vm_flags & VM_WRITE) != 0;
+	/*
+	 * We want to touch writable mappings with a write fault in order
+	 * to break COW, except for shared mappings because these don't COW
+	 * and we would not want to dirty them for nothing.
+	 */
+	write = (vma->vm_flags & (VM_WRITE|VM_SHARED)) == VM_WRITE;
 	BUG_ON(addr >= end);
 	BUG_ON(end > vma->vm_end);
 	len = DIV_ROUND_UP(end, PAGE_SIZE) - addr/PAGE_SIZE;


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] mm: avoid dirtying shared mappings on mlock

[Nick Piggin]

On Friday 12 October 2007 19:03, Peter Zijlstra wrote:
> Subject: mm: avoid dirtying shared mappings on mlock
>
> Suleiman noticed that shared mappings get dirtied when mlocked.
> Avoid this by teaching make_pages_present about this case.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Acked-by: Suleiman Souhlal <suleiman@google.com>

Umm, I don't see the other piece of this thread, so I don't
know what the actual problem was.

But I would really rather not do this. If you do this, then you
now can get random SIGBUSes when you write into the memory if it
can't allocate blocks or ... (some other filesystem specific
condition).

I agree it feels suboptimal, but we've _always_ done this, right?
Is it suddenly a problem? Unless a really nice general way is
made to solve this, optimising it like this makes it harder to
ensure good semantics for all corner cases I think (and then once
the optimisation is in place, it's a lot harder to remove it).

I'll go away and have a better look for the old thread.

> ---
>  mm/memory.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> Index: linux-2.6/mm/memory.c
> ===================================================================
> --- linux-2.6.orig/mm/memory.c
> +++ linux-2.6/mm/memory.c
> @@ -2719,7 +2719,12 @@ int make_pages_present(unsigned long add
>  	vma = find_vma(current->mm, addr);
>  	if (!vma)
>  		return -1;
> -	write = (vma->vm_flags & VM_WRITE) != 0;
> +	/*
> +	 * We want to touch writable mappings with a write fault in order
> +	 * to break COW, except for shared mappings because these don't COW
> +	 * and we would not want to dirty them for nothing.
> +	 */
> +	write = (vma->vm_flags & (VM_WRITE|VM_SHARED)) == VM_WRITE;
>  	BUG_ON(addr >= end);
>  	BUG_ON(end > vma->vm_end);
>  	len = DIV_ROUND_UP(end, PAGE_SIZE) - addr/PAGE_SIZE;

--
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: avoid dirtying shared mappings on mlock

[Nick Piggin]

On Friday 12 October 2007 02:57, Nick Piggin wrote:
> On Friday 12 October 2007 19:03, Peter Zijlstra wrote:
> > Subject: mm: avoid dirtying shared mappings on mlock
> >
> > Suleiman noticed that shared mappings get dirtied when mlocked.
> > Avoid this by teaching make_pages_present about this case.
> >
> > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Acked-by: Suleiman Souhlal <suleiman@google.com>
>
> Umm, I don't see the other piece of this thread, so I don't
> know what the actual problem was.

Found it, but no more clues. Presumably it's some horrible
google workload... they're pretty happy to carry these kinds
of patches internally, right? ;)


> But I would really rather not do this. If you do this, then you
> now can get random SIGBUSes when you write into the memory if it
> can't allocate blocks or ... (some other filesystem specific
> condition).
>
> I agree it feels suboptimal, but we've _always_ done this, right?
> Is it suddenly a problem? Unless a really nice general way is
> made to solve this, optimising it like this makes it harder to
> ensure good semantics for all corner cases I think (and then once
> the optimisation is in place, it's a lot harder to remove it).

Yeah, I really would rather not do this. If we actually go
through the whole fault path in mlock, then it doesn't
really matter what future baggage we attach to fault handlers...
(OK, we still technically have some problems with invalidations,
but mostly they're avoidable unless doing something silly).

How about just doing another PROT_READ mmap, and mlocking that?
(I was going to suggest another syscall, but that's probably
overkill).

--
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: avoid dirtying shared mappings on mlock

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

On Fri, 2007-10-12 at 02:57 +1000, Nick Piggin wrote:
> On Friday 12 October 2007 19:03, Peter Zijlstra wrote:
> > Subject: mm: avoid dirtying shared mappings on mlock
> >
> > Suleiman noticed that shared mappings get dirtied when mlocked.
> > Avoid this by teaching make_pages_present about this case.
> >
> > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Acked-by: Suleiman Souhlal <suleiman@google.com>
> 
> Umm, I don't see the other piece of this thread, so I don't
> know what the actual problem was.
> 
> But I would really rather not do this. If you do this, then you
> now can get random SIGBUSes when you write into the memory if it
> can't allocate blocks or ... (some other filesystem specific
> condition).

I'm not getting this, make_pages_present() only has to ensure all the
pages are read from disk and in memory. How is this different from a
read-scan?

The pages will still be read-only due to dirty tracking, so the first
write will still do page_mkwrite().



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] mm: avoid dirtying shared mappings on mlock

[Nick Piggin]

On Friday 12 October 2007 20:37, Peter Zijlstra wrote:
> On Fri, 2007-10-12 at 02:57 +1000, Nick Piggin wrote:
> > On Friday 12 October 2007 19:03, Peter Zijlstra wrote:
> > > Subject: mm: avoid dirtying shared mappings on mlock
> > >
> > > Suleiman noticed that shared mappings get dirtied when mlocked.
> > > Avoid this by teaching make_pages_present about this case.
> > >
> > > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > Acked-by: Suleiman Souhlal <suleiman@google.com>
> >
> > Umm, I don't see the other piece of this thread, so I don't
> > know what the actual problem was.
> >
> > But I would really rather not do this. If you do this, then you
> > now can get random SIGBUSes when you write into the memory if it
> > can't allocate blocks or ... (some other filesystem specific
> > condition).
>
> I'm not getting this, make_pages_present() only has to ensure all the
> pages are read from disk and in memory. How is this different from a
> read-scan?

I guess because we've mlocked a region that has PROT_WRITE access...
but actually, I suppose mlock doesn't technically require that we
can write to the memory, only that the page isn't swapped out.

Still, it is nice to be able to have a reasonable guarantee of
writability.


> The pages will still be read-only due to dirty tracking, so the first
> write will still do page_mkwrite().

Which can SIGBUS, no?

--
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: avoid dirtying shared mappings on mlock

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]

On Fri, 2007-10-12 at 04:14 +1000, Nick Piggin wrote:
> On Friday 12 October 2007 20:37, Peter Zijlstra wrote:
> > On Fri, 2007-10-12 at 02:57 +1000, Nick Piggin wrote:
> > > On Friday 12 October 2007 19:03, Peter Zijlstra wrote:
> > > > Subject: mm: avoid dirtying shared mappings on mlock
> > > >
> > > > Suleiman noticed that shared mappings get dirtied when mlocked.
> > > > Avoid this by teaching make_pages_present about this case.
> > > >
> > > > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > > Acked-by: Suleiman Souhlal <suleiman@google.com>
> > >
> > > Umm, I don't see the other piece of this thread, so I don't
> > > know what the actual problem was.
> > >
> > > But I would really rather not do this. If you do this, then you
> > > now can get random SIGBUSes when you write into the memory if it
> > > can't allocate blocks or ... (some other filesystem specific
> > > condition).
> >
> > I'm not getting this, make_pages_present() only has to ensure all the
> > pages are read from disk and in memory. How is this different from a
> > read-scan?
> 
> I guess because we've mlocked a region that has PROT_WRITE access...
> but actually, I suppose mlock doesn't technically require that we
> can write to the memory, only that the page isn't swapped out.
> 
> Still, it is nice to be able to have a reasonable guarantee of
> writability.
> 
> 
> > The pages will still be read-only due to dirty tracking, so the first
> > write will still do page_mkwrite().
> 
> Which can SIGBUS, no?

Sure, but that is no different than any other mmap'ed write. I'm not
seeing how an mlocked region is special here.

I agree it would be nice if mmap'ed writes would have better error
reporting than SIGBUS, but such is life.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] mm: avoid dirtying shared mappings on mlock

[Nick Piggin]

On Friday 12 October 2007 20:50, Peter Zijlstra wrote:
> On Fri, 2007-10-12 at 04:14 +1000, Nick Piggin wrote:
> > On Friday 12 October 2007 20:37, Peter Zijlstra wrote:

> > > The pages will still be read-only due to dirty tracking, so the first
> > > write will still do page_mkwrite().
> >
> > Which can SIGBUS, no?
>
> Sure, but that is no different than any other mmap'ed write. I'm not
> seeing how an mlocked region is special here.

Well it is a change in behaviour (admittedly, so was the change
to SIGBUS mmaped writes in the first place). It's a matter of
semantics I guess. Is the current behaviour actually a _problem_
for anyone? If not, then do we need to change it?

I'm not saying it does matter, just that it might matter ;)

--
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: avoid dirtying shared mappings on mlock

[Arjan van de Ven]

On Fri, 12 Oct 2007 12:50:22 +0200
> > > The pages will still be read-only due to dirty tracking, so the
> > > first write will still do page_mkwrite().
> > 
> > Which can SIGBUS, no?
> 
> Sure, but that is no different than any other mmap'ed write. I'm not
> seeing how an mlocked region is special here.
> 
> I agree it would be nice if mmap'ed writes would have better error
> reporting than SIGBUS, but such is life.

well... there's another consideration
people use mlock() in cases where they don't want to go to the
filesystem for paging and stuff as well (think the various iscsi
daemons and other things that get in trouble).. those kind of uses
really use mlock to avoid
1) IO to the filesystem
2) Needing memory allocations for pagefault like things
at least for the more "hidden" cases...

prefaulting everything ready pretty much gives them that... letting
things fault on demand... nicely breaks that.

--
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: avoid dirtying shared mappings on mlock

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

On Fri, 2007-10-12 at 07:53 -0700, Arjan van de Ven wrote:
> On Fri, 12 Oct 2007 12:50:22 +0200
> > > > The pages will still be read-only due to dirty tracking, so the
> > > > first write will still do page_mkwrite().
> > > 
> > > Which can SIGBUS, no?
> > 
> > Sure, but that is no different than any other mmap'ed write. I'm not
> > seeing how an mlocked region is special here.
> > 
> > I agree it would be nice if mmap'ed writes would have better error
> > reporting than SIGBUS, but such is life.
> 
> well... there's another consideration
> people use mlock() in cases where they don't want to go to the
> filesystem for paging and stuff as well (think the various iscsi
> daemons and other things that get in trouble).. those kind of uses
> really use mlock to avoid
> 1) IO to the filesystem
> 2) Needing memory allocations for pagefault like things
> at least for the more "hidden" cases...
> 
> prefaulting everything ready pretty much gives them that... letting
> things fault on demand... nicely breaks that.

Non of that is changed. So I'm a little puzzled as to which side you
argue.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] mm: avoid dirtying shared mappings on mlock

[Suleiman Souhlal]

On Oct 12, 2007, at 7:58 AM, Peter Zijlstra wrote:

> On Fri, 2007-10-12 at 07:53 -0700, Arjan van de Ven wrote:
>> On Fri, 12 Oct 2007 12:50:22 +0200
>>>>> The pages will still be read-only due to dirty tracking, so the
>>>>> first write will still do page_mkwrite().
>>>>
>>>> Which can SIGBUS, no?
>>>
>>> Sure, but that is no different than any other mmap'ed write. I'm not
>>> seeing how an mlocked region is special here.
>>>
>>> I agree it would be nice if mmap'ed writes would have better error
>>> reporting than SIGBUS, but such is life.
>>
>> well... there's another consideration
>> people use mlock() in cases where they don't want to go to the
>> filesystem for paging and stuff as well (think the various iscsi
>> daemons and other things that get in trouble).. those kind of uses
>> really use mlock to avoid
>> 1) IO to the filesystem
>> 2) Needing memory allocations for pagefault like things
>> at least for the more "hidden" cases...
>>
>> prefaulting everything ready pretty much gives them that... letting
>> things fault on demand... nicely breaks that.
>
> Non of that is changed. So I'm a little puzzled as to which side you
> argue.

I think this might change the behavior in case you mlock sparse files.
I guess currently the holes disappear when you mlock them, but with  
the patch the blocks wouldn't get allocated until they get written to.

-- Suleiman

--
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: avoid dirtying shared mappings on mlock

[Arjan van de Ven]

On Fri, 12 Oct 2007 10:45:17 -0700
Suleiman Souhlal <ssouhlal@freebsd.org> wrote:

> 
> On Oct 12, 2007, at 7:58 AM, Peter Zijlstra wrote:
> 
> > On Fri, 2007-10-12 at 07:53 -0700, Arjan van de Ven wrote:
> >> On Fri, 12 Oct 2007 12:50:22 +0200
> >>>>> The pages will still be read-only due to dirty tracking, so the
> >>>>> first write will still do page_mkwrite().
> >>>>
> >>>> Which can SIGBUS, no?
> >>>
> >>> Sure, but that is no different than any other mmap'ed write. I'm
> >>> not seeing how an mlocked region is special here.
> >>>
> >>> I agree it would be nice if mmap'ed writes would have better error
> >>> reporting than SIGBUS, but such is life.
> >>
> >> well... there's another consideration
> >> people use mlock() in cases where they don't want to go to the
> >> filesystem for paging and stuff as well (think the various iscsi
> >> daemons and other things that get in trouble).. those kind of uses
> >> really use mlock to avoid
> >> 1) IO to the filesystem
> >> 2) Needing memory allocations for pagefault like things
> >> at least for the more "hidden" cases...
> >>
> >> prefaulting everything ready pretty much gives them that... letting
> >> things fault on demand... nicely breaks that.
> >
> > Non of that is changed. So I'm a little puzzled as to which side you
> > argue.
> 
> I think this might change the behavior in case you mlock sparse files.
> I guess currently the holes disappear when you mlock them, but with  
> the patch the blocks wouldn't get allocated until they get written to.

eh yeah I forgot to mention this was for the sparse case....

--
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: avoid dirtying shared mappings on mlock

[Peter Zijlstra]

On Fri, 2007-10-12 at 10:45 -0700, Suleiman Souhlal wrote:
> On Oct 12, 2007, at 7:58 AM, Peter Zijlstra wrote:
> 
> > On Fri, 2007-10-12 at 07:53 -0700, Arjan van de Ven wrote:
> >> On Fri, 12 Oct 2007 12:50:22 +0200
> >>>>> The pages will still be read-only due to dirty tracking, so the
> >>>>> first write will still do page_mkwrite().
> >>>>
> >>>> Which can SIGBUS, no?
> >>>
> >>> Sure, but that is no different than any other mmap'ed write. I'm not
> >>> seeing how an mlocked region is special here.
> >>>
> >>> I agree it would be nice if mmap'ed writes would have better error
> >>> reporting than SIGBUS, but such is life.
> >>
> >> well... there's another consideration
> >> people use mlock() in cases where they don't want to go to the
> >> filesystem for paging and stuff as well (think the various iscsi
> >> daemons and other things that get in trouble).. those kind of uses
> >> really use mlock to avoid
> >> 1) IO to the filesystem
> >> 2) Needing memory allocations for pagefault like things
> >> at least for the more "hidden" cases...
> >>
> >> prefaulting everything ready pretty much gives them that... letting
> >> things fault on demand... nicely breaks that.
> >
> > Non of that is changed. So I'm a little puzzled as to which side you
> > argue.
> 
> I think this might change the behavior in case you mlock sparse files.
> I guess currently the holes disappear when you mlock them, but with  
> the patch the blocks wouldn't get allocated until they get written to.

Sure, but by point 1 - avoiding IO - that doesn't matter. Once you write
to a shared mapping you'll generate IO and you'll hit kernel allocations
and other delays no matter what you do.


--
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>