lkml.org 
[lkml]   [2022]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] mm: madvise: MADV_DONTNEED_LOCKED
[please CC linux-api if you are going to repost with the fix suggested
by Nadav]

On Thu 03-03-22 16:47:34, Johannes Weiner wrote:
> On Thu, Mar 03, 2022 at 04:29:56PM -0500, Johannes Weiner wrote:
> > MADV_DONTNEED historically rejects mlocked ranges, but with
> > MLOCK_ONFAULT and MCL_ONFAULT allowing to mlock without populating,
> > there are valid use cases for depopulating locked ranges as well.
> >
> > Users mlock memory to protect secrets. There are allocators for secure
> > buffers that want in-use memory generally mlocked, but cleared and
> > invalidated memory to give up the physical pages. This could be done
> > with explicit munlock -> mlock calls on free -> alloc of course, but
> > that adds two unnecessary syscalls, heavy mmap_sem write locks, vma
> > splits and re-merges - only to get rid of the backing pages.
> >
> > Users also mlockall(MCL_ONFAULT) to suppress sustained paging, but are
> > okay with on-demand initial population. It seems valid to selectively
> > free some memory during the lifetime of such a process, without having
> > to mess with its overall policy.
> >
> > Why add a separate flag? Isn't this a pretty niche usecase?
> >
> > - MADV_DONTNEED has been bailing on locked vmas forever. It's at least
> > conceivable that someone, somewhere is relying on mlock to protect
> > data from perhaps broader invalidation calls. Changing this behavior
> > now could lead to quiet data corruption.
> >
> > - It also clarifies expectations around MADV_FREE and maybe
> > MADV_REMOVE. It avoids the situation where one quietly behaves
> > different than the others. MADV_FREE_LOCKED can be added later.
> >
> > - The combination of mlock() and madvise() in the first place is
> > probably niche. But where it happens, I'd say that dropping pages
> > from a locked region once they don't contain secrets or won't page
> > anymore is much saner than relying on mlock to protect memory from
> > speculative or errant invalidation calls. It's just that we can't
> > change the default behavior because of the two previous points.
> >
> > Given that, an explicit new flag seems to make the most sense.
> >
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
>
> Just for context, I found this discussion back from 2018:
>
> https://lkml.iu.edu/hypermail/linux/kernel/1806.1/00483.html
>
> It seems to me that the usecase wasn't really in question, but people
> weren't sure about the API, and then Jason found a workaround before
> the discussion really concluded. I was asked internally about this
> feature, so I'm submitting another patch in this direction, but with
> more thoughts on why I chose to go with a new flag. Hopefully we can
> work it out this time around :-)

Thanks for the link. The topic sounded familiar but I couldn't really
remember any details anymore. Now I do remember that I wasn't happy
about special casing MLOCK_ONFAULT. A dedicated madvise operation
is definitely safer and I am OK with that. Presented usecases make sense
to me as well.

Btw. I have a recollection that Mike is working on MADV_DONTNEED support
for hugetlb pages. I do not know the current state of that work. Not
that it would make nay impact on your new flag but some minor changes
might be needed.

Anyway, after the madvise_need_mmap_write is addressed, feel free to add
Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!
--
Michal Hocko
SUSE Labs

\
 
 \ /
  Last update: 2022-03-04 10:13    [W:0.120 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site