Sorry Jason, I didn't see your previous reply before sending this. Based
on that I assume your preference is to have
migrate_longterm_pinnable_pages() return -EAGAIN directly. Happy to
respin with the below changes if that's the case.
Alistair Popple <apopple@xxxxxxxxxx> writes:
[...]
> +/*
> + * Unpins all pages and migrates device coherent pages and movable_page_list.
> + * Returns zero if all pages were successfully migrated or -errno for failure
Returns -EAGAIN if all pages were successfully migrated or -errno for failure
> + * (or partial success).
> + */
> +static int migrate_longterm_unpinnable_pages(
> + struct list_head *movable_page_list,
> + unsigned long nr_pages,
> + struct page **pages)
> +{
> + int ret;
> + unsigned long i;
>
> -unpin_pages:
> - /*
> - * pages[i] might be NULL if any device coherent pages were found.
> - */
> for (i = 0; i < nr_pages; i++) {
> - if (!pages[i])
> + struct folio *folio = page_folio(pages[i]);
> +
> + if (folio_is_device_coherent(folio)) {
> + /*
> + * Migration will fail if the page is pinned, so convert
> + * the pin on the source page to a normal reference.
> + */
> + pages[i] = NULL;
> + folio_get(folio);
> + gup_put_folio(folio, 1, FOLL_PIN);
> +
> + if (migrate_device_coherent_page(&folio->page)) {
> + ret = -EBUSY;
> + goto err;
> + }
> +
> continue;
> + }
>
> + /*
> + * We can't migrate pages with unexpected references, so drop
> + * the reference obtained by __get_user_pages_locked().
> + * Migrating pages have been added to movable_page_list after
> + * calling folio_isolate_lru() which takes a reference so the
> + * page won't be freed if it's migrating.
> + */
> unpin_user_page(pages[i]);
> + pages[i] = NULL;
> }
>
> - if (!list_empty(&movable_page_list)) {
> + if (!list_empty(movable_page_list)) {
> struct migration_target_control mtc = {
> .nid = NUMA_NO_NODE,
> .gfp_mask = GFP_USER | __GFP_NOWARN,
> };
>
> - ret = migrate_pages(&movable_page_list, alloc_migration_target,
> - NULL, (unsigned long)&mtc, MIGRATE_SYNC,
> - MR_LONGTERM_PIN, NULL);
> - if (ret > 0) /* number of pages not migrated */
> + if (migrate_pages(movable_page_list, alloc_migration_target,
> + NULL, (unsigned long)&mtc, MIGRATE_SYNC,
> + MR_LONGTERM_PIN, NULL)) {
> ret = -ENOMEM;
> + goto err;
> + }
> }
>
> - if (ret && !list_empty(&movable_page_list))
> - putback_movable_pages(&movable_page_list);
> + putback_movable_pages(movable_page_list);
> +
> + return 0;
return -EAGAIN;
> +
> +err:
> + for (i = 0; i < nr_pages; i++)
> + if (pages[i])
> + unpin_user_page(pages[i]);
> + putback_movable_pages(movable_page_list);
> +
> return ret;
> }
> +
> +/*
> + * Check whether all pages are *allowed* to be pinned. Rather confusingly, all
> + * pages in the range are required to be pinned via FOLL_PIN, before calling
> + * this routine.
> + *
> + * If any pages in the range are not allowed to be pinned, then this routine
> + * will migrate those pages away, unpin all the pages in the range and return
> + * -EAGAIN. The caller should re-pin the entire range with FOLL_PIN and then
> + * call this routine again.
> + *
> + * If an error other than -EAGAIN occurs, this indicates a migration failure.
> + * The caller should give up, and propagate the error back up the call stack.
> + *
> + * If everything is OK and all pages in the range are allowed to be pinned, then
> + * this routine leaves all pages pinned and returns zero for success.
> + */
> +static long check_and_migrate_movable_pages(unsigned long nr_pages,
> + struct page **pages)
> +{
> + int ret;
> + unsigned long collected;
> + LIST_HEAD(movable_page_list);
> +
> + collected = collect_longterm_unpinnable_pages(&movable_page_list,
> + nr_pages, pages);
> + if (!collected)
> + return 0;
> +
> + ret = migrate_longterm_unpinnable_pages(&movable_page_list, nr_pages,
> + pages);
return migrate_longterm_unpinnable_pages(&movable_page_list, nr_pages,
pages);
- Alistair
