On Tue, Feb 12, 2019 at 11:41:10AM -0700, Alex Williamson wrote: > Daniel Jordan <daniel.m.jordan@xxxxxxxxxx> wrote: > > On Mon, Feb 11, 2019 at 03:56:20PM -0700, Jason Gunthorpe wrote: > > > I haven't looked at this super closely, but how does this stuff work? > > > > > > do_mlock doesn't touch pinned_vm, and this doesn't touch locked_vm... > > > > > > Shouldn't all this be 'if (locked_vm + pinned_vm < RLIMIT_MEMLOCK)' ? > > > > > > Otherwise MEMLOCK is really doubled.. > > > > So this has been a problem for some time, but it's not as easy as adding them > > together, see [1][2] for a start. > > > > The locked_vm/pinned_vm issue definitely needs fixing, but all this series is > > trying to do is account to the right counter. Thanks for taking a look, Alex. > This still makes me nervous because we have userspace dependencies on > setting process locked memory. Could you please expand on this? Trying to get more context. > There's a user visible difference if we > account for them in the same bucket vs separate. Perhaps we're > counting in the wrong bucket now, but if we "fix" that and userspace > adapts, how do we ever go back to accounting both mlocked and pinned > memory combined against rlimit? Thanks, PeterZ posted an RFC that addresses this point[1]. It kept pinned_vm and locked_vm accounting separate, but allowed the two to be added safely to be compared against RLIMIT_MEMLOCK. Anyway, until some solution is agreed on, are there objections to converting locked_vm to an atomic, to avoid user-visible changes, instead of switching locked_vm users to pinned_vm? Daniel [1] http://lkml.kernel.org/r/20130524140114.GK23650@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
