Re: [PATCH V3 8/9] iommufd: optimize file mapping

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Oct 18, 2024 at 02:10:26PM -0400, Steven Sistare wrote:

> > > > But why is that such a problem? The ufolios_offset already handles a
> > > > partial folio.
> > > > 
> > > > If the batch fills you leave ufolios_next alone and adjust
> > > > ufolios_offset. The next iteration does exactly the same as the first
> > > > iteration and takes the partial folio.
> > > > 
> > > > The refcount has to be handled carefully as the the portion that has
> > > > been put in the batch needs to be refcounted at 4k and the remainder
> > > > remain a single ref, but that just means adjusting the refcount after
> > > > putting it in the batch.
> > > > 
> > > > It seems like it just works naturally and avoids the double loop
> > > 
> > > Yes, I can squash the nested loop in batch_from_folios_huge.  (I was off
> > > point because I thought you were suggesting something else).
> > > 
> > > Do you want me to delete ufolios_huge and its special cases?
> > 
> > I think yes, if you can get to one loop then it would be a degradation
> > to sweep the folio list an extra time just to figure out ufolios_huge.
> 
> So now I'm confused again and don't know which "double loops" you want me
> to eliminate. There is still a loop in pin_memfd_pages.  It calls folio_add_pins,
> which I could push to batch_from_folios_huge, but it also counts and returns
> pages.

This is the one I was looking at, yes..

> It must do the count because the caller calls
> iopt_pages_add_npinned().

Ah, this is what I was trying to get to, if there was some reason why
this loop and this pages calculation was needed..

So, to fix that up you'd need to adjust the memfd_pin_folios() to also
return back the end va of what it returned - it calls it start_idx
internally. Then that can directly give the npages for add_npinned
with simple math and the loop in pin_memfd_pages can go away.

Then you have a single loop in the batch_from_folios(), which
is what I am thinking about.

What do you think?

Jason