All users have been converted to use the folio version of these macros,
we can safely remove the page based interface.
Signed-off-by: Sidhartha Kumar <[email protected]>
---
include/linux/hugetlb.h | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 77b30a8c6076b..c99ed9d2192d8 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -657,18 +657,14 @@ static __always_inline \
void folio_set_hugetlb_##flname(struct folio *folio) \
{ void *private = &folio->private; \
set_bit(HPG_##flname, private); \
- } \
-static inline void SetHPage##uname(struct page *page) \
- { set_bit(HPG_##flname, &(page->private)); }
+ }
#define CLEARHPAGEFLAG(uname, flname) \
static __always_inline \
void folio_clear_hugetlb_##flname(struct folio *folio) \
{ void *private = &folio->private; \
clear_bit(HPG_##flname, private); \
- } \
-static inline void ClearHPage##uname(struct page *page) \
- { clear_bit(HPG_##flname, &(page->private)); }
+ }
#else
#define TESTHPAGEFLAG(uname, flname) \
static inline bool \
@@ -680,15 +676,11 @@ static inline int HPage##uname(struct page *page) \
#define SETHPAGEFLAG(uname, flname) \
static inline void \
folio_set_hugetlb_##flname(struct folio *folio) \
- { } \
-static inline void SetHPage##uname(struct page *page) \
{ }
#define CLEARHPAGEFLAG(uname, flname) \
static inline void \
folio_clear_hugetlb_##flname(struct folio *folio) \
- { } \
-static inline void ClearHPage##uname(struct page *page) \
{ }
#endif
--
2.45.1
On Mon, May 20, 2024 at 03:44:07PM -0700, Sidhartha Kumar wrote:
> All users have been converted to use the folio version of these macros,
> we can safely remove the page based interface.
Yay!
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
On 5/20/24 4:30 PM, Matthew Wilcox wrote:
> On Mon, May 20, 2024 at 03:44:07PM -0700, Sidhartha Kumar wrote:
>> All users have been converted to use the folio version of these macros,
>> we can safely remove the page based interface.
>
> Yay!
>
> Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
>
>
There is only one remaining user of page-based Test version of these macros.
in mm/memory-hotplug.c:
if (!PageHuge(page))
continue;
head = compound_head(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
* a hugetlb page before the following check. In such unlikely
* cases false positives and negatives are possible. Calling
* code must deal with these scenarios.
*/
if (HPageMigratable(head))
goto found;
skip = compound_nr(head) - (pfn - page_to_pfn(head));
I've previously sent a patch to convert this to folios[1] but got feedback that
it was unsafe. But I'm not sure why replacing compound_head() with page_folio()
and using folio_test_hugetlb_migratable(folio) rather than HPageMigratable(head)
changes the existing behavior. With no reference or lock, can't the head pointer
also be moved and no longer be a part of page like the comment states. So would
the folio conversion just be maintaining this level of existing un-safety that
the calling code should handle anyways?
[1]:
https://lore.kernel.org/lkml/[email protected]/T/#mb3a339b98386b1cd0b87f94f45163756ebd7feaa
On Mon, May 20, 2024 at 04:44:53PM -0700, Sidhartha Kumar wrote:
> On 5/20/24 4:30 PM, Matthew Wilcox wrote:
> > On Mon, May 20, 2024 at 03:44:07PM -0700, Sidhartha Kumar wrote:
> > > All users have been converted to use the folio version of these macros,
> > > we can safely remove the page based interface.
> >
> > Yay!
> >
> > Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
>
> There is only one remaining user of page-based Test version of these macros.
>
> in mm/memory-hotplug.c:
>
> if (!PageHuge(page))
> continue;
> head = compound_head(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
> * a hugetlb page before the following check. In such unlikely
> * cases false positives and negatives are possible. Calling
> * code must deal with these scenarios.
> */
> if (HPageMigratable(head))
> goto found;
> skip = compound_nr(head) - (pfn - page_to_pfn(head));
>
>
> I've previously sent a patch to convert this to folios[1] but got feedback
> that it was unsafe. But I'm not sure why replacing compound_head() with
> page_folio() and using folio_test_hugetlb_migratable(folio) rather than
> HPageMigratable(head) changes the existing behavior. With no reference or
> lock, can't the head pointer also be moved and no longer be a part of page
> like the comment states. So would the folio conversion just be maintaining
> this level of existing un-safety that the calling code should handle
> anyways?
To be fair, that wasn't the last thing I said about that ...
https://lore.kernel.org/lkml/[email protected]/
and looked like David agreed that this was a case where false
postive/negative was fine; we were just looking to be right most of
the time.
On 21.05.24 00:44, Sidhartha Kumar wrote:
> All users have been converted to use the folio version of these macros,
> we can safely remove the page based interface.
>
> Signed-off-by: Sidhartha Kumar <[email protected]>
> ---
Reviewed-by: David Hildenbrand <[email protected]>
--
Cheers,
David / dhildenb
On Mon, May 20, 2024 at 03:44:07PM -0700, Sidhartha Kumar wrote:
> All users have been converted to use the folio version of these macros,
> we can safely remove the page based interface.
>
> Signed-off-by: Sidhartha Kumar <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
--
Oscar Salvador
SUSE Labs