On Thursday, 27 January 2022 2:09:42 PM AEDT Alex Sierra wrote:
> Avoid long term pinning for Coherent device type pages. This could
> interfere with their own device memory manager. For now, we are just
> returning error for PIN_LONGTERM Coherent device type pages. Eventually,
> these type of pages will get migrated to system memory, once the device
> migration pages support is added.
>
> Signed-off-by: Alex Sierra
> ---
> mm/gup.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 886d6148d3d0..5291d7221826 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1720,6 +1720,12 @@ static long check_and_migrate_movable_pages(unsigned
> long nr_pages,
>* If we get a movable page, since we are going to be pinning
>* these entries, try to move them out if possible.
>*/
> + if (is_dev_private_or_coherent_page(head)) {
> + WARN_ON_ONCE(is_device_private_page(head));
> + ret = -EFAULT;
> + goto unpin_pages;
> + }
> +
> if (!is_pinnable_page(head)) {
> if (PageHuge(head)) {
> if (!isolate_huge_page(head,
> _page_list))
> @@ -1750,6 +1756,7 @@ static long check_and_migrate_movable_pages(unsigned
> long nr_pages,
> if (list_empty(_page_list) && !isolation_error_count)
> return nr_pages;
>
> +unpin_pages:
> if (gup_flags & FOLL_PIN) {
> unpin_user_pages(pages, nr_pages);
> } else {
If there is a mix of ZONE_MOVABLE and device pages the return value (ret) will
be subsequently lost here:
if (!list_empty(_page_list)) {
ret = migrate_pages(_page_list, alloc_migration_target,
NULL, (unsigned long), MIGRATE_SYNC,
MR_LONGTERM_PIN, NULL);
if (ret && !list_empty(_page_list))
putback_movable_pages(_page_list);
}
Which won't actually cause any problems, but it will lead to the GUP getting
retried unnecessarily. I do still intend to address this with a series to
migrate pages instead though, so I think this is ok for now as it's an unlikely
corner case anyway. Therefore feel tree to add the below when you repost:
Reviewed-by: Alistair Poppple