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

["Kasireddy, Vivek" <vivek.kasireddy@intel.com>]

Hi Oscar,

> 
> 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.
Sounds good; will send a v16 with your suggestions included.

Thanks,
Vivek

> 
> --
> Oscar Salvador
> SUSE Labs