Re: [PATCH v15 3/9] mm/gup: Introduce memfd_pin_folios() for pinning memfd folios

[Oscar Salvador <osalvador@suse.de>]

On Thu, Jun 13, 2024 at 02:42:05PM -0700, Vivek Kasireddy wrote:
> For drivers that would like to longterm-pin the folios associated
> with a memfd, the memfd_pin_folios() API provides an option to
> not only pin the folios via FOLL_PIN but also to check and migrate
> them if they reside in movable zone or CMA block. This API
> currently works with memfds but it should work with any files
> that belong to either shmemfs or hugetlbfs. Files belonging to
> other filesystems are rejected for now.
> 
> The folios need to be located first before pinning them via FOLL_PIN.
> If they are found in the page cache, they can be immediately pinned.
> Otherwise, they need to be allocated using the filesystem specific
> APIs and then pinned.
> 
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Cc: Junxiao Chang <junxiao.chang@intel.com>
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> (v2)
> Reviewed-by: David Hildenbrand <david@redhat.com> (v3)
> Reviewed-by: Christoph Hellwig <hch@lst.de> (v6)
> Acked-by: Dave Airlie <airlied@redhat.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
...
> +struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
> +{
> +#ifdef CONFIG_HUGETLB_PAGE
> +	struct folio *folio;
> +	int err;
> +
> +	if (is_file_hugepages(memfd)) {
> +		folio = alloc_hugetlb_folio_nodemask(hstate_file(memfd),
> +						     NUMA_NO_NODE,
> +						     NULL,
> +						     GFP_USER,
> +						     false);

I dislike the direct use of GFP_USER there, because it opens the door for new
users to start passing their own GFP_ flags directly into hugetlb code, which
is not optimal, and something I would really like to prevent.

Hugetlb pages, until now, they have been mapped only to userspace and only used
in there, and they can or cannot be migrated based on its size, and that is why
we have 'htlb_alloc_mask'.

Now, you need something special because 1) these pages might need to be accessible by
some DMA driver, so you have HIGHMEM contraints and 2) cannot be migrated away.
Ok, but I see this as an exception, and it should really be called out
here.

 gfp_t = htlb_alloc_mask;

 /*
  * These pages will be accessible by a DMA driver, so we have zone memory
  * constraints where we can alloc from.
  * Also, these pages will be pinned for an undefinied amount of time, so do
  * not expect them to be able to be migrated away.
  */
 gfp &=  ~(__GFP_HIGHMEM | __GFP_MOVABLE)

So it is clear what is going on here.

-- 
Oscar Salvador
SUSE Labs