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

[David Hildenbrand <david@redhat.com>]

On 07.07.24 21:22, Linus Torvalds wrote:
> On Sun, 7 Jul 2024 at 11:52, David Hildenbrand <david@redhat.com> wrote:
>>
>> 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.
> 
> I think MAP_SHARED_VALIDATE was mostly about worrying about the API impact.
> 
> And I think it worked out so well that this is probably the first time
> it has been brought up ever since ;)
> 
> That said, the *reason* for MAP_SHARED_VALIDATE is actually very
> valid: we have historically just ignored any random flags in the
> mmap() interfaces, and with shared mappings, that can be dangerous.
> 
> IOW, the real issue wasn't MAP_SHARED_VALIDATE itself, but introducing
> *other* flags that affected maps that old kernels would ignore, and
> then the worry was "now old kernels and new kernels work very
> differently for this binary".
> 
> That's technically obviously true of any MAP_DROPPABLE thing too - old
> kernels would happily just ignore it. I suspect that's more of a
> feature than a mis-feature, but..
> 
>> 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.
> 
> It doesn't sound like a bad model in general. I'm not entirely sure it
> makes sense for something like "droppable", since that is a fairly
> fundamental behavioral thing. Does it make sense to make something
> undroppable when it can drop pages concurrently with that operation?
> 
> I mean, you can't switch MAP_SHARED around either.
> 
> The other bits already _do_ have madvise() things, and Jason added a
> way to just do it all in one go.

I just recalled that with MAP_HUGETLB, bits [26:31] encode a hugetlb
size (see include/uapi/asm-generic/hugetlb_encode.h). hugetlb, the gift
that keeps on giving.

We're using:

+#define MAP_WIPEONFORK		0x08000000	/* Zero memory in child forks. */
+#define MAP_DONTDUMP		0x10000000	/* Do not write to coredumps. */
+#define MAP_DROPPABLE		0x20000000	/* Zero memory under memory pressure. */

Which should be bit 27-29.

So using these flags with MAP_HUGETLB will result in surprises.

At least MAP_DROPPABLE doesn't quite make sense with hugetlb, but at least
the other ones do have semantics with hugetlb?

It's late Sunday here in Germany, so I might just have messed something up.

Just raising that there might be a "bit" conflict.

-- 
Cheers,

David / dhildenb