On Fri, 12 Jun 2015, Vlastimil Babka wrote: > On 06/11/2015 09:34 PM, Andrew Morton wrote: > >On Thu, 11 Jun 2015 15:21:30 -0400 Eric B Munson <emunson@xxxxxxxxxx> 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 ;) > > > >How are you testing the vma merging and splitting, btw? Parsing > >the profcs files? > > > >>>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. > > Note that the semantic of MAP_LOCKED can be subtly surprising: > > "mlock(2) fails if the memory range cannot get populated to guarantee > that no future major faults will happen on the range. > mmap(MAP_LOCKED) on the other hand silently succeeds even if the > range was populated only > partially." > > ( from http://marc.info/?l=linux-mm&m=143152790412727&w=2 ) > > So MAP_LOCKED can silently behave like MAP_LOCKONFAULT. While > MAP_LOCKONFAULT doesn't suffer from such problem, I wonder if that's > sufficient reason not to extend mmap by new mlock() flags that can > be instead applied to the VMA after mmapping, using the proposed > mlock2() with flags. So I think instead we could deprecate > MAP_LOCKED more prominently. I doubt the overhead of calling the > extra syscall matters here? We could talk about retiring the MAP_LOCKED flag but I suspect that would get significantly more pushback than adding a new mmap flag. Likely that the overhead does not matter in most cases, but presumably there are cases where it does (as we have a MAP_LOCKED flag today). Even with the proposed new system calls I think we should have the MAP_LOCKONFAULT for parity with MAP_LOCKED. > > >- 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. > > If the new LOCKONFAULT functionality is indeed desired (I haven't > still decided myself) then I agree that would be the cleanest way. Do you disagree with the use cases I have listed or do you think there is a better way of addressing those cases? > > >What do others think?
Attachment:
signature.asc
Description: Digital signature