Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/28/2015 03:49 PM, Eric B Munson wrote:
On Tue, 28 Jul 2015, Michal Hocko wrote:


[...]

The only
remaining question I have is should we have 2 new mlockall flags so that
the caller can explicitly set VM_LOCKONFAULT in the mm->def_flags vs
locking all current VMAs on fault.  I ask because if the user wants to
lock all current VMAs the old way, but all future VMAs on fault they
have to call mlockall() twice:

	mlockall(MCL_CURRENT);
	mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);

This has the side effect of converting all the current VMAs to
VM_LOCKONFAULT, but because they were all made present and locked in the
first call, this should not matter in most cases.

Shouldn't the user be able to do this?

mlockall(MCL_CURRENT)
mlockall(MCL_FUTURE | MCL_ONFAULT);

Note that the second call shouldn't change (i.e. munlock) existing vma's just because MCL_CURRENT is not present. The current implementation doesn't do that thanks to the following in do_mlockall():

        if (flags == MCL_FUTURE)
                goto out;

before current vma's are processed and MCL_CURRENT is checked. This is probably so that do_mlockall() can also handle the munlockall() syscall. So we should be careful not to break this, but otherwise there are no limitations by not having two MCL_ONFAULT flags. Having to do invoke syscalls instead of one is not an issue as this shouldn't be frequent syscall.

The catch is that,
like mmap(MAP_LOCKED), mlockall() does not communicate if mm_populate()
fails.  This has been true of mlockall() from the beginning so I don't
know if it needs more than an entry in the man page to clarify (which I
will add when I add documentation for MCL_ONFAULT).

Good point.

In a much less
likely corner case, it is not possible in the current setup to request
all current VMAs be VM_LOCKONFAULT and all future be VM_LOCKED.

So again this should work:

mlockall(MCL_CURRENT | MCL_ONFAULT)
mlockall(MCL_FUTURE);

But the order matters here, as current implementation of do_mlockall() will clear VM_LOCKED from def_flags if MCL_FUTURE is not passed. So *it's different* from how it handles MCL_CURRENT (as explained above). And not documented in manpage. Oh crap, this API is a closet full of skeletons. Maybe it was an unnoticed regression and we can restore some sanity?




[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux