Re: [PATCH 6.15] mm/vma: add give_up_on_oom option on modify/merge, use in uffd release

Liam R. Howlett
2025-03-21T18:11:40Z

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250321 13:16]:
> On Fri, Mar 21, 2025 at 11:27:34AM -0400, Liam R. Howlett wrote:
> > +cc Peter due to uffd interests
>
> Gentle nudge for Peter to make himself uffd maintainer :) I am not a fan of
> this 'happen to know person X often touches Y' stuff, this is what
> MAINTAINERS is for. If you're not there, good chance I won't cc you...
>
> I also strongly feel we need somebody to take overall responsibility for
> uffd at this point.

Yes, uffd isn't well represented today and Peter seems to be doing the
work of R:, if not M:.

...

>
> >
> > We are essentially avoiding the compiler from catching the error for us
> > by returning that ERR_PTR(), which (keeping with the theme of my email)
> > I hate. It's fine for little functions but we've made a mess of it too
> > often.
> >
> > Reality will probably not align with the realistic view and people will
> > just copy/paste from wherever they saw it called... so we should think
> > twice about the failure scenarios on code review and I think a flag
> > (or a function name change?) might make this more obvious.
>
> OK so what I think we have have is a break in abstraction, where we are
> trying to do an 'iteration through VMAs where it's convenient to keep track
> of prev' and then people duplicating the code, making subtly false
> assumptions (yes pointer being returned with the obnoxious ERR_PTR() stuff
> possible and -god knows what happens to the state if not present-) and
> etc. etc.
>
> Don't we just need a new kind of vma iterator that handles the prev bits
> for us that can just do away with this crap?

I've been thinking about the iterator and the prev/next stuff for a
while.

I am not entirely sure on pulling it into the iterator. My hesitation
is that a lot of the time we don't really care about prev, except
merging. Merging only matters if the vma is touching the start of the
vma being modified, and if that's the case then we are very likely to be
in the same maple tree node and the previous slot. This should be in
the cpu cache, almost always.

So I'm wondering if we want to have an iterator do some fancy "this is
prev" or just ask "what's the previous slot?" - aka mas_peek_prev() or
something (that doesn't exist today).

We also have the users of contiguous iterations which wants to fail if
there is a gap anywhere before the end, and detect that error after the
iterator too.. ie, did we reach the end (or is the end a gap?), or did
we find an intermediate gap?

So there are a few common scenarios, and maybe we are getting to the
point of having a clear view of specific users for each that would
result in less bugs with common patterns?

The patters are also not entirely clear as the regular number of vmas
iterated - many are written this way to work on many vmas, but in
practice there is only one vma checked the majority of the time. So we
may be over-complicating things by keeping prev around and up to date in
the first place. Perhaps clever iterator coding could avoid this as
well.

Thanks,
Liam