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

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Fri, Mar 21, 2025 at 02:11:06PM -0400, Liam R. Howlett wrote:
> * 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.

I will look at actually analysing this stuff.

I absolutely HATE having vmg->prev, next etc.

Anyway tomorrow (Heathrow cooperating) we'll be co-located so maybe a light
night beer and discussion tomorrow night eh? ;)

>
> Thanks,
> Liam