[PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

[PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

[Sidhartha Kumar]

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 <sidhartha.kumar@oracle.com>
---
 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

Re: [PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

[Matthew Wilcox]

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) <willy@infradead.org>

Re: [PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

[Sidhartha Kumar]

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) <willy@infradead.org>
> 
> 

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/Y89DK23hYiLtgGNk@casper.infradead.org/T/#mb3a339b98386b1cd0b87f94f45163756ebd7feaa

Re: [PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

[Matthew Wilcox]

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) <willy@infradead.org>
> 
> 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/Y89DK23hYiLtgGNk@casper.infradead.org/

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.

Re: [PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

[David Hildenbrand]

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 <sidhartha.kumar@oracle.com>
> ---


Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

[Oscar Salvador]

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 <sidhartha.kumar@oracle.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

 

-- 
Oscar Salvador
SUSE Labs