Skip to contentSkip to conversation

hugetlbfs: cleanup and new primitives for s390

PATCH In review Gerald Schaefer 4 participants 8 messages Apr 2, 2008 – Apr 3, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover hugetlbfs: cleanup and new primitives for s390

Quoted 7 lines · Andrew Morton
Quoted 2 lines

As there were no objections so far, which would be the right tree for this to get it upstream?

-mm, I suppose.

But all the patcehs had the same title and I'd prefer to not have to invent your patch titles for you.

This patchset cleans up hugetlbfs architecture headers and adds new primitives as preparation for the s390 large page support. Most notably we found that we need to add a new tlb flush for the copy-on-write of a large page (patch #2). We think that this is a bug, but obviously one that hasn't shown up so far on any of the other large page architectures.

--
Gerald Schaefer

#1 lore raw

PATCH 1/3 hugetlbfs: architecture header cleanup

Subject: [PATCH 1/3] hugetlbfs: architecture header cleanup

From: Gerald Schaefer <[email protected]>

This patch moves all architecture functions for hugetlb to architecture header files (include/asm-foo/hugetlb.h). It also removes (!) ARCH_HAS_HUGEPAGE_ONLY_RANGE, ARCH_HAS_HUGETLB_FREE_PGD_RANGE, ARCH_HAS_PREPARE_HUGEPAGE_RANGE, ARCH_HAS_SETCLEAR_HUGE_PTE and ARCH_HAS_HUGETLB_PREFAULT_HOOK.

Cross-Compile tests on the affected architectures (and one unaffected) worked fine, but I had no cross-compiler for sh.

---

 include/asm-ia64/hugetlb.h    |   21 +++++++++++++++++++
 include/asm-ia64/page.h       |    6 -----
 include/asm-powerpc/hugetlb.h |   35 +++++++++++++++++++++++++++++++
 include/asm-powerpc/page_64.h |    7 ------
 include/asm-sh/hugetlb.h      |   28 +++++++++++++++++++++++++
 include/asm-sparc64/hugetlb.h |   30 +++++++++++++++++++++++++++
 include/asm-sparc64/page.h    |    2 -
 include/asm-x86/hugetlb.h     |   28 +++++++++++++++++++++++++
 include/linux/hugetlb.h       |   46 ------------------------------------------
 9 files changed, 143 insertions(+), 60 deletions(-)
include/asm-ia64/hugetlb.h new file +21 -0
Index: linux-2.6.25-rc7/include/asm-ia64/hugetlb.h
===================================================================
@@ -0,0 +1,21 @@
+#ifndef _ASM_IA64_HUGETLB_H
+#define _ASM_IA64_HUGETLB_H
+
+#include <asm/page.h>
+
+
+#define is_hugepage_only_range(mm, addr, len)		\
+	(REGION_NUMBER(addr) == RGN_HPAGE ||	\
+	 REGION_NUMBER((addr)+(len)-1) == RGN_HPAGE)
+
+void hugetlb_free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
+			    unsigned long end, unsigned long floor,
+			    unsigned long ceiling);
+int prepare_hugepage_range(unsigned long addr, unsigned long len);
+
+#define set_huge_pte_at(mm, addr, ptep, pte)	set_pte_at(mm, addr, ptep, pte)
+#define huge_ptep_get_and_clear(mm, addr, ptep) ptep_get_and_clear(mm, addr, ptep)
+
+#define hugetlb_prefault_arch_hook(mm)		do { } while (0)
+
+#endif /* _ASM_IA64_HUGETLB_H */
include/asm-sh/hugetlb.h new file +28 -0
Index: linux-2.6.25-rc7/include/asm-sh/hugetlb.h
===================================================================
@@ -0,0 +1,28 @@
+#ifndef _ASM_SH_HUGETLB_H
+#define _ASM_SH_HUGETLB_H
+
+#include <asm/page.h>
+
+
+#define is_hugepage_only_range(mm, addr, len)	0
+#define hugetlb_free_pgd_range			free_pgd_range
+
+/*
+ * If the arch doesn't supply something else, assume that hugepage
+ * size aligned regions are ok without further preparation.
+ */
+static inline int prepare_hugepage_range(unsigned long addr, unsigned long len)
+{
+	if (len & ~HPAGE_MASK)
+		return -EINVAL;
+	if (addr & ~HPAGE_MASK)
+		return -EINVAL;
+	return 0;
+}
+
+#define set_huge_pte_at(mm, addr, ptep, pte)	set_pte_at(mm, addr, ptep, pte)
+#define huge_ptep_get_and_clear(mm, addr, ptep) ptep_get_and_clear(mm, addr, ptep)
+
+#define hugetlb_prefault_arch_hook(mm)		do { } while (0)
+
+#endif /* _ASM_SH_HUGETLB_H */
include/asm-sparc64/hugetlb.h new file +30 -0
Index: linux-2.6.25-rc7/include/asm-sparc64/hugetlb.h
===================================================================
@@ -0,0 +1,30 @@
+#ifndef _ASM_SPARC64_HUGETLB_H
+#define _ASM_SPARC64_HUGETLB_H
+
+#include <asm/page.h>
+
+
+#define is_hugepage_only_range(mm, addr, len)	0
+#define hugetlb_free_pgd_range			free_pgd_range
+
+/*
+ * If the arch doesn't supply something else, assume that hugepage
+ * size aligned regions are ok without further preparation.
+ */
+static inline int prepare_hugepage_range(unsigned long addr, unsigned long len)
+{
+	if (len & ~HPAGE_MASK)
+		return -EINVAL;
+	if (addr & ~HPAGE_MASK)
+		return -EINVAL;
+	return 0;
+}
+
+void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
+		     pte_t *ptep, pte_t pte);
+pte_t huge_ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
+			      pte_t *ptep);
+
+void hugetlb_prefault_arch_hook(struct mm_struct *mm);
+
+#endif /* _ASM_SPARC64_HUGETLB_H */
include/asm-x86/hugetlb.h new file +28 -0
Index: linux-2.6.25-rc7/include/asm-x86/hugetlb.h
===================================================================
@@ -0,0 +1,28 @@
+#ifndef _ASM_X86_HUGETLB_H
+#define _ASM_X86_HUGETLB_H
+
+#include <asm/page.h>
+
+
+#define is_hugepage_only_range(mm, addr, len)	0
+#define hugetlb_free_pgd_range			free_pgd_range
+
+/*
+ * If the arch doesn't supply something else, assume that hugepage
+ * size aligned regions are ok without further preparation.
+ */
+static inline int prepare_hugepage_range(unsigned long addr, unsigned long len)
+{
+	if (len & ~HPAGE_MASK)
+		return -EINVAL;
+	if (addr & ~HPAGE_MASK)
+		return -EINVAL;
+	return 0;
+}
+
+#define set_huge_pte_at(mm, addr, ptep, pte)	set_pte_at(mm, addr, ptep, pte)
+#define huge_ptep_get_and_clear(mm, addr, ptep) ptep_get_and_clear(mm, addr, ptep)
+
+#define hugetlb_prefault_arch_hook(mm)		do { } while (0)
+
+#endif /* _ASM_X86_HUGETLB_H */
include/linux/hugetlb.h +1 -45

58 lines not on this page · raw message

include/asm-powerpc/hugetlb.h new file +35 -0
Index: linux-2.6.25-rc7/include/asm-powerpc/hugetlb.h
===================================================================
@@ -0,0 +1,35 @@
+#ifndef _ASM_POWERPC_HUGETLB_H
+#define _ASM_POWERPC_HUGETLB_H
+
+#include <asm/page.h>
+
+
+extern int is_hugepage_only_range(struct mm_struct *m,
+				  unsigned long addr,
+				  unsigned long len);
+
+void hugetlb_free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
+			    unsigned long end, unsigned long floor,
+			    unsigned long ceiling);
+
+/*
+ * If the arch doesn't supply something else, assume that hugepage
+ * size aligned regions are ok without further preparation.
+ */
+static inline int prepare_hugepage_range(unsigned long addr, unsigned long len)
+{
+	if (len & ~HPAGE_MASK)
+		return -EINVAL;
+	if (addr & ~HPAGE_MASK)
+		return -EINVAL;
+	return 0;
+}
+
+void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
+		     pte_t *ptep, pte_t pte);
+pte_t huge_ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
+			      pte_t *ptep);
+
+#define hugetlb_prefault_arch_hook(mm)		do { } while (0)
+
+#endif /* _ASM_POWERPC_HUGETLB_H */
include/asm-ia64/page.h +0 -6
Index: linux-2.6.25-rc7/include/asm-ia64/page.h
===================================================================
@@ -54,9 +54,6 @@
 # define HPAGE_MASK		(~(HPAGE_SIZE - 1))
 
 # define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
-# define ARCH_HAS_HUGEPAGE_ONLY_RANGE
-# define ARCH_HAS_PREPARE_HUGEPAGE_RANGE
-# define ARCH_HAS_HUGETLB_FREE_PGD_RANGE
 #endif /* CONFIG_HUGETLB_PAGE */
 
 #ifdef __ASSEMBLY__
@@ -153,9 +150,6 @@ typedef union ia64_va {
 # define htlbpage_to_page(x)	(((unsigned long) REGION_NUMBER(x) << 61)			\
 				 | (REGION_OFFSET(x) >> (HPAGE_SHIFT-PAGE_SHIFT)))
 # define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
-# define is_hugepage_only_range(mm, addr, len)		\
-	 (REGION_NUMBER(addr) == RGN_HPAGE ||	\
-	  REGION_NUMBER((addr)+(len)-1) == RGN_HPAGE)
 extern unsigned int hpage_shift;
 #endif
 
include/asm-powerpc/page_64.h +0 -7
Index: linux-2.6.25-rc7/include/asm-powerpc/page_64.h
===================================================================
@@ -128,11 +128,6 @@ extern void slice_init_context(struct mm
 extern void slice_set_user_psize(struct mm_struct *mm, unsigned int psize);
 #define slice_mm_new_context(mm)	((mm)->context.id == 0)
 
-#define ARCH_HAS_HUGEPAGE_ONLY_RANGE
-extern int is_hugepage_only_range(struct mm_struct *m,
-				  unsigned long addr,
-				  unsigned long len);
-
 #endif /* __ASSEMBLY__ */
 #else
 #define slice_init()
@@ -146,8 +141,6 @@ do {						\
 
 #ifdef CONFIG_HUGETLB_PAGE
 
-#define ARCH_HAS_HUGETLB_FREE_PGD_RANGE
-#define ARCH_HAS_SETCLEAR_HUGE_PTE
 #define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
 
 #endif /* !CONFIG_HUGETLB_PAGE */
include/asm-sparc64/page.h +0 -2
Index: linux-2.6.25-rc7/include/asm-sparc64/page.h
===================================================================
@@ -39,8 +39,6 @@
 #define HPAGE_SIZE		(_AC(1,UL) << HPAGE_SHIFT)
 #define HPAGE_MASK		(~(HPAGE_SIZE - 1UL))
 #define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
-#define ARCH_HAS_SETCLEAR_HUGE_PTE
-#define ARCH_HAS_HUGETLB_PREFAULT_HOOK
 #define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
 #endif
 
#2 lore raw

Re: [PATCH 1/3] hugetlbfs: architecture header cleanup

Quoted 10 lines · Gerald Schaefer
 include/asm-ia64/hugetlb.h    |   21 +++++++++++++++++++
 include/asm-ia64/page.h       |    6 -----
 include/asm-powerpc/hugetlb.h |   35 +++++++++++++++++++++++++++++++
 include/asm-powerpc/page_64.h |    7 ------
 include/asm-sh/hugetlb.h      |   28 +++++++++++++++++++++++++
 include/asm-sparc64/hugetlb.h |   30 +++++++++++++++++++++++++++
 include/asm-sparc64/page.h    |    2 -
 include/asm-x86/hugetlb.h     |   28 +++++++++++++++++++++++++
 include/linux/hugetlb.h       |   46 ------------------------------------------

The way I read this, you took some arch-independent bits, like prepare_hugepage_range(), and copied them to several architectures. How is this a cleanup? Can they really not share common code?

-- Dave

#3 lore raw

Re: [PATCH 1/3] hugetlbfs: architecture header cleanup

Quoted 13 lines · Dave Hansen
Quoted 10 lines · Gerald Schaefer
 include/asm-ia64/hugetlb.h    |   21 +++++++++++++++++++
 include/asm-ia64/page.h       |    6 -----
 include/asm-powerpc/hugetlb.h |   35 +++++++++++++++++++++++++++++++
 include/asm-powerpc/page_64.h |    7 ------
 include/asm-sh/hugetlb.h      |   28 +++++++++++++++++++++++++
 include/asm-sparc64/hugetlb.h |   30 +++++++++++++++++++++++++++
 include/asm-sparc64/page.h    |    2 -
 include/asm-x86/hugetlb.h     |   28 +++++++++++++++++++++++++
 include/linux/hugetlb.h       |   46 ------------------------------------------
Quoted 3 lines

The way I read this, you took some arch-independent bits, like prepare_hugepage_range(), and copied them to several architectures. How is this a cleanup? Can they really not share common code?

This question has been raised before. See: http://lkml.org/lkml/2008/3/28/267 .

maintainer
#4 lore raw

Re: [PATCH 1/3] hugetlbfs: architecture header cleanup

In reply to Gerald Schaefer
Quoted 4 lines · On Wed, 02 Apr 2008 16:26:55 +0200 Gerald Schaefer…

Subject: [PATCH 1/3] hugetlbfs: architecture header cleanup

From: Gerald Schaefer <[email protected]>

Quoted 9 lines

This patch moves all architecture functions for hugetlb to architecture header files (include/asm-foo/hugetlb.h). It also removes (!) ARCH_HAS_HUGEPAGE_ONLY_RANGE, ARCH_HAS_HUGETLB_FREE_PGD_RANGE, ARCH_HAS_PREPARE_HUGEPAGE_RANGE, ARCH_HAS_SETCLEAR_HUGE_PTE and ARCH_HAS_HUGETLB_PREFAULT_HOOK.

Cross-Compile tests on the affected architectures (and one unaffected) worked fine, but I had no cross-compiler for sh.

This text explains what the patch does but not why it does it. Always provide both, please. That way people will stop asking why this:

Quoted 11 lines
 include/asm-ia64/hugetlb.h    |   21 +++++++++++++++++++
 include/asm-ia64/page.h       |    6 -----
 include/asm-powerpc/hugetlb.h |   35 +++++++++++++++++++++++++++++++
 include/asm-powerpc/page_64.h |    7 ------
 include/asm-sh/hugetlb.h      |   28 +++++++++++++++++++++++++
 include/asm-sparc64/hugetlb.h |   30 +++++++++++++++++++++++++++
 include/asm-sparc64/page.h    |    2 -
 include/asm-x86/hugetlb.h     |   28 +++++++++++++++++++++++++
 include/linux/hugetlb.h       |   46 ------------------------------------------
 9 files changed, 143 insertions(+), 60 deletions(-)

is the way it is.

Quoted 47 lines

Index: linux-2.6.25-rc7/include/asm-ia64/hugetlb.h
===================================================================

include/asm-ia64/hugetlb.h new file +21 -0
@@ -0,0 +1,21 @@
+#ifndef _ASM_IA64_HUGETLB_H
+#define _ASM_IA64_HUGETLB_H
+
+#include <asm/page.h>
+
+
+#define is_hugepage_only_range(mm, addr, len)		\
+	(REGION_NUMBER(addr) == RGN_HPAGE ||	\
+	 REGION_NUMBER((addr)+(len)-1) == RGN_HPAGE)
+
+void hugetlb_free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
+			    unsigned long end, unsigned long floor,
+			    unsigned long ceiling);
+int prepare_hugepage_range(unsigned long addr, unsigned long len);
+
+#define set_huge_pte_at(mm, addr, ptep, pte)	set_pte_at(mm, addr, ptep, pte)
+#define huge_ptep_get_and_clear(mm, addr, ptep) ptep_get_and_clear(mm, addr, ptep)
+
+#define hugetlb_prefault_arch_hook(mm)		do { } while (0)
+
+#endif /* _ASM_IA64_HUGETLB_H */

Index: linux-2.6.25-rc7/include/asm-sh/hugetlb.h
===================================================================

include/asm-sh/hugetlb.h new file +16 -0
@@ -0,0 +1,28 @@
+#ifndef _ASM_SH_HUGETLB_H
+#define _ASM_SH_HUGETLB_H
+
+#include <asm/page.h>
+
+
+#define is_hugepage_only_range(mm, addr, len)	0
+#define hugetlb_free_pgd_range			free_pgd_range
+
+/*
+ * If the arch doesn't supply something else, assume that hugepage
+ * size aligned regions are ok without further preparation.
+ */
+static inline int prepare_hugepage_range(unsigned long addr, unsigned long len)
+{
+	if (len & ~HPAGE_MASK)

include/asm-sh/hugetlb.h line 26

Quoted 10 lines
include/asm-sh/hugetlb.h new file +10 -0
+		return -EINVAL;
+	if (addr & ~HPAGE_MASK)
+		return -EINVAL;
+	return 0;
+}
+
+#define set_huge_pte_at(mm, addr, ptep, pte)	set_pte_at(mm, addr, ptep, pte)
+#define huge_ptep_get_and_clear(mm, addr, ptep) ptep_get_and_clear(mm, addr, ptep)
+
+#define hugetlb_prefault_arch_hook(mm)		do { } while (0)

urgh. I wouldn't say the result of this "cleanup" is very clean. Basically all the macros there should be zapped and turned into inlines. That will happen to fix the bug in ia64's is_hugepage_only_range(), which presently references its arg once or twice, and will do bad things if called with an expression-with-side-effects.

Quoted 7 lines
include/asm-sh/hugetlb.h new file +2 -0
+
+#endif /* _ASM_SH_HUGETLB_H */

Index: linux-2.6.25-rc7/include/asm-sparc64/hugetlb.h
===================================================================

include/asm-sparc64/hugetlb.h line 7

Quoted 7 lines
include/asm-sparc64/hugetlb.h new file +7 -0
@@ -0,0 +1,30 @@
+#ifndef _ASM_SPARC64_HUGETLB_H
+#define _ASM_SPARC64_HUGETLB_H
+
+#include <asm/page.h>
+
+
+#define is_hugepage_only_range(mm, addr, len)	0

This is bad too, because it can lead to unused-var warnings if compiled with suitable config combinations.

So basically the existing hugetlb arch support code needs a big de-macro crapectomy. But your patch has gone and massively duplicated the existing crap, making that decrapectomy larger and harder.

#5 lore raw

PATCH 2/3 hugetlbfs: add missing TLB flush to hugetlb_cow()

In reply to Gerald Schaefer

Subject: [PATCH 2/3] hugetlbfs: add missing TLB flush to hugetlb_cow()

From: Gerald Schaefer <[email protected]>

A cow break on a hugetlbfs page with page_count > 1 will set a new pte with set_huge_pte_at(), w/o any tlb flush operation. The old pte will remain in the tlb and subsequent write access to the page will result in a page fault loop, for as long as it may take until the tlb is flushed from somewhere else.
This patch introduces an architecture-specific huge_ptep_clear_flush() function, which is called before the the set_huge_pte_at() in hugetlb_cow().

NOTE: This is just a nop on all architectures for now, there will be an s390 implementation with our large page patch later. Other architectures should define their own huge_ptep_clear_flush() if needed.

---

 include/asm-ia64/hugetlb.h    |    2 ++
 include/asm-powerpc/hugetlb.h |    2 ++
 include/asm-sh/hugetlb.h      |    2 ++
 include/asm-sparc64/hugetlb.h |    2 ++
 include/asm-x86/hugetlb.h     |    2 ++
 mm/hugetlb.c                  |    1 +
 6 files changed, 11 insertions(+)
mm/hugetlb.c +1 -0
Index: linux-2.6.25-rc7/mm/hugetlb.c
===================================================================
@@ -864,6 +864,7 @@ static int hugetlb_cow(struct mm_struct 
 	ptep = huge_pte_offset(mm, address & HPAGE_MASK);
 	if (likely(pte_same(*ptep, pte))) {
 		/* Break COW */
+		huge_ptep_clear_flush(vma, address, ptep);
 		set_huge_pte_at(mm, address, ptep,
 				make_huge_pte(vma, new_page, 1));
 		/* Make the old page be freed below */
include/asm-ia64/hugetlb.h +2 -0
Index: linux-2.6.25-rc7/include/asm-ia64/hugetlb.h
===================================================================
@@ -18,4 +18,6 @@ int prepare_hugepage_range(unsigned long
 
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+
 #endif /* _ASM_IA64_HUGETLB_H */
include/asm-powerpc/hugetlb.h +2 -0
Index: linux-2.6.25-rc7/include/asm-powerpc/hugetlb.h
===================================================================
@@ -32,4 +32,6 @@ pte_t huge_ptep_get_and_clear(struct mm_
 
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+
 #endif /* _ASM_POWERPC_HUGETLB_H */
include/asm-sh/hugetlb.h +2 -0
Index: linux-2.6.25-rc7/include/asm-sh/hugetlb.h
===================================================================
@@ -25,4 +25,6 @@ static inline int prepare_hugepage_range
 
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+
 #endif /* _ASM_SH_HUGETLB_H */
include/asm-sparc64/hugetlb.h +2 -0
Index: linux-2.6.25-rc7/include/asm-sparc64/hugetlb.h
===================================================================
@@ -27,4 +27,6 @@ pte_t huge_ptep_get_and_clear(struct mm_
 
 void hugetlb_prefault_arch_hook(struct mm_struct *mm);
 
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+
 #endif /* _ASM_SPARC64_HUGETLB_H */
include/asm-x86/hugetlb.h +2 -0
Index: linux-2.6.25-rc7/include/asm-x86/hugetlb.h
===================================================================
@@ -25,4 +25,6 @@ static inline int prepare_hugepage_range
 
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+
 #endif /* _ASM_X86_HUGETLB_H */
maintainer
#6 lore raw

Re: [PATCH 2/3] hugetlbfs: add missing TLB flush to hugetlb_cow()

Quoted 17 lines · On Wed, 02 Apr 2008 16:29:05 +0200 Gerald Schaefer…

Subject: [PATCH 2/3] hugetlbfs: add missing TLB flush to hugetlb_cow()

From: Gerald Schaefer <[email protected]>

A cow break on a hugetlbfs page with page_count > 1 will set a new pte with set_huge_pte_at(), w/o any tlb flush operation. The old pte will remain in the tlb and subsequent write access to the page will result in a page fault loop, for as long as it may take until the tlb is flushed from somewhere else.
This patch introduces an architecture-specific huge_ptep_clear_flush() function, which is called before the the set_huge_pte_at() in hugetlb_cow().

NOTE: This is just a nop on all architectures for now, there will be an s390 implementation with our large page patch later. Other architectures should define their own huge_ptep_clear_flush() if needed.

Review

Quoted 5 lines
(quoted diff) +5 -0
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
+#define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)

Again, the problem here is that the macro doesn't have typechecking so I can merrily add

huge_ptep_clear_flush(some_u64, some_struct_ntfs_inode, some_undefined_variable)

and the compiler will just swallow it, depending on config options. An inline solves this.

And the macro can lead to unused-variable warnings because the macro doesn't count as a reference of its args. An inline solves this too.

There is almost never any need to put any code in macros ever. Please make it a last resort, not a first one.

#7 lore raw

PATCH 3/3 hugetlbfs: common code update for s390

In reply to Gerald Schaefer

Subject: [PATCH 3/3] hugetlbfs: common code update for s390

From: Gerald Schaefer <[email protected]>

Huge ptes have a special type on s390 and cannot be handled with the standard pte functions in certain cases. This patch adds some new architecture-specific definitions and functions to hugetlb common code, as a prerequisite for the s390 large page support. They won't affect other architectures.

---

 include/asm-ia64/hugetlb.h    |   11 +++++++++++
 include/asm-powerpc/hugetlb.h |   11 +++++++++++
 include/asm-sh/hugetlb.h      |   11 +++++++++++
 include/asm-sparc64/hugetlb.h |   11 +++++++++++
 include/asm-x86/hugetlb.h     |   11 +++++++++++
 mm/hugetlb.c                  |   36 +++++++++++++++++++++---------------
 6 files changed, 76 insertions(+), 15 deletions(-)
mm/hugetlb.c +21 -15
Index: linux-2.6.25-rc7/mm/hugetlb.c
===================================================================
@@ -129,6 +129,7 @@ static void update_and_free_page(struct 
 	}
 	set_compound_page_dtor(page, NULL);
 	set_page_refcounted(page);
+	arch_release_hugepage(page);
 	__free_pages(page, HUGETLB_PAGE_ORDER);
 }
 
@@ -198,6 +199,10 @@ static struct page *alloc_fresh_huge_pag
 		htlb_alloc_mask|__GFP_COMP|__GFP_THISNODE|__GFP_NOWARN,
 		HUGETLB_PAGE_ORDER);
 	if (page) {
+		if (arch_prepare_hugepage(page)) {
+			__free_pages(page, HUGETLB_PAGE_ORDER);
+			return 0;
+		}
 		set_compound_page_dtor(page, free_huge_page);
 		spin_lock(&hugetlb_lock);
 		nr_huge_pages++;
@@ -707,7 +712,7 @@ static pte_t make_huge_pte(struct vm_are
 		entry =
 		    pte_mkwrite(pte_mkdirty(mk_pte(page, vma->vm_page_prot)));
 	} else {
-		entry = pte_wrprotect(mk_pte(page, vma->vm_page_prot));
+		entry = huge_pte_wrprotect(mk_pte(page, vma->vm_page_prot));
 	}
 	entry = pte_mkyoung(entry);
 	entry = pte_mkhuge(entry);
@@ -720,8 +725,8 @@ static void set_huge_ptep_writable(struc
 {
 	pte_t entry;
 
-	entry = pte_mkwrite(pte_mkdirty(*ptep));
-	if (ptep_set_access_flags(vma, address, ptep, entry, 1)) {
+	entry = pte_mkwrite(pte_mkdirty(huge_ptep_get(ptep)));
+	if (huge_ptep_set_access_flags(vma, address, ptep, entry, 1)) {
 		update_mmu_cache(vma, address, entry);
 	}
 }
@@ -751,10 +756,10 @@ int copy_hugetlb_page_range(struct mm_st
 
 		spin_lock(&dst->page_table_lock);
 		spin_lock(&src->page_table_lock);
-		if (!pte_none(*src_pte)) {
+		if (!huge_pte_none(huge_ptep_get(src_pte))) {
 			if (cow)
-				ptep_set_wrprotect(src, addr, src_pte);
-			entry = *src_pte;
+				huge_ptep_set_wrprotect(src, addr, src_pte);
+			entry = huge_ptep_get(src_pte);
 			ptepage = pte_page(entry);
 			get_page(ptepage);
 			set_huge_pte_at(dst, addr, dst_pte, entry);
@@ -798,7 +803,7 @@ void __unmap_hugepage_range(struct vm_ar
 			continue;
 
 		pte = huge_ptep_get_and_clear(mm, address, ptep);
-		if (pte_none(pte))
+		if (huge_pte_none(pte))
 			continue;
 
 		page = pte_page(pte);
@@ -862,7 +867,7 @@ static int hugetlb_cow(struct mm_struct 
 	spin_lock(&mm->page_table_lock);
 
 	ptep = huge_pte_offset(mm, address & HPAGE_MASK);
-	if (likely(pte_same(*ptep, pte))) {
+	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
 		/* Break COW */
 		huge_ptep_clear_flush(vma, address, ptep);
 		set_huge_pte_at(mm, address, ptep,
@@ -932,7 +937,7 @@ retry:
 		goto backout;
 
 	ret = 0;
-	if (!pte_none(*ptep))
+	if (!huge_pte_none(huge_ptep_get(ptep)))
 		goto backout;
 
 	new_pte = make_huge_pte(vma, page, ((vma->vm_flags & VM_WRITE)
@@ -974,8 +979,8 @@ int hugetlb_fault(struct mm_struct *mm, 
 	 * the same page in the page cache.
 	 */
 	mutex_lock(&hugetlb_instantiation_mutex);
-	entry = *ptep;
-	if (pte_none(entry)) {
+	entry = huge_ptep_get(ptep);
+	if (huge_pte_none(entry)) {
 		ret = hugetlb_no_page(mm, vma, address, ptep, write_access);
 		mutex_unlock(&hugetlb_instantiation_mutex);
 		return ret;
@@ -985,7 +990,7 @@ int hugetlb_fault(struct mm_struct *mm, 
 
 	spin_lock(&mm->page_table_lock);
 	/* Check for a racing update before calling hugetlb_cow */
-	if (likely(pte_same(entry, *ptep)))
+	if (likely(pte_same(entry, huge_ptep_get(ptep))))
 		if (write_access && !pte_write(entry))
 			ret = hugetlb_cow(mm, vma, address, ptep, entry);
 	spin_unlock(&mm->page_table_lock);
@@ -1015,7 +1020,8 @@ int follow_hugetlb_page(struct mm_struct
 		 */
 		pte = huge_pte_offset(mm, vaddr & HPAGE_MASK);
 
-		if (!pte || pte_none(*pte) || (write && !pte_write(*pte))) {
+		if (!pte || huge_pte_none(huge_ptep_get(pte)) ||
+		    (write && !pte_write(huge_ptep_get(pte)))) {
 			int ret;
 
 			spin_unlock(&mm->page_table_lock);
@@ -1031,7 +1037,7 @@ int follow_hugetlb_page(struct mm_struct
 		}
 
 		pfn_offset = (vaddr & ~HPAGE_MASK) >> PAGE_SHIFT;
-		page = pte_page(*pte);
+		page = pte_page(huge_ptep_get(pte));
 same_page:
 		if (pages) {
 			get_page(page);
@@ -1080,7 +1086,7 @@ void hugetlb_change_protection(struct vm
 			continue;
 		if (huge_pmd_unshare(mm, &address, ptep))
 			continue;
-		if (!pte_none(*ptep)) {
+		if (!huge_pte_none(huge_ptep_get(ptep))) {
 			pte = huge_ptep_get_and_clear(mm, address, ptep);
 			pte = pte_mkhuge(pte_modify(pte, newprot));
 			set_huge_pte_at(mm, address, ptep, pte);
include/asm-ia64/hugetlb.h +11 -0
Index: linux-2.6.25-rc7/include/asm-ia64/hugetlb.h
===================================================================
@@ -18,6 +18,17 @@ int prepare_hugepage_range(unsigned long
 
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 
+#define huge_pte_none(pte)			pte_none(pte)
+#define huge_pte_wrprotect(pte)			pte_wrprotect(pte)
+#define huge_ptep_set_wrprotect(mm, addr, ptep)	\
+	ptep_set_wrprotect(mm, addr, ptep)
+#define huge_ptep_set_access_flags(vma, addr, ptep, pte, dirty)	\
+	ptep_set_access_flags(vma, addr, ptep, pte, dirty)
+#define huge_ptep_get(ptep)			(*ptep)
+
+#define arch_prepare_hugepage(page)		0
+#define arch_release_hugepage(page)		do { } while (0)
+
 #define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
 
 #endif /* _ASM_IA64_HUGETLB_H */
include/asm-powerpc/hugetlb.h +11 -0
Index: linux-2.6.25-rc7/include/asm-powerpc/hugetlb.h
===================================================================
@@ -32,6 +32,17 @@ pte_t huge_ptep_get_and_clear(struct mm_
 
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 
+#define huge_pte_none(pte)			pte_none(pte)
+#define huge_pte_wrprotect(pte)			pte_wrprotect(pte)
+#define huge_ptep_set_wrprotect(mm, addr, ptep)	\
+	ptep_set_wrprotect(mm, addr, ptep)
+#define huge_ptep_set_access_flags(vma, addr, ptep, pte, dirty)	\
+	ptep_set_access_flags(vma, addr, ptep, pte, dirty)
+#define huge_ptep_get(ptep)			(*ptep)
+
+#define arch_prepare_hugepage(page)		0
+#define arch_release_hugepage(page)		do { } while (0)
+
 #define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
 
 #endif /* _ASM_POWERPC_HUGETLB_H */
include/asm-sh/hugetlb.h +11 -0
Index: linux-2.6.25-rc7/include/asm-sh/hugetlb.h
===================================================================
@@ -25,6 +25,17 @@ static inline int prepare_hugepage_range
 
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 
+#define huge_pte_none(pte)			pte_none(pte)
+#define huge_pte_wrprotect(pte)			pte_wrprotect(pte)
+#define huge_ptep_set_wrprotect(mm, addr, ptep)	\
+	ptep_set_wrprotect(mm, addr, ptep)
+#define huge_ptep_set_access_flags(vma, addr, ptep, pte, dirty)	\
+	ptep_set_access_flags(vma, addr, ptep, pte, dirty)
+#define huge_ptep_get(ptep)			(*ptep)
+
+#define arch_prepare_hugepage(page)		0
+#define arch_release_hugepage(page)		do { } while (0)
+
 #define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
 
 #endif /* _ASM_SH_HUGETLB_H */
include/asm-sparc64/hugetlb.h +11 -0
Index: linux-2.6.25-rc7/include/asm-sparc64/hugetlb.h
===================================================================
@@ -27,6 +27,17 @@ pte_t huge_ptep_get_and_clear(struct mm_
 
 void hugetlb_prefault_arch_hook(struct mm_struct *mm);
 
+#define huge_pte_none(pte)			pte_none(pte)
+#define huge_pte_wrprotect(pte)			pte_wrprotect(pte)
+#define huge_ptep_set_wrprotect(mm, addr, ptep)	\
+	ptep_set_wrprotect(mm, addr, ptep)
+#define huge_ptep_set_access_flags(vma, addr, ptep, pte, dirty)	\
+	ptep_set_access_flags(vma, addr, ptep, pte, dirty)
+#define huge_ptep_get(ptep)			(*ptep)
+
+#define arch_prepare_hugepage(page)		0
+#define arch_release_hugepage(page)		do { } while (0)
+
 #define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
 
 #endif /* _ASM_SPARC64_HUGETLB_H */
include/asm-x86/hugetlb.h +11 -0
Index: linux-2.6.25-rc7/include/asm-x86/hugetlb.h
===================================================================
@@ -25,6 +25,17 @@ static inline int prepare_hugepage_range
 
 #define hugetlb_prefault_arch_hook(mm)		do { } while (0)
 
+#define huge_pte_none(pte)			pte_none(pte)
+#define huge_pte_wrprotect(pte)			pte_wrprotect(pte)
+#define huge_ptep_set_wrprotect(mm, addr, ptep)	\
+	ptep_set_wrprotect(mm, addr, ptep)
+#define huge_ptep_set_access_flags(vma, addr, ptep, pte, dirty)	\
+	ptep_set_access_flags(vma, addr, ptep, pte, dirty)
+#define huge_ptep_get(ptep)			(*ptep)
+
+#define arch_prepare_hugepage(page)		0
+#define arch_release_hugepage(page)		do { } while (0)
+
 #define huge_ptep_clear_flush(vma, addr, ptep)	do { } while (0)
 
 #endif /* _ASM_X86_HUGETLB_H */