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

[Jason Gunthorpe <jgg@nvidia.com>]

On Sat, Sep 14, 2024 at 06:05:53AM -0700, Steve Sistare wrote:
> Extend pfn_reader_user to pin file mappings, by calling memfd_pin_folios.
> Repin at small page granularity and unpack pages into upages[] to mesh
> with the existing code paths.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  drivers/iommu/iommufd/io_pagetable.h |  5 ++++
>  drivers/iommu/iommufd/pages.c        | 58 ++++++++++++++++++++++++++++++++----
>  2 files changed, 57 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/iommufd/io_pagetable.h b/drivers/iommu/iommufd/io_pagetable.h
> index 7c4a338..3a28f46 100644
> --- a/drivers/iommu/iommufd/io_pagetable.h
> +++ b/drivers/iommu/iommufd/io_pagetable.h
> @@ -177,6 +177,7 @@ enum {
>  
>  enum iopt_address_type {
>  	IOPT_ADDRESS_USER = 0,
> +	IOPT_ADDRESS_FILE = 1,
>  };
>  
>  /*
> @@ -202,6 +203,10 @@ struct iopt_pages {
>  	enum iopt_address_type type;
>  	union {
>  		void __user *uptr;		/* IOPT_ADDRESS_USER */
> +		struct {			/* IOPT_ADDRESS_FILE */
> +			struct file *file;
> +			unsigned long start;
> +		};
>  	};
>  	bool writable:1;
>  	u8 account_mode;
> diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c
> index 69822d4..065c28f 100644
> --- a/drivers/iommu/iommufd/pages.c
> +++ b/drivers/iommu/iommufd/pages.c
> @@ -695,6 +695,7 @@ static unsigned long batch_rw(struct pfn_batch *batch, void *data,
>  struct pfn_reader_user {
>  	struct page **upages;
>  	size_t upages_len;
> +	struct file *file;
>  	unsigned long upages_start;
>  	unsigned long upages_end;
>  	unsigned int gup_flags;
> @@ -712,6 +713,7 @@ static void pfn_reader_user_init(struct pfn_reader_user *user,
>  	user->upages_start = 0;
>  	user->upages_end = 0;
>  	user->locked = -1;
> +	user->file = (pages->type == IOPT_ADDRESS_FILE) ? pages->file : NULL;
>  
>  	user->gup_flags = FOLL_LONGTERM;
>  	if (pages->writable)
> @@ -733,13 +735,54 @@ static void pfn_reader_user_destroy(struct pfn_reader_user *user,
>  	user->upages = NULL;
>  }
>  
> +static long pin_memfd_pages(struct pfn_reader_user *user,
> +			    unsigned long start,
> +			    unsigned long npages)
> +{
> +	unsigned long end, nr, i, j, k, npin, nfolios, pgoff, max_folios;
> +	unsigned long npages_orig = npages;
> +	struct folio *folio;
> +	size_t size = npages * sizeof(folio);
> +	struct folio **folios = temp_kmalloc(&size, NULL, 0);
> +
> +	if (!folios)
> +		return -ENOMEM;
> +
> +	k = 0;
> +	max_folios = size / sizeof(folio);
> +	end = start + (npages << PAGE_SHIFT) - 1;
> +
> +	while (npages > 0) {
> +		nfolios = memfd_pin_folios(user->file, start, end,
> +					   folios, max_folios, &pgoff);
> +		if (nfolios <= 0)
> +			return nfolios;
> +
> +		pgoff >>= PAGE_SHIFT;
> +		for (i = 0; i < nfolios; i++) {
> +			folio = folios[i];
> +			nr = folio_nr_pages(folio);
> +			npin = min(nr - pgoff, npages);
> +			repin_folio_unhugely(folio, npin);
> +			for (j = pgoff; j < pgoff + npin; j++)
> +				user->upages[k++] = folio_page(folio, j);
> +			npages -= npin;
> +			start += npin << PAGE_SHIFT;
> +			pgoff = 0;

You should try harder to avoid this loop, the batch already can just
swallow a full folio, so this would be much better to copy more of the
code from it's caller and be more fully stand alone. Use only a temp
folio array and stick that directly into the batch full folio at a
time.

This above will be functionally the same, but alot slower..

Jason