2024-05-28 22:03:57

by Sidhartha Kumar

Subject: [PATCH] mm/hugetlb: mm/memory_hotplug: use a folio in scan_movable_pages()

By using a folio in scan_movable_pages(), we convert the last user of the
page-based hugetlb meta-data macro functions to the folio version.
After this conversion, we can safely remove the page-based definitions
from include/linux/hugetlb.h.

Signed-off-by: Sidhartha Kumar <[email protected]>
---
include/linux/hugetlb.h | 6 +-----
mm/memory_hotplug.c | 9 +++++----
2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 15a58f69782c..279aca379b95 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -616,9 +616,7 @@ static __always_inline \
bool folio_test_hugetlb_##flname(struct folio *folio) \
{ void *private = &folio->private; \
return test_bit(HPG_##flname, private); \
- } \
-static inline int HPage##uname(struct page *page) \
- { return test_bit(HPG_##flname, &(page->private)); }
+ }

#define SETHPAGEFLAG(uname, flname) \
static __always_inline \
@@ -637,8 +635,6 @@ void folio_clear_hugetlb_##flname(struct folio *folio) \
#define TESTHPAGEFLAG(uname, flname) \
static inline bool \
folio_test_hugetlb_##flname(struct folio *folio) \
- { return 0; } \
-static inline int HPage##uname(struct page *page) \
{ return 0; }

#define SETHPAGEFLAG(uname, flname) \
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 431b1f6753c0..3573f39fbaa6 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1731,7 +1731,8 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
unsigned long pfn;

for (pfn = start; pfn < end; pfn++) {
- struct page *page, *head;
+ struct page *page;
+ struct folio *folio;
unsigned long skip;

if (!pfn_valid(pfn))
@@ -1753,7 +1754,7 @@ static int scan_movable_pages(unsigned long start, unsigned long end,

if (!PageHuge(page))
continue;
- head = compound_head(page);
+ folio = page_folio(page);
/*
* This test is racy as we hold no reference or lock. The
* hugetlb page could have been free'ed and head is no longer
@@ -1761,9 +1762,9 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
* cases false positives and negatives are possible. Calling
* code must deal with these scenarios.
*/
- if (HPageMigratable(head))
+ if (folio_test_hugetlb_migratable(folio))
goto found;
- skip = compound_nr(head) - (pfn - page_to_pfn(head));
+ skip = folio_nr_pages(folio) - folio_page_idx(folio, page);
pfn += skip - 1;
}
return -ENOENT;
--
2.45.1

2024-05-28 22:48:10

by Matthew Wilcox

Subject: Re: [PATCH] mm/hugetlb: mm/memory_hotplug: use a folio in scan_movable_pages()

On Tue, May 28, 2024 at 03:03:21PM -0700, Sidhartha Kumar wrote:
> @@ -1761,9 +1762,9 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
> * cases false positives and negatives are possible. Calling
> * code must deal with these scenarios.
> */
> - if (HPageMigratable(head))
> + if (folio_test_hugetlb_migratable(folio))
> goto found;
> - skip = compound_nr(head) - (pfn - page_to_pfn(head));
> + skip = folio_nr_pages(folio) - folio_page_idx(folio, page);
> pfn += skip - 1;

Isn't this an unnecessarily complicated way of writing:

pfn |= folio_nr_pages(folio) - 1;
?

2024-05-29 18:21:47

by Sidhartha Kumar

Subject: Re: [PATCH] mm/hugetlb: mm/memory_hotplug: use a folio in scan_movable_pages()

On 5/28/24 3:47 PM, Matthew Wilcox wrote:
> On Tue, May 28, 2024 at 03:03:21PM -0700, Sidhartha Kumar wrote:
>> @@ -1761,9 +1762,9 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
>> * cases false positives and negatives are possible. Calling
>> * code must deal with these scenarios.
>> */
>> - if (HPageMigratable(head))
>> + if (folio_test_hugetlb_migratable(folio))
>> goto found;
>> - skip = compound_nr(head) - (pfn - page_to_pfn(head));
>> + skip = folio_nr_pages(folio) - folio_page_idx(folio, page);
>> pfn += skip - 1;
>
> Isn't this an unnecessarily complicated way of writing:
>
> pfn |= folio_nr_pages(folio) - 1;
> ?

Because folio_nr_pages() is a power of 2, I agree this is simpler. I'll make
this change in a v2.

Thanks,
Sid