Re: [PATCH v4 03/10] mm/gup: fail get_user_pages for LONGTERM dev coherent type

2022-01-28 Thread Alistair Popple
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 





[PATCH v4 03/10] mm/gup: fail get_user_pages for LONGTERM dev coherent type

2022-01-26 Thread Alex Sierra
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 {
-- 
2.32.0