Thread (8 messages) 8 messages, 2 authors, 2020-05-05

Re: [PATCH 3/3] mm/hugetlb: Introduce HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS

From: Anshuman Khandual <hidden>
Date: 2020-05-05 02:52:20
Also in: linux-arch, linux-arm-kernel, linux-mips, linux-mm, linux-riscv, linux-s390, linux-sh, lkml, sparclinux


On 04/26/2020 08:31 AM, Andrew Morton wrote:
On Sun, 26 Apr 2020 08:13:17 +0530 Anshuman Khandual [off-list ref] wrote:
quoted

On 04/26/2020 06:25 AM, Andrew Morton wrote:
quoted
On Tue, 14 Apr 2020 17:14:30 +0530 Anshuman Khandual [off-list ref] wrote:
quoted
There are multiple similar definitions for arch_clear_hugepage_flags() on
various platforms. This introduces HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS for those
platforms that need to define their own arch_clear_hugepage_flags() while
also providing a generic fallback definition for others to use. This help
reduce code duplication.

...
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -544,6 +544,10 @@ static inline int is_hugepage_only_range(struct mm_struct *mm,
 }
 #endif
 
+#ifndef HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS
+static inline void arch_clear_hugepage_flags(struct page *page) { }
+#endif
+
 #ifndef arch_make_huge_pte
 static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
 				       struct page *page, int writable)
This is the rather old-school way of doing it.  The Linus-suggested way is

#ifndef arch_clear_hugepage_flags
static inline void arch_clear_hugepage_flags(struct page *page)
{
}
#define arch_clear_hugepage_flags arch_clear_hugepage_flags
Do we need that above line here ? Is not that implicit.
It depends if other header files want to test whether
arch_clear_hugepage_flags is already defined.  If the header heorarchy
is well-defined and working properly, they shouldn't need to, because
we're reliably indluding the relevant arch header before (or early
within) include/linux/hugetlb.h.

It would be nice if

#define arch_clear_hugepage_flags arch_clear_hugepage_flags
#define arch_clear_hugepage_flags arch_clear_hugepage_flags

were to generate an compiler error but it doesn't.  If it did we could
detect these incorrect inclusion orders.
quoted
quoted
#endif

And the various arch headers do

static inline void arch_clear_hugepage_flags(struct page *page)
{
	<some implementation>
}
#define arch_clear_hugepage_flags arch_clear_hugepage_flags

It's a small difference - mainly to avoid adding two variables to the
overall namespace where one would do.
Understood, will change and resend.
That's OK - I've queued up that fix.
Hello Andrew,

I might not have searched all the relevant trees or might have just searched
earlier than required. But I dont see these patches (or your proposed fixes)
either in mmotm (2020-04-29-23-04) or in next-20200504. Wondering if you are
waiting on a V2 for this series accommodating the changes you had proposed.

- Anshuman
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help