Re: [PATCH V3 4/9] iommufd: pfn reader for file mappings

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Oct 17, 2024 at 01:01:02PM -0400, Steven Sistare wrote:
> On 10/16/2024 8:37 AM, Jason Gunthorpe wrote:
> > On Fri, Oct 04, 2024 at 11:48:15AM -0700, Steve Sistare wrote:
> > 
> > > +static long pin_memfd_pages(struct pfn_reader_user *user, unsigned long start,
> > > +			    unsigned long npages)
> > > +{
> > > +	unsigned long end, nr, i, j, npin, offset, npages_out;
> > > +	long nfolios;
> > > +	int rc;
> > > +	struct folio *folio;
> > > +	struct page **upages = user->upages;
> > > +
> > > +	nfolios = user->ufolios_len / sizeof(*user->ufolios);
> > > +	end = start + (npages << PAGE_SHIFT) - 1;
> > > +
> > > +	nfolios = memfd_pin_folios(user->file, start, end, user->ufolios,
> > > +				   nfolios, &offset);
> > > +	if (nfolios <= 0)
> > > +		return nfolios;
> > > +
> > > +	offset >>= PAGE_SHIFT;
> > > +	npages_out = 0;
> > > +
> > > +	for (i = 0; i < nfolios; i++) {
> > > +		folio = user->ufolios[i];
> > > +		nr = folio_nr_pages(folio);
> > > +		npin = min(nr - offset, npages);
> > 
> > What happens if all the folios don't fit into the upages array once
> > expanded? It looks like they are leaked?
> > 
> > I suppose the later patch gets this all sane, but I wonder if we need
> > this inbetween step?
> 
> Yup, I missed the leak if a folio is only partially converted to pages.
> The optimization patch does get it right.  I'll squash them and reduce its
> size by making a separate patch for the new subroutines
> batch_add_pfn_num etc.

That sounds good!

Jason