[PATCH v3 0/1] mm: gup: avoid CMA page pinning by retrying migration

[PATCH v3 0/1] mm: gup: avoid CMA page pinning by retrying migration

[Hyesoo Yu]

This patch addresses an issue where longterm GUP requests
could inadvertently pin unpinnable pages (such as CMA) when no
migratable folios are found. This regression was introduced by
commit 1aaf8c122918 ("mm: gup: fix infinite loop within __get_longterm_locked").

This patch fixes the logic to return -EAGAIN when unpinnable
pages are detected but no folios could be collected, allowing
migration to be retried instead of incorrectly pinning the page.

This v3 drops the cleanup patch included in v2,
per Andrew Morton's request. It now contains only the regression fix.
The cleanup changes will be proposed separately for the 6.17-rc1.

Hyesoo Yu (1):
  mm: gup: avoid CMA page pinning by retrying migration if no migratable
    page

 mm/gup.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

-- 
2.49.0

[PATCH v3 1/1] mm: gup: avoid CMA page pinning by retrying migration if no migratable page

[Hyesoo Yu]

Commit 1aaf8c122918 ("mm: gup: fix infinite loop within __get_longterm_locked")
introduced an issue where CMA pages could be pinned by longterm GUP requests.
This occurs when unpinnable pages are detected but the movable_page_list is empty;
the commit would return success without retrying, allowing unpinnable
pages (such as CMA) to become pinned.

CMA pages may be temporarily off the LRU due to concurrent isolation,
for example when multiple longterm GUP requests are racing and therefore
not appear in movable_page_list. Before commit 1aaf8c, the kernel would
retry migration in such cases, which helped avoid accidental CMA pinning.

The original intent of the commit was to support longterm GUP on non-LRU
CMA pages in out-of-tree use cases such as pKVM. However, allowing this
can lead to broader CMA pinning issues.

To avoid this, the logic is restored to return -EAGAIN instead of success
when no folios could be collected but unpinnable pages were found.
This ensures that migration is retried until success, and avoids
inadvertently pinning unpinnable pages.

Fixes: 1aaf8c122918 ("mm: gup: fix infinite loop within __get_longterm_locked")
Acked-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>

---
We have confirmed that this regression causes CMA pages to be pinned
in our kernel 6.12-based environment.

In addition to CMA allocation failures, we also observed longterm GUP failures
when repeatedly accessing the same VMA. Specifically, the first longterm GUP
call would pin a CMA page, and a second call on the same region
would fail the migration because the cma page was already pinned.

After reverting commit 1aaf8c122918, the issue no longer reproduced.

Therefore, this fix is important to ensure reliable behavior of longterm GUP
and CMA-backed memory, and should be backported to stable.
---
 mm/gup.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index e065a49842a8..66193421c1d4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2300,14 +2300,12 @@ static void pofs_unpin(struct pages_or_folios *pofs)
 		unpin_user_pages(pofs->pages, pofs->nr_entries);
 }
 
-/*
- * Returns the number of collected folios. Return value is always >= 0.
- */
-static void collect_longterm_unpinnable_folios(
+static bool collect_longterm_unpinnable_folios(
 		struct list_head *movable_folio_list,
 		struct pages_or_folios *pofs)
 {
 	struct folio *prev_folio = NULL;
+	bool any_unpinnable = false;
 	bool drain_allow = true;
 	unsigned long i;
 
@@ -2321,6 +2319,8 @@ static void collect_longterm_unpinnable_folios(
 		if (folio_is_longterm_pinnable(folio))
 			continue;
 
+		any_unpinnable = true;
+
 		if (folio_is_device_coherent(folio))
 			continue;
 
@@ -2342,6 +2342,8 @@ static void collect_longterm_unpinnable_folios(
 				    NR_ISOLATED_ANON + folio_is_file_lru(folio),
 				    folio_nr_pages(folio));
 	}
+
+	return any_unpinnable;
 }
 
 /*
@@ -2417,11 +2419,25 @@ migrate_longterm_unpinnable_folios(struct list_head *movable_folio_list,
 static long
 check_and_migrate_movable_pages_or_folios(struct pages_or_folios *pofs)
 {
+	bool any_unpinnable;
+
 	LIST_HEAD(movable_folio_list);
 
-	collect_longterm_unpinnable_folios(&movable_folio_list, pofs);
-	if (list_empty(&movable_folio_list))
+	any_unpinnable = collect_longterm_unpinnable_folios(&movable_folio_list, pofs);
+
+	if (list_empty(&movable_folio_list)) {
+		/*
+		 * If we find any longterm unpinnable page that we failed to
+		 * isolated for migration, it might be because someone else
+		 * concurrently isolated it. Make the caller retry until it
+		 * succeeds.
+		 */
+		if (any_unpinnable) {
+			pofs_unpin(pofs);
+			return -EAGAIN;
+		}
 		return 0;
+	}
 
 	return migrate_longterm_unpinnable_folios(&movable_folio_list, pofs);
 }
-- 
2.49.0

Re: [PATCH v3 1/1] mm: gup: avoid CMA page pinning by retrying migration if no migratable page

[Hyesoo Yu]

[-- Attachment #1: Type: text/plain, Size: 4779 bytes --]

On Thu, Jun 05, 2025 at 05:04:31PM +0900, Hyesoo Yu wrote:
> Commit 1aaf8c122918 ("mm: gup: fix infinite loop within __get_longterm_locked")
> introduced an issue where CMA pages could be pinned by longterm GUP requests.
> This occurs when unpinnable pages are detected but the movable_page_list is empty;
> the commit would return success without retrying, allowing unpinnable
> pages (such as CMA) to become pinned.
> 
> CMA pages may be temporarily off the LRU due to concurrent isolation,
> for example when multiple longterm GUP requests are racing and therefore
> not appear in movable_page_list. Before commit 1aaf8c, the kernel would
> retry migration in such cases, which helped avoid accidental CMA pinning.
> 
> The original intent of the commit was to support longterm GUP on non-LRU
> CMA pages in out-of-tree use cases such as pKVM. However, allowing this
> can lead to broader CMA pinning issues.
> 
> To avoid this, the logic is restored to return -EAGAIN instead of success
> when no folios could be collected but unpinnable pages were found.
> This ensures that migration is retried until success, and avoids
> inadvertently pinning unpinnable pages.
> 
> Fixes: 1aaf8c122918 ("mm: gup: fix infinite loop within __get_longterm_locked")
> Acked-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
> 
> ---
> We have confirmed that this regression causes CMA pages to be pinned
> in our kernel 6.12-based environment.
> 
> In addition to CMA allocation failures, we also observed longterm GUP failures
> when repeatedly accessing the same VMA. Specifically, the first longterm GUP
> call would pin a CMA page, and a second call on the same region
> would fail the migration because the cma page was already pinned.
> 
> After reverting commit 1aaf8c122918, the issue no longer reproduced.
> 
> Therefore, this fix is important to ensure reliable behavior of longterm GUP
> and CMA-backed memory, and should be backported to stable.
> ---
>  mm/gup.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index e065a49842a8..66193421c1d4 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2300,14 +2300,12 @@ static void pofs_unpin(struct pages_or_folios *pofs)
>  		unpin_user_pages(pofs->pages, pofs->nr_entries);
>  }
>  
> -/*
> - * Returns the number of collected folios. Return value is always >= 0.
> - */
> -static void collect_longterm_unpinnable_folios(
> +static bool collect_longterm_unpinnable_folios(
>  		struct list_head *movable_folio_list,
>  		struct pages_or_folios *pofs)
>  {
>  	struct folio *prev_folio = NULL;
> +	bool any_unpinnable = false;
>  	bool drain_allow = true;
>  	unsigned long i;
>  
> @@ -2321,6 +2319,8 @@ static void collect_longterm_unpinnable_folios(
>  		if (folio_is_longterm_pinnable(folio))
>  			continue;
>  
> +		any_unpinnable = true;
> +
>  		if (folio_is_device_coherent(folio))
>  			continue;
>
> @@ -2342,6 +2342,8 @@ static void collect_longterm_unpinnable_folios(
>  				    NR_ISOLATED_ANON + folio_is_file_lru(folio),
>  				    folio_nr_pages(folio));
>  	}
> +
> +	return any_unpinnable;
>  }
>  
>  /*
> @@ -2417,11 +2419,25 @@ migrate_longterm_unpinnable_folios(struct list_head *movable_folio_list,
>  static long
>  check_and_migrate_movable_pages_or_folios(struct pages_or_folios *pofs)
>  {
> +	bool any_unpinnable;
> +
>  	LIST_HEAD(movable_folio_list);
>  
> -	collect_longterm_unpinnable_folios(&movable_folio_list, pofs);
> -	if (list_empty(&movable_folio_list))
> +	any_unpinnable = collect_longterm_unpinnable_folios(&movable_folio_list, pofs);
> +

Hi David,

While re-reviewing the v3 patch, I realized that migrate_longterm_unpinnable_folios()
should always be called whenever unpinnable folios are present, regardless of whether
the movable_folio_list is empty.

In collect_longterm_unpinnable_folios(), if folio_is_device_coherent() is true,
the folio is not added to movable_folio_list. However, such device-coherent folios
still need to be migrated later in migrate_longterm_unpinnable_folios().

I think the condition `list_empty(&movable_folio_list)` should be removed,
and it might be better to revert commit 1aaf8c122918 rather than adding a new patch.

What do you think?

Thanks,
Regards.

> +	if (list_empty(&movable_folio_list)) {
> +		/*
> +		 * If we find any longterm unpinnable page that we failed to
> +		 * isolated for migration, it might be because someone else
> +		 * concurrently isolated it. Make the caller retry until it
> +		 * succeeds.
> +		 */
> +		if (any_unpinnable) {
> +			pofs_unpin(pofs);
> +			return -EAGAIN;
> +		}
>  		return 0;
> +	}
>  
>  	return migrate_longterm_unpinnable_folios(&movable_folio_list, pofs);
>  }
> -- 
> 2.49.0
> 
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 1/1] mm: gup: avoid CMA page pinning by retrying migration if no migratable page

[David Hildenbrand]

On 05.06.25 10:39, Hyesoo Yu wrote:
> On Thu, Jun 05, 2025 at 05:04:31PM +0900, Hyesoo Yu wrote:
>> Commit 1aaf8c122918 ("mm: gup: fix infinite loop within __get_longterm_locked")
>> introduced an issue where CMA pages could be pinned by longterm GUP requests.
>> This occurs when unpinnable pages are detected but the movable_page_list is empty;
>> the commit would return success without retrying, allowing unpinnable
>> pages (such as CMA) to become pinned.
>>
>> CMA pages may be temporarily off the LRU due to concurrent isolation,
>> for example when multiple longterm GUP requests are racing and therefore
>> not appear in movable_page_list. Before commit 1aaf8c, the kernel would
>> retry migration in such cases, which helped avoid accidental CMA pinning.
>>
>> The original intent of the commit was to support longterm GUP on non-LRU
>> CMA pages in out-of-tree use cases such as pKVM. However, allowing this
>> can lead to broader CMA pinning issues.
>>
>> To avoid this, the logic is restored to return -EAGAIN instead of success
>> when no folios could be collected but unpinnable pages were found.
>> This ensures that migration is retried until success, and avoids
>> inadvertently pinning unpinnable pages.
>>
>> Fixes: 1aaf8c122918 ("mm: gup: fix infinite loop within __get_longterm_locked")
>> Acked-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
>>
>> ---
>> We have confirmed that this regression causes CMA pages to be pinned
>> in our kernel 6.12-based environment.
>>
>> In addition to CMA allocation failures, we also observed longterm GUP failures
>> when repeatedly accessing the same VMA. Specifically, the first longterm GUP
>> call would pin a CMA page, and a second call on the same region
>> would fail the migration because the cma page was already pinned.
>>
>> After reverting commit 1aaf8c122918, the issue no longer reproduced.
>>
>> Therefore, this fix is important to ensure reliable behavior of longterm GUP
>> and CMA-backed memory, and should be backported to stable.
>> ---
>>   mm/gup.c | 28 ++++++++++++++++++++++------
>>   1 file changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/gup.c b/mm/gup.c
>> index e065a49842a8..66193421c1d4 100644
>> --- a/mm/gup.c
>> +++ b/mm/gup.c
>> @@ -2300,14 +2300,12 @@ static void pofs_unpin(struct pages_or_folios *pofs)
>>   		unpin_user_pages(pofs->pages, pofs->nr_entries);
>>   }
>>   
>> -/*
>> - * Returns the number of collected folios. Return value is always >= 0.
>> - */
>> -static void collect_longterm_unpinnable_folios(
>> +static bool collect_longterm_unpinnable_folios(
>>   		struct list_head *movable_folio_list,
>>   		struct pages_or_folios *pofs)
>>   {
>>   	struct folio *prev_folio = NULL;
>> +	bool any_unpinnable = false;
>>   	bool drain_allow = true;
>>   	unsigned long i;
>>   
>> @@ -2321,6 +2319,8 @@ static void collect_longterm_unpinnable_folios(
>>   		if (folio_is_longterm_pinnable(folio))
>>   			continue;
>>   
>> +		any_unpinnable = true;
>> +
>>   		if (folio_is_device_coherent(folio))
>>   			continue;
>>
>> @@ -2342,6 +2342,8 @@ static void collect_longterm_unpinnable_folios(
>>   				    NR_ISOLATED_ANON + folio_is_file_lru(folio),
>>   				    folio_nr_pages(folio));
>>   	}
>> +
>> +	return any_unpinnable;
>>   }
>>   
>>   /*
>> @@ -2417,11 +2419,25 @@ migrate_longterm_unpinnable_folios(struct list_head *movable_folio_list,
>>   static long
>>   check_and_migrate_movable_pages_or_folios(struct pages_or_folios *pofs)
>>   {
>> +	bool any_unpinnable;
>> +
>>   	LIST_HEAD(movable_folio_list);
>>   
>> -	collect_longterm_unpinnable_folios(&movable_folio_list, pofs);
>> -	if (list_empty(&movable_folio_list))
>> +	any_unpinnable = collect_longterm_unpinnable_folios(&movable_folio_list, pofs);
>> +
> 
> Hi David,
> 
> While re-reviewing the v3 patch, I realized that migrate_longterm_unpinnable_folios()
> should always be called whenever unpinnable folios are present, regardless of whether
> the movable_folio_list is empty.
 > > In collect_longterm_unpinnable_folios(), if 
folio_is_device_coherent() is true,
> the folio is not added to movable_folio_list. However, such device-coherent folios
> still need to be migrated later in migrate_longterm_unpinnable_folios().

Ohh, because we cannot isolate them ... and they are always 
longterm-unpinnable.

> 
> I think the condition `list_empty(&movable_folio_list)` should be removed,
> and it might be better to revert commit 1aaf8c122918 rather than adding a new patch.
> 
> What do you think?

Yeah, with that in mind, a revert might indeed be the better option.

-- 
Cheers,

David / dhildenb