On 10/17/24 1:51 AM, David Hildenbrand wrote:
On 16.10.24 22:22, John Hubbard wrote:
...
diff --git a/mm/gup.c b/mm/gup.c
index a82890b46a36..24acf53c8294 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2492,6 +2492,17 @@ static long __gup_longterm_locked(struct
mm_struct *mm,
/* FOLL_LONGTERM implies FOLL_PIN */
rc = check_and_migrate_movable_pages(nr_pinned_pages, pages);
+
+ /*
+ * The __get_user_pages_locked() call happens before we know
+ * that whether it's possible to successfully complete the whole
oops, s/that whether/that/
+ * operation. To compensate for this, if we get an unexpected
+ * error (such as -ENOMEM) then we must unpin everything, before
+ * erroring out.
+ */
+ if (rc != -EAGAIN && rc != 0)
+ unpin_user_pages(pages, nr_pinned_pages);
+
} while (rc == -EAGAIN);
Wouldn't it be cleaner to simply have here after the loop (possibly even
after the memalloc_pin_restore())
if (rc)
unpin_user_pages(pages, nr_pinned_pages);
But maybe I am missing something.
Yes, I think you are correct. That is cleaner. Let me retest real quick just
in case, and then send a v2 that also picks up the typo and moves the
comment
to the new location.
thanks,
--
John Hubbard
