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
