[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c1587c7f-9155-45be-bd62-1e36c0dd6923@nvidia.com>
Date: Wed, 6 Nov 2024 20:20:05 -0800
From: John Hubbard <jhubbard@...dia.com>
To: Oscar Salvador <osalvador@...e.de>
CC: Andrew Morton <akpm@...ux-foundation.org>, LKML
<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>, David Hildenbrand
<david@...hat.com>, Vivek Kasireddy <vivek.kasireddy@...el.com>, Dave Airlie
<airlied@...hat.com>, Gerd Hoffmann <kraxel@...hat.com>, Matthew Wilcox
<willy@...radead.org>, Christoph Hellwig <hch@...radead.org>, Jason Gunthorpe
<jgg@...dia.com>, Peter Xu <peterx@...hat.com>, Arnd Bergmann
<arnd@...db.de>, Daniel Vetter <daniel.vetter@...ll.ch>, Dongwon Kim
<dongwon.kim@...el.com>, Hugh Dickins <hughd@...gle.com>, Junxiao Chang
<junxiao.chang@...el.com>, Mike Kravetz <mike.kravetz@...cle.com>,
<linux-stable@...r.kernel.org>
Subject: Re: [PATCH v2 1/1] [PATCH] mm/gup: avoid an unnecessary allocation
call for FOLL_LONGTERM cases
On 11/6/24 1:23 AM, Oscar Salvador wrote:
> On Mon, Nov 04, 2024 at 07:29:44PM -0800, John Hubbard wrote:
...
> Hi John, thanks for doing this.
>
> Reviewed-by: Oscar Salvador <osalvador@...e.de>
Thanks for the review!
>
> Nit below:
>
...
>> @@ -2363,8 +2399,8 @@ static int migrate_longterm_unpinnable_folios(
>> * calling folio_isolate_lru() which takes a reference so the
>> * folio won't be freed if it's migrating.
>> */
>> - unpin_folio(folios[i]);
>> - folios[i] = NULL;
>> + unpin_folio(pofs_get_folio(pofs, i));
>
> We already retrieved the folio before, cannot we just bypass
> pofs_get_folio() here?
>
Yes, you are right. Andrew, can we please add this fixup on top of the commits
in today's mm-hotfixes-unstable (or, let me know if you'd prefer a v3 instead):
diff --git a/mm/gup.c b/mm/gup.c
index 0a22f7def83c..ad0c8922dac3 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2399,7 +2399,7 @@ migrate_longterm_unpinnable_folios(struct list_head *movable_folio_list,
* calling folio_isolate_lru() which takes a reference so the
* folio won't be freed if it's migrating.
*/
- unpin_folio(pofs_get_folio(pofs, i));
+ unpin_folio(folio);
pofs_clear_entry(pofs, i);
}
thanks,
--
John Hubbard
Powered by blists - more mailing lists