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

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Oct 18, 2024 at 01:54:12PM -0400, Steven Sistare wrote:
> On 10/18/2024 12:04 PM, Jason Gunthorpe wrote:
> > On Fri, Oct 18, 2024 at 10:34:39AM -0400, Steven Sistare wrote:
> > > > Ie instead of things like:
> > > > 
> > > > 		batch_from_folios_huge(&pfns->batch, &user->ufolios_next,
> > > > 				       &user->ufolios_offset, npages);
> > > > 
> > > > Where npages is the total number of pages inside the folio list, it
> > > > would be nfolios.
> > > 
> > > The problem is that a batch may reach capacity after accepting only part
> > > of a folio, a condition I forced in testing by reducing array_size and
> > > MAX_NPFNS. We need to track which folio is partial and what is the next index
> > > to use in the folio.  That is ufolios_next and ufolios_offset.
> > 
> > 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.

Just check nr=1 in the batch_from_folios_huge body if that helps

Jason