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")
Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
---
mm/gup.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index 68d91b000199..a25c8c894882 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2300,15 +2300,13 @@ 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 drain_allow = true;
+ bool any_unpinnable = false;
unsigned long i;
for (i = 0; i < pofs->nr_entries; 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;
}
/*
@@ -2407,11 +2409,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.0On 05.06.25 05:32, 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.
>
The fix should always come before the cleanup. But Andrew already asked
for a separate submission, so that will be taken care of.
> Fixes: 1aaf8c122918 ("mm: gup: fix infinite loop within __get_longterm_locked")
> Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
> ---
> mm/gup.c | 28 ++++++++++++++++++++++------
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 68d91b000199..a25c8c894882 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2300,15 +2300,13 @@ 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 drain_allow = true;
> + bool any_unpinnable = false;
bool any_unpinnable = false;
bool drain_allow = true;
(maintain reverse xmas tree)
Apart from that LGTM
Acked-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
On Thu, 5 Jun 2025 12:32:07 +0900 Hyesoo Yu <hyesoo.yu@samsung.com> 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")
v6.14.
As ever, a question is "should we backport this fix". To answer that
we should understand the effect the regression has upon our users.
Readers can guess, but it's better if you tell us this, please?
On Wed, Jun 04, 2025 at 08:43:23PM -0700, Andrew Morton wrote:
> On Thu, 5 Jun 2025 12:32:07 +0900 Hyesoo Yu <hyesoo.yu@samsung.com> 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")
>
> v6.14.
>
> As ever, a question is "should we backport this fix". To answer that
> we should understand the effect the regression has upon our users.
> Readers can guess, but it's better if you tell us this, please?
>
Hi Andrew.
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 GUP longterm
failures in cases where the same VMA was accessed repeatedly.
Specifically, the first GUP longterm call would pin a CMA page, and a second
call on the same region would fail the migration due to the cma page already
being pinned.
After reverting commit 1aaf8c122918, the issue no longer reproduced.
Therefore, this fix is important to ensure reliable behavior of GUP longterm
and CMA-backed memory, and should be backported to stable.
Thanks,
Regards.
>
>
On Thu, 5 Jun 2025 14:11:31 +0900 Hyesoo Yu <hyesoo.yu@samsung.com> wrote: > 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 GUP longterm > failures in cases where the same VMA was accessed repeatedly. > > Specifically, the first GUP longterm call would pin a CMA page, and a second > call on the same region would fail the migration due to the cma page already > being pinned. > > After reverting commit 1aaf8c122918, the issue no longer reproduced. > > Therefore, this fix is important to ensure reliable behavior of GUP longterm > and CMA-backed memory, and should be backported to stable. Great, thanks. Please add this to the patch's changelog. The problem is, this series combines a non-urgent cleanup with an important, backportable regression fix. We shouldn't backport the cleanup into earlier kernels - that just adds undesirable noise. So can I ask you to prepare a single standalone fix for the regression against current -linus and to later propose the cleanup patch for 6.17-rc1? In other words, pleas reverse the patching order, send the patches separately and test the regression fix without the presence of the cleanup? (I could do these manipulations locally but then what I have for the regression fix wasn't standalone tested by yourself). Thanks.
On Wed, Jun 04, 2025 at 10:24:00PM -0700, Andrew Morton wrote: > On Thu, 5 Jun 2025 14:11:31 +0900 Hyesoo Yu <hyesoo.yu@samsung.com> wrote: > > > 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 GUP longterm > > failures in cases where the same VMA was accessed repeatedly. > > > > Specifically, the first GUP longterm call would pin a CMA page, and a second > > call on the same region would fail the migration due to the cma page already > > being pinned. > > > > After reverting commit 1aaf8c122918, the issue no longer reproduced. > > > > Therefore, this fix is important to ensure reliable behavior of GUP longterm > > and CMA-backed memory, and should be backported to stable. > > Great, thanks. Please add this to the patch's changelog. > > > The problem is, this series combines a non-urgent cleanup with an > important, backportable regression fix. We shouldn't backport the > cleanup into earlier kernels - that just adds undesirable noise. > > So can I ask you to prepare a single standalone fix for the regression > against current -linus and to later propose the cleanup patch for > 6.17-rc1? > > In other words, pleas reverse the patching order, send the patches > separately and test the regression fix without the presence of the > cleanup? > > (I could do these manipulations locally but then what I have for the > regression fix wasn't standalone tested by yourself). > > Thanks. > Thanks for the clarification. I'll prepare a standalone v3 patch with just the fix, and send the cleanup separately for 6.17-rc1 as suggested. Thanks, Regards.
© 2016 - 2025 Red Hat, Inc.