On Fri, Mar 21, 2025 at 02:11:06PM -0400, Liam R. Howlett wrote: > * Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> [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
