Re: [PATCH v2 1/4] mm: Introduce vm_uffd_ops API

[Liam R. Howlett]

* Peter Xu <peterx@xxxxxxxxxx> [250919 10:17]:

...

> > > > Can you please point me to which patch has clean up?
> > > 
> > > Patch 4.  If you want me to explain every change I touched that is a
> > > cleanup, I can go into details.  Maybe it's faster if you read them, it's
> > > not a huge patch.
> > 
> > I responded here [1].  I actually put a lot of effort into that response
> > and took a lot of time to dig into some of this to figure out if it was
> > possible, and suggested some ideas.
> > 
> > That was back in July, so the details aren't that fresh anymore.  Maybe
> > you missed my reply?
> 
> AFAICT, we made it the other way round.  My reply is here:
> 
> https://lore.kernel.org/all/aMnAscxj_h42wOAC@x1.local/

Thanks, yes.  I missed your reply.

> > > 
> > > If we accepted ->fault() for all these years, I don't see a reason we
> > > should reject ->uffd_get_folio(), especially one of the goals is to keep
> > > the core mm path clean, per my comment to proposal (a).
> > 
> > I see this argument as saying "there's a hole in our boat so why can't I
> > make another?"  It's not the direction we have to go to get what we need
> > right now, so why are we doing it?  Like you said, it can be evaluated
> > later if things change..
> 
> You described ->fault() as "a hole in our boat"?  I'm astonished and do not
> know what to say on this.
> 
> There was a great comment saying one may want to make Linux an unikernel.
> I thought it was a good one, but only when it was a joke. Is it not?

Well it's leaking the internals, which is what we don't want to do.
Certainly it is useful and does what is needed.

> 
> > 
> > My thoughts were around an idea that we only really need to do a limited
> > number of operations on that pointer you are returning.  Those
> > operations may share code, and could be internal to mm.  I don't see
> > this as (a), (b), or (c), but maybe an addition to (b)?  Maybe we need
> > more ops to cover the uses?
> 
> That's exactly what this proposal is about, isn't it?  Userfaultfd minor
> fault shares almost all the code except the one hook fetching a folio from
> a page cache from the memory type.
> 
> "could be internal to mm" is (c) at least.  No one can do what you
> mentioned without moving guest-memfd into mm/ first.
> 
> Nikita and I drafted these changes, so likely we may likely have better
> idea what is happening.
> 
> Would you perhaps implement your idea, if that's better?  Either you're
> right, we're happy to use it.  Or you found what you're missing.
> 

I spoke to Mike on this and I understand what I was missing.  I'm fine
with the folio part as you have it.

Apologies for holding this up and the added stress on your side.

Thanks,
Liam