On Mon, 27 Jul 2015, Vlastimil Babka wrote: > On 07/24/2015 11:28 PM, Eric B Munson wrote: > > ... > > >Changes from V4: > >Drop all architectures for new sys call entries except x86[_64] and MIPS > >Drop munlock2 and munlockall2 > >Make VM_LOCKONFAULT a modifier to VM_LOCKED only to simplify book keeping > >Adjust tests to match > > Hi, thanks for considering my suggestions. Well, I do hope there > were correct as API's are hard and I'm no API expert. But since > API's are also impossible to change after merging, I'm sorry but > I'll keep pestering for one last thing. Thanks again for persisting, > I do believe it's for the good thing! > > The thing is that I still don't like that one has to call > mlock2(MLOCK_LOCKED) to get the equivalent of the old mlock(). Why > is that flag needed? We have two modes of locking now, and v5 no > longer treats them separately in vma flags. But having two flags > gives us four possible combinations, so two of them would serve > nothing but to confuse the programmer IMHO. What will mlock2() > without flags do? What will mlock2(MLOCK_LOCKED | MLOCK_ONFAULT) do? > (Note I haven't studied the code yet, as having agreed on the API > should come first. But I did suggest documenting these things more > thoroughly too...) > OK I checked now and both cases above seem to return EINVAL. > > So about the only point I see in MLOCK_LOCKED flag is parity with > MAP_LOCKED for mmap(). But as Kirill said (and me before as well) > MAP_LOCKED is broken anyway so we shouldn't twist the rest just of > the API to keep the poor thing happier in its misery. > > Also note that AFAICS you don't have MCL_LOCKED for mlockall() so > there's no full parity anyway. But please don't fix that by adding > MCL_LOCKED :) > > Thanks! I have an MLOCK_LOCKED flag because I prefer an interface to be explicit. The caller of mlock2() will be required to fill in the flags argument regardless. I can drop the MLOCK_LOCKED flag with 0 being the value for LOCKED, but I thought it easier to make clear what was going on at any call to mlock2(). If user space defines a MLOCK_LOCKED that happens to be 0, I suppose that would be okay. We do actually have an MCL_LOCKED, we just call it MCL_CURRENT. Would you prefer that I match the name in mlock2() (add MLOCK_CURRENT instead)? Finally, on the question of MAP_LOCKONFAULT, do you just dislike MAP_LOCKED and do not want to see it extended, or is this a NAK on the set if that patch is included. I ask because I have to spin a V6 to get the MLOCK flag declarations right, but I would prefer not to do a V7+. If this is a NAK with, I can drop that patch and rework the tests to cover without the mmap flag. Otherwise I want to keep it, I have an internal user that would like to see it added.
Attachment:
signature.asc
Description: Digital signature