On 27.08.24 03:26, Kefeng Wang wrote:
On 2024/8/26 22:55, David Hildenbrand wrote:
On 17.08.24 10:49, Kefeng Wang wrote:
Use the isolate_folio_to_list() to unify hugetlb/LRU/non-LRU
folio isolation, which cleanup code a bit and save a few calls
to compound_head().
Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
---
mm/memory_hotplug.c | 45 +++++++++++++++++----------------------------
1 file changed, 17 insertions(+), 28 deletions(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 02a0d4fbc3fe..cc9c16db2f8c 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1773,14 +1773,14 @@ static int scan_movable_pages(unsigned long
start, unsigned long end,
static void do_migrate_range(unsigned long start_pfn, unsigned long
end_pfn)
{
unsigned long pfn;
- struct page *page;
LIST_HEAD(source);
+ struct folio *folio;
static DEFINE_RATELIMIT_STATE(migrate_rs,
DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);
for (pfn = start_pfn; pfn < end_pfn; pfn++) {
- struct folio *folio;
- bool isolated;
+ struct page *page;
+ bool huge;
Please use "hugetlb" if you mean hugetlb :)
OK.
if (!pfn_valid(pfn))
continue;
@@ -1812,34 +1812,22 @@ static void do_migrate_range(unsigned long
start_pfn, unsigned long end_pfn)
continue;
}
- if (folio_test_hugetlb(folio)) {
- isolate_hugetlb(folio, &source);
- continue;
+ huge = folio_test_hugetlb(folio);
+ if (!huge) {
+ folio = folio_get_nontail_page(page);
+ if (!folio)
+ continue;
}
Hm, remind me why we are changing the hugetlb code to not take a
reference here? It does look odd.
Different from folio_isolate_lru(), isolate_hugetlb() will check folio
and try get a reference of folio after get hugetlb_lock, other hugetlb
operation protected by this big lock too, so no need to take a reference
here.
But this hugetlb-special casing looks quite ... special TBH.
Is there no way to avoid it?
I'd prefer something like the following, with a comment
if (hugetlb)
/*
* We also want to migrate hugetlb folios that span multiple
* memory blocks. So use whatever head page we identified.
*/
folio = folio_try_get(folio);
else
folio = folio_get_nontail_page(page);
if (!folio)
continue;
And then just dropping the reference unconditionally.
Is there a problem with that? Optimizing for hugetlb references during
migration is not worth the trouble.
But now I wonder, why we not simply unconditionally do a folio_try_get() ...
--
Cheers,
David / dhildenb
