Subject: Re: [RESEND PATCH V2 0/3] Allow user to request memory to be locked on page fault

From: Eric B Munson <[email-masked]>
Date: Thu, 11 Jun 2015 15:55:47 -0400

On 06/11/2015 03:34 PM, Andrew Morton wrote:
> On Thu, 11 Jun 2015 15:21:30 -0400 Eric B Munson
> <[email-masked]> wrote:
> 
>>> Ditto mlockall(MCL_ONFAULT) followed by munlock().  I'm not
>>> sure that even makes sense but the behaviour should be
>>> understood and tested.
>> 
>> I have extended the kselftest for lock-on-fault to try both of
>> these scenarios and they work as expected.  The VMA is split and
>> the VM flags are set appropriately for the resulting VMAs.
> 
> munlock() should do vma merging as well.  I *think* we implemented 
> that.  More tests for you to add ;)

I will add a test for this as well.  But the code is in place to merge
VMAs IIRC.

> 
> How are you testing the vma merging and splitting, btw?  Parsing 
> the profcs files?

To show the VMA split happened, I dropped a printk in mlock_fixup()
and the user space test simply checks that unlocked pages are not
marked as unevictable.  The test does not parse maps or smaps for
actual VMA layout.  Given that we want to check the merging of VMAs as
well I will add this.

> 
>>> What's missing here is a syscall to set VM_LOCKONFAULT on an 
>>> arbitrary range of memory - mlock() for lock-on-fault.  It's a 
>>> shame that mlock() didn't take a `mode' argument.  Perhaps we 
>>> should add such a syscall - that would make the mmap flag
>>> unneeded but I suppose it should be kept for symmetry.
>> 
>> Do you want such a system call as part of this set?  I would need
>> some time to make sure I had thought through all the possible
>> corners one could get into with such a call, so it would delay a
>> V3 quite a bit. Otherwise I can send a V3 out immediately.
> 
> I think the way to look at this is to pretend that mm/mlock.c
> doesn't exist and ask "how should we design these features".
> 
> And that would be:
> 
> - mmap() takes a `flags' argument: MAP_LOCKED|MAP_LOCKONFAULT.
> 
> - mlock() takes a `flags' argument.  Presently that's 
> MLOCK_LOCKED|MLOCK_LOCKONFAULT.
> 
> - munlock() takes a `flags' arument.
> MLOCK_LOCKED|MLOCK_LOCKONFAULT to specify which flags are being
> cleared.
> 
> - mlockall() and munlockall() ditto.
> 
> 
> IOW, LOCKED and LOCKEDONFAULT are treated identically and
> independently.
> 
> Now, that's how we would have designed all this on day one.  And I 
> think we can do this now, by adding new mlock2() and munlock2() 
> syscalls.  And we may as well deprecate the old mlock() and
> munlock(), not that this matters much.
> 
> *should* we do this?  I'm thinking "yes" - it's all pretty simple 
> boilerplate and wrappers and such, and it gets the interface
> correct, and extensible.
> 
> What do others think?
>