Re: [PATCH v1 0/3] udmabuf: Add support for page migration out of movable zone or CMA

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Aug 22, 2023 at 05:36:56AM +0000, Kasireddy, Vivek wrote:
> Hi Jason,
> 
> > > This patch series adds support for migrating pages associated with
> > > a udmabuf out of the movable zone or CMA to avoid breaking features
> > > such as memory hotunplug.
> > >
> > > The first patch exports check_and_migrate_movable_pages() function
> > > out of GUP so that the udmabuf driver can leverage it for page
> > > migration that is done as part of the second patch. The last patch
> > > adds two new udmabuf selftests to verify data coherency after
> > > page migration.
> > 
> > Please don't do this. If you want to do what GUP does then call
> > GUP. udmabuf is not so special that it needs to open code its own
> > weird version of it.
> We can't call GUP directly as explained in the first patch of this series:
> "For drivers that would like to migrate pages out of the movable
> zone (or CMA) in order to pin them (longterm) for DMA, using
> check_and_migrate_movable_pages() directly provides a convenient
> option instead of duplicating similar checks (e.g, checking
> the folios for zone, hugetlb, etc) and calling migrate_pages()
> directly.
> 
> Ideally, a driver is expected to call pin_user_pages(FOLL_LONGTERM)
> to migrate and pin the pages for longterm DMA but there are
> situations where the GUP APIs cannot be used directly for
> various reasons (e.g, when the VMA or start addr cannot be
> easily determined but the relevant pages are available)."
> 
> Given the current (model and) UAPI (udmabuf_create), the userspace
> only shares (memfd, offset, size) values that we use to find the
> relevant pages and pin them (by doing get_page()). Since the goal
> is to also migrate these pages, I think we have the following options:

This seems like a bad choice of uAPI - we don't have any kernel
support for pinning from a memfd. If you want this then you have to
build this as generic code, not open code it into udmabuf.

> - Leverage check_and_migrate_movable_pages(); but this function
>   needs to be exported from GUP.

GUP has many behaviors, we keep adding more, these functions should
not leak out of the mm core code into drivers.
 
> - Iterate over all the pages (in udmabuf) to check for folio_is_longterm_pinnable()
>   and call migrate_pages() eventually. This requires changes only to
>   the udmabuf driver but we'd be duplicating much of the functionality
>   provided by check_and_migrate_movable_pages().

Definately not

> - Call pin_user_pages_fast(FOLL_LONGTERM) from udmabuf driver. In
>   order to do this, we have to first unpin all pages and iterate over all
>   the VMAs of the VMM to identify the Guest RAM VMA and then use
>   page_address_in_vma() to find the start addr of the ranges and then
>   call GUP. Although this approach is feasible, it feels a bit convoluted.

Userspace should have told the kernel where the memfd is mapped.
 
> - Add a new udmabuf UAPI to have userspace share (start addr, len) values
>   so that the udmabuf driver can directly call GUP APIs. But this means all
>   the current users of udmabuf such as Qemu, CrosVM, etc, need to be
>   updated to use the new UAPI. 

There you go
 
> - Add a new API to the backing store/allocator to longterm-pin the page.
>   For example, something along the lines of shmem_pin_mapping_page_longterm()
>   for shmem as suggested by Daniel. A similar one needs to be added for
>   hugetlbfs as well.

This may also be reasonable.

Jason