Re: [PATCH v21 1/4] mm: add VM_DROPPABLE for designating always lazily freeable mappings

[David Hildenbrand <david@redhat.com>]

On 07.07.24 20:19, Linus Torvalds wrote:
> On Sun, 7 Jul 2024 at 00:42, David Hildenbrand <david@redhat.com> wrote:
>>
>> But I don't immediately see why MAP_WIPEONFORK and MAP_DONTDUMP have to
>> be mmap() flags.

Hi Linus,

> 
> I don't think they have to be mmap() flags, but that said, I think
> it's technically the better alternative than saying "you have to
> madvise things later".

Having the option to madvise usually means that you can toggle it on/off 
(e.g., MADV_DONTFORK vs. MADV_DOFORK). Not sure if having that option 
could be valuable here (droppable) as well; maybe not.

> 
> I very much understand the "we don't have a lot of MAP_xyz flags and
> we don't want to waste them" argument, but at the same time
> 
>   (a) we _do_ have those flags
> 
>   (b) picking a worse interface seems bad
> 
>   (c) we could actually use the PROT_xyz bits, which we have a ton of
> 

I recall that introducing things like MAP_SHARED_VALIDATE received a lot 
of pushback in the past. But that was before my MM days, and I only had 
people tell me stories about it.

(and at LSF/MM it's been a recurring theme that if you want to propose 
new MMAP flag, you're going to have a hard time)

> And yes, (c) is ugly, but is it uglier than "use two system calls to
> do one thing"? I mean, "flags" and "prot" are just two sides of the
> same coin in the end, the split is kind of arbitrary, and "prot" only
> has four bits right now, and one of them is historical and useless,
> and actually happens to be *exactly* this kind of MAP_xyz bit.

Yeah, I always had the same feeling about prot vs. flags.

My understanding so far was that we should have madvise() ways to toggle 
stuff and add mmap bits if not avoidable; at least that's what I learned 
from the community.

Good to hear that this is changing. (or it's just been an urban myth)


I'll use your mail as reference in the future when that topic pops up ;)


Maybe, historically we used madvise options so it's easier to sense 
which options the current kernel actually supports. (e.g., let mmap() 
succeed but let a separate madvise(MADV_HUGEPAGE) etc. fail if not 
supported by the kernel; no need to fail the whole operation).

> 
> (In case it's not clear, I'm talking about PROT_SEM, which is very
> much a behavioral bit for broken architectures that we've actually
> never implemented).

Yeah.

> 
> We also have PROT_GROSDOWN and PROT_GROWSUP , which is basically a
> "match MAP_GROWSxyz and change the mprotect() limits appropriately"

It's the first time I hear about these two mprotect() options, thanks 
for mentioning that :)

> 
> So I actually think we could use the PROT_xyz bits, and anybody who
> says "those are for PROT_READ and PROT_WRITE is already very very
> wrong.
> 
> Again - not pretty, but mappens to match reality.
> 
>> Interestingly, when looking into something comparable in the past I
>> stumbled over "vrange" [1], which would have had a slightly different
>> semantic (signal on reaccess).
> 
> We literally talked about exactly this with Jason, except unlike you I
> couldn't find the historical archive (I tried in vain to find
> something from lore).

Good that you discussed it, I primarily scanned this patch set here only.

I took notes back when I was looking for something like VM_DROPPABLE 
(also, being more interested in the non-signal version for a VM cache 
use case).

> 
>    https://lore.kernel.org/lkml/CAHk-=whRpLyY+U9mkKo8O=2_BXNk=7sjYeObzFr3fGi0KLjLJw@mail.gmail.com/
> 
> I do think that a "explicit populate and get a signal on access" is a
> very valid model, but I think the "zero on access" is a more
> immediately real model.
> 
> And we actually have had the "get signal on access" before: that's
> what VM_DONTCOPY is.
> 
> And it was the *less* useful model, which is why we added
> VM_WIPEONCOPY, because that's the semantics people actually wanted.
> 
> So I think the "signal on thrown out data access" is interesting, but
> not necessarily the *more* interesting case.

Absolutely agreed.

> 
> And I think if we do want that case, I think having MAP_DROPPABLE have
> those semantics for MAP_SHARED would be the way to go. IOW, starting
> off with the "zero on next access after drop" case doesn't make it any
> harder to then later add a "fault on next access after drop" version.
> 
>> There needs to be better reasoning why we have to consume three mmap
>> bits for something that can likely be achieved without any.
> 
> I think it goes the other way: why are MAP_xyz bits so precious to
> make this harder to actually use?

If things changed and we can have as many as we want, good!

Things like MADV_HUGEPAGE/MADV_MERGEABLE might benefit from a mmap flag 
as well.

-- 
Cheers,

David / dhildenb