[LSF/MM TOPIC] mmap_sem in ->fault and ->page_mkwrite

[LSF/MM TOPIC] mmap_sem in ->fault and ->page_mkwrite

[Jan Kara]

  Hi,

  I'm not sure if this is such a great topic but it's a question which
I came across a few times already and LSF/MM is a good place for
brainstorming somewhat crazy ideas ;).

So currently ->fault() and ->page_mkwrite() are called under mmap_sem held
for reading. Now this creates sometimes unpleasant locking dependencies for
filesystems (modern filesystems have to do an equivalent of ->write_begin
in ->page_mkwrite and that is a non-trivial operation). Just to mention my
last itch, I had to split reader side of filesystem freezing lock into two
locks - one which ranks above mmap_sem and one which ranks below it. Then
writer side has to wait for both locks. It works but ...

So I was wondering: Would it be somehow possible we could drop mmap_sem in
these two callbacks (especially ->page_mkwrite())? I understand process'
mapping can change under us once we drop the semaphore so we'd have to
somehow recheck we have still the right page after re-taking mmap_sem. Like
if we protected VMAs with SRCU so that they don't disappear under us once
we drop mmap_sem and after retaking mmap_sem we would recheck whether VMA
still applies to our fault.

And I know there's VM_FAULT_RETRY but that really seems like a special hack
for x86 architecture page fault code. Making it work for all architectures
and callers such as get_user_pages() didn't really seem plausible to me.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [LSF/MM TOPIC] mmap_sem in ->fault and ->page_mkwrite

[Al Viro]

On Thu, Jan 31, 2013 at 11:23:35PM +0100, Jan Kara wrote:
>   Hi,
> 
>   I'm not sure if this is such a great topic but it's a question which
> I came across a few times already and LSF/MM is a good place for
> brainstorming somewhat crazy ideas ;).
> 
> So currently ->fault() and ->page_mkwrite() are called under mmap_sem held
> for reading. Now this creates sometimes unpleasant locking dependencies for
> filesystems (modern filesystems have to do an equivalent of ->write_begin
> in ->page_mkwrite and that is a non-trivial operation). Just to mention my
> last itch, I had to split reader side of filesystem freezing lock into two
> locks - one which ranks above mmap_sem and one which ranks below it. Then
> writer side has to wait for both locks. It works but ...
> 
> So I was wondering: Would it be somehow possible we could drop mmap_sem in
> these two callbacks (especially ->page_mkwrite())? I understand process'
> mapping can change under us once we drop the semaphore so we'd have to
> somehow recheck we have still the right page after re-taking mmap_sem. Like
> if we protected VMAs with SRCU so that they don't disappear under us once
> we drop mmap_sem and after retaking mmap_sem we would recheck whether VMA
> still applies to our fault.
> 
> And I know there's VM_FAULT_RETRY but that really seems like a special hack
> for x86 architecture page fault code. Making it work for all architectures
> and callers such as get_user_pages() didn't really seem plausible to me.

Please, *please*, don't.  VMA locking is complete horror without SRCU
mess thrown in.  It's a bloody bad idea, at least without a very massive
cleanup prior to that thing.

Start with drawing the call graph for vma-related code - at least the
parts from relevant locks grabbed to accesses of fields protected by
said locks.

Re: [LSF/MM TOPIC] mmap_sem in ->fault and ->page_mkwrite

[Jan Kara]

On Thu 31-01-13 23:03:27, Al Viro wrote:
> On Thu, Jan 31, 2013 at 11:23:35PM +0100, Jan Kara wrote:
> >   Hi,
> > 
> >   I'm not sure if this is such a great topic but it's a question which
> > I came across a few times already and LSF/MM is a good place for
> > brainstorming somewhat crazy ideas ;).
> > 
> > So currently ->fault() and ->page_mkwrite() are called under mmap_sem held
> > for reading. Now this creates sometimes unpleasant locking dependencies for
> > filesystems (modern filesystems have to do an equivalent of ->write_begin
> > in ->page_mkwrite and that is a non-trivial operation). Just to mention my
> > last itch, I had to split reader side of filesystem freezing lock into two
> > locks - one which ranks above mmap_sem and one which ranks below it. Then
> > writer side has to wait for both locks. It works but ...
> > 
> > So I was wondering: Would it be somehow possible we could drop mmap_sem in
> > these two callbacks (especially ->page_mkwrite())? I understand process'
> > mapping can change under us once we drop the semaphore so we'd have to
> > somehow recheck we have still the right page after re-taking mmap_sem. Like
> > if we protected VMAs with SRCU so that they don't disappear under us once
> > we drop mmap_sem and after retaking mmap_sem we would recheck whether VMA
> > still applies to our fault.
> > 
> > And I know there's VM_FAULT_RETRY but that really seems like a special hack
> > for x86 architecture page fault code. Making it work for all architectures
> > and callers such as get_user_pages() didn't really seem plausible to me.
> 
> Please, *please*, don't.  VMA locking is complete horror without SRCU
> mess thrown in.  It's a bloody bad idea, at least without a very massive
> cleanup prior to that thing.
> 
> Start with drawing the call graph for vma-related code - at least the
> parts from relevant locks grabbed to accesses of fields protected by
> said locks.
  VMAs are protected by mmap_sem AFAIK so that doesn't look all that
complex. But I guess you are pointing at the fact that sometimes mmap_sem
is acquired rather far (sometimes even in arch code) from the places which
use the protection of mmap_sem and so it would be difficult (if possible at
all) to verify that once we drop mmap_sem, all these places will happily
handle that fact. I agree it would be a mess unless we somehow simplify
things first...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [LSF/MM TOPIC] mmap_sem in ->fault and ->page_mkwrite

[Michel Lespinasse]

On Mon, Feb 4, 2013 at 10:04 AM, Jan Kara <jack@suse.cz> wrote:
> On Thu 31-01-13 23:03:27, Al Viro wrote:
>> On Thu, Jan 31, 2013 at 11:23:35PM +0100, Jan Kara wrote:
>> >   Hi,
>> >
>> >   I'm not sure if this is such a great topic but it's a question which
>> > I came across a few times already and LSF/MM is a good place for
>> > brainstorming somewhat crazy ideas ;).
>> >
>> > So currently ->fault() and ->page_mkwrite() are called under mmap_sem held
>> > for reading. Now this creates sometimes unpleasant locking dependencies for
>> > filesystems (modern filesystems have to do an equivalent of ->write_begin
>> > in ->page_mkwrite and that is a non-trivial operation). Just to mention my
>> > last itch, I had to split reader side of filesystem freezing lock into two
>> > locks - one which ranks above mmap_sem and one which ranks below it. Then
>> > writer side has to wait for both locks. It works but ...
>> >
>> > So I was wondering: Would it be somehow possible we could drop mmap_sem in
>> > these two callbacks (especially ->page_mkwrite())? I understand process'
>> > mapping can change under us once we drop the semaphore so we'd have to
>> > somehow recheck we have still the right page after re-taking mmap_sem. Like
>> > if we protected VMAs with SRCU so that they don't disappear under us once
>> > we drop mmap_sem and after retaking mmap_sem we would recheck whether VMA
>> > still applies to our fault.

I'm not sure if there is enough interest for an MM topic there;
however I would like to at least discuss this privately with you - I
have a lot of mmap_sem frustrations too :)

>> > And I know there's VM_FAULT_RETRY but that really seems like a special hack
>> > for x86 architecture page fault code. Making it work for all architectures
>> > and callers such as get_user_pages() didn't really seem plausible to me.

There is really nothing x86 specific about FAULT_FLAG_ALLOW_RETRY -
upstream code already uses it (on all archs) to drop mmap_sem during
large mlocks that hit disk; and patches in -mm extend this to handle
MAP_POPULATE mmaps as well. Using it during page faults is currently
only done on x86, but doing that on other arch page fault handlers
wouldn't be hard - the code is easy to write, it's just a matter of
getting it tested on all archs.

This leaves the issue of all the other gup users. I don't think
dropping and regrabbing mmap_sem within gup is realistic in general,
as the call sites don't expect VMAs to change in the middle of the gup
call.

>> Please, *please*, don't.  VMA locking is complete horror without SRCU
>> mess thrown in.  It's a bloody bad idea, at least without a very massive
>> cleanup prior to that thing.
>>
>> Start with drawing the call graph for vma-related code - at least the
>> parts from relevant locks grabbed to accesses of fields protected by
>> said locks.
>   VMAs are protected by mmap_sem AFAIK so that doesn't look all that
> complex. But I guess you are pointing at the fact that sometimes mmap_sem
> is acquired rather far (sometimes even in arch code) from the places which
> use the protection of mmap_sem and so it would be difficult (if possible at
> all) to verify that once we drop mmap_sem, all these places will happily
> handle that fact. I agree it would be a mess unless we somehow simplify
> things first...

Yes.

FAULT_FLAG_ALLOW_RETRY is my attempt at giving a way for call sites
which can deal with mmap_sem being dropped to signal that, so that we
don't need to convert every call sites at once. But if you have a
better way to go about it, I would be open to discuss it :)

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.