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

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Oct 02, 2024 at 01:26:32PM -0400, Steven Sistare wrote:
> On 10/1/2024 5:03 PM, Jason Gunthorpe wrote:
> > On Tue, Sep 24, 2024 at 08:05:37AM -0700, Steve Sistare wrote:
> > > @@ -622,6 +630,67 @@ static void batch_from_pages(struct pfn_batch *batch, struct page **pages,
> > >   			break;
> > >   }
> > > +static void batch_from_folios(struct pfn_batch *batch, struct folio **folios,
> > > +			      unsigned long offset, unsigned long npages)
> > > +{
> > > +	unsigned long nr, pfn, i = 0;
> > > +	struct folio *folio;
> > > +
> > > +	while (npages) {
> > > +		folio = folios[i++];
> > > +		nr = folio_nr_pages(folio) - offset;
> > > +		nr = min_t(unsigned long, nr, npages);
> > > +		pfn = page_to_pfn(folio_page(folio, offset));
> > > +		batch_add_pfn_num(batch, pfn, nr);
> > 
> > I think we technically need to cap nr here as well?
> 
> Do you mean cap nr at max-unsigned-int and loop over the folio?

The max that can be put into a batch entry

> > Why does this exist? I would expect you'd call memfd_pin_folios with
> > the correct offset so that the first folio was the right folio, this
> > is how the pin_user_pages path works?
> 
> We need folios_start for the case where the 'if' branch below is not taken,
> so the folio was pinned by a previous call to pfn_reader_fill_span,
> so the requested start_index for the current call does not necessarily
> fall in the first folio of user->ufolios.

But that should be handled by keeping track of where we are in the
array, not searching it again..


> > > +static void folios_unpin_partial(struct folio **folios, unsigned long offset,
> > > +				 unsigned long npages)
> > > +{
> > > +	unsigned long nr, j, i = 0;
> > > +	struct folio *folio;
> > > +
> > > +	while (npages) {
> > > +		folio = folios[i++];
> > > +		nr = folio_nr_pages(folio);
> > > +		if (offset == 0 && nr < npages) {
> > > +			unpin_folio(folio);
> > > +		} else {
> > > +			nr = min_t(unsigned long, npages, nr - offset);
> > > +			for (j = 0; j < nr; j++)
> > > +				unpin_user_page(folio_page(folio, offset + j));
> > 
> > Don't loop, we have unpin_user_pages() for this
> 
> Cannot call it, I do not have a pages[] array in hand.

Ugh, still, don't loop, this needs a new mm helper then

Jason