Generic Linux architectural discussions
 help / color / mirror / Atom feed
* [PATCH 10/10] arch: fix has_transparent_hugepage()
       [not found] <alpine.LSU.2.11.1604051329480.5965@eggly.anvils>
@ 2016-04-05 21:02 ` Hugh Dickins
  2016-04-05 21:02   ` Hugh Dickins
                     ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Hugh Dickins @ 2016-04-05 21:02 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kirill A. Shutemov, Andrea Arcangeli, Andres Lagar-Cavilla,
	Yang Shi, Ning Qu, Arnd Bergman, Ralf Baechle, Vineet Gupta,
	Russell King, Will Deacon, Michael Ellerman, Aneesh Kumar K.V,
	Martin Schwidefsky, Gerald Schaefer, David Miller, Chris Metcalf,
	Ingo Molnar, linux-kernel, linux-arch, linux-mm

I've just discovered that the useful-sounding has_transparent_hugepage()
is actually an architecture-dependent minefield: on some arches it only
builds if CONFIG_TRANSPARENT_HUGEPAGE=y, on others it's also there when
not, but on some of those (arm and arm64) it then gives the wrong answer;
and on mips alone it's marked __init, which would crash if called later
(but so far it has not been called later).

Straighten this out: make it available to all configs, with a sensible
default in asm-generic/pgtable.h, removing its definitions from those
arches (arc, arm, arm64, sparc, tile) which are served by the default,
adding #define has_transparent_hugepage has_transparent_hugepage to those
(mips, powerpc, s390, x86) which need to override the default at runtime,
and removing the __init from mips (but maybe that kind of code should be
avoided after init: set a static variable the first time it's called).

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 arch/arc/include/asm/hugepage.h              |    2 -
 arch/arm/include/asm/pgtable-3level.h        |    5 ----
 arch/arm64/include/asm/pgtable.h             |    5 ----
 arch/mips/include/asm/pgtable.h              |    1 
 arch/mips/mm/tlb-r4k.c                       |   21 ++++++++---------
 arch/powerpc/include/asm/book3s/64/pgtable.h |    1 
 arch/powerpc/include/asm/pgtable.h           |    1 
 arch/s390/include/asm/pgtable.h              |    1 
 arch/sparc/include/asm/pgtable_64.h          |    2 -
 arch/tile/include/asm/pgtable.h              |    1 
 arch/x86/include/asm/pgtable.h               |    1 
 include/asm-generic/pgtable.h                |    8 ++++++
 12 files changed, 23 insertions(+), 26 deletions(-)

--- a/arch/arc/include/asm/hugepage.h
+++ b/arch/arc/include/asm/hugepage.h
@@ -61,8 +61,6 @@ static inline void set_pmd_at(struct mm_
 extern void update_mmu_cache_pmd(struct vm_area_struct *vma, unsigned long addr,
 				 pmd_t *pmd);
 
-#define has_transparent_hugepage() 1
-
 /* Generic variants assume pgtable_t is struct page *, hence need for these */
 #define __HAVE_ARCH_PGTABLE_DEPOSIT
 extern void pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -281,11 +281,6 @@ static inline void set_pmd_at(struct mm_
 	flush_pmd_entry(pmdp);
 }
 
-static inline int has_transparent_hugepage(void)
-{
-	return 1;
-}
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_PGTABLE_3LEVEL_H */
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -304,11 +304,6 @@ static inline pgprot_t mk_sect_prot(pgpr
 
 #define set_pmd_at(mm, addr, pmdp, pmd)	set_pte_at(mm, addr, (pte_t *)pmdp, pmd_pte(pmd))
 
-static inline int has_transparent_hugepage(void)
-{
-	return 1;
-}
-
 #define __pgprot_modify(prot,mask,bits) \
 	__pgprot((pgprot_val(prot) & ~(mask)) | (bits))
 
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -468,6 +468,7 @@ static inline int io_remap_pfn_range(str
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
+#define has_transparent_hugepage has_transparent_hugepage
 extern int has_transparent_hugepage(void);
 
 static inline int pmd_trans_huge(pmd_t pmd)
--- a/arch/mips/mm/tlb-r4k.c
+++ b/arch/mips/mm/tlb-r4k.c
@@ -399,19 +399,20 @@ void add_wired_entry(unsigned long entry
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-int __init has_transparent_hugepage(void)
+int has_transparent_hugepage(void)
 {
-	unsigned int mask;
-	unsigned long flags;
+	static unsigned int mask = -1;
 
-	local_irq_save(flags);
-	write_c0_pagemask(PM_HUGE_MASK);
-	back_to_back_c0_hazard();
-	mask = read_c0_pagemask();
-	write_c0_pagemask(PM_DEFAULT_MASK);
-
-	local_irq_restore(flags);
+	if (mask == -1) {	/* first call comes during __init */
+		unsigned long flags;
 
+		local_irq_save(flags);
+		write_c0_pagemask(PM_HUGE_MASK);
+		back_to_back_c0_hazard();
+		mask = read_c0_pagemask();
+		write_c0_pagemask(PM_DEFAULT_MASK);
+		local_irq_restore(flags);
+	}
 	return mask == PM_HUGE_MASK;
 }
 
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -219,6 +219,7 @@ extern void set_pmd_at(struct mm_struct
 		       pmd_t *pmdp, pmd_t pmd);
 extern void update_mmu_cache_pmd(struct vm_area_struct *vma, unsigned long addr,
 				 pmd_t *pmd);
+#define has_transparent_hugepage has_transparent_hugepage
 extern int has_transparent_hugepage(void);
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -65,7 +65,6 @@ extern int gup_hugepte(pte_t *ptep, unsi
 		       struct page **pages, int *nr);
 #ifndef CONFIG_TRANSPARENT_HUGEPAGE
 #define pmd_large(pmd)		0
-#define has_transparent_hugepage() 0
 #endif
 pte_t *__find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
 				   bool *is_thp, unsigned *shift);
--- a/arch/s390/include/asm/pgtable.h
+++ b/arch/s390/include/asm/pgtable.h
@@ -1223,6 +1223,7 @@ static inline int pmd_trans_huge(pmd_t p
 	return pmd_val(pmd) & _SEGMENT_ENTRY_LARGE;
 }
 
+#define has_transparent_hugepage has_transparent_hugepage
 static inline int has_transparent_hugepage(void)
 {
 	return MACHINE_HAS_HPAGE ? 1 : 0;
--- a/arch/sparc/include/asm/pgtable_64.h
+++ b/arch/sparc/include/asm/pgtable_64.h
@@ -681,8 +681,6 @@ static inline unsigned long pmd_trans_hu
 	return pte_val(pte) & _PAGE_PMD_HUGE;
 }
 
-#define has_transparent_hugepage() 1
-
 static inline pmd_t pmd_mkold(pmd_t pmd)
 {
 	pte_t pte = __pte(pmd_val(pmd));
--- a/arch/tile/include/asm/pgtable.h
+++ b/arch/tile/include/asm/pgtable.h
@@ -487,7 +487,6 @@ static inline pmd_t pmd_modify(pmd_t pmd
 }
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-#define has_transparent_hugepage() 1
 #define pmd_trans_huge pmd_huge_page
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -181,6 +181,7 @@ static inline int pmd_trans_huge(pmd_t p
 	return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE;
 }
 
+#define has_transparent_hugepage has_transparent_hugepage
 static inline int has_transparent_hugepage(void)
 {
 	return cpu_has_pse;
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -806,4 +806,12 @@ static inline int pmd_clear_huge(pmd_t *
 #define io_remap_pfn_range remap_pfn_range
 #endif
 
+#ifndef has_transparent_hugepage
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+#define has_transparent_hugepage() 1
+#else
+#define has_transparent_hugepage() 0
+#endif
+#endif
+
 #endif /* _ASM_GENERIC_PGTABLE_H */

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-05 21:02 ` [PATCH 10/10] arch: fix has_transparent_hugepage() Hugh Dickins
@ 2016-04-05 21:02   ` Hugh Dickins
  2016-04-05 23:25   ` David Miller
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Hugh Dickins @ 2016-04-05 21:02 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kirill A. Shutemov, Andrea Arcangeli, Andres Lagar-Cavilla,
	Yang Shi, Ning Qu, Arnd Bergman, Ralf Baechle, Vineet Gupta,
	Russell King, Will Deacon, Michael Ellerman, Aneesh Kumar K.V,
	Martin Schwidefsky, Gerald Schaefer, David Miller, Chris Metcalf,
	Ingo Molnar, linux-kernel, linux-arch, linux-mm

I've just discovered that the useful-sounding has_transparent_hugepage()
is actually an architecture-dependent minefield: on some arches it only
builds if CONFIG_TRANSPARENT_HUGEPAGE=y, on others it's also there when
not, but on some of those (arm and arm64) it then gives the wrong answer;
and on mips alone it's marked __init, which would crash if called later
(but so far it has not been called later).

Straighten this out: make it available to all configs, with a sensible
default in asm-generic/pgtable.h, removing its definitions from those
arches (arc, arm, arm64, sparc, tile) which are served by the default,
adding #define has_transparent_hugepage has_transparent_hugepage to those
(mips, powerpc, s390, x86) which need to override the default at runtime,
and removing the __init from mips (but maybe that kind of code should be
avoided after init: set a static variable the first time it's called).

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 arch/arc/include/asm/hugepage.h              |    2 -
 arch/arm/include/asm/pgtable-3level.h        |    5 ----
 arch/arm64/include/asm/pgtable.h             |    5 ----
 arch/mips/include/asm/pgtable.h              |    1 
 arch/mips/mm/tlb-r4k.c                       |   21 ++++++++---------
 arch/powerpc/include/asm/book3s/64/pgtable.h |    1 
 arch/powerpc/include/asm/pgtable.h           |    1 
 arch/s390/include/asm/pgtable.h              |    1 
 arch/sparc/include/asm/pgtable_64.h          |    2 -
 arch/tile/include/asm/pgtable.h              |    1 
 arch/x86/include/asm/pgtable.h               |    1 
 include/asm-generic/pgtable.h                |    8 ++++++
 12 files changed, 23 insertions(+), 26 deletions(-)

--- a/arch/arc/include/asm/hugepage.h
+++ b/arch/arc/include/asm/hugepage.h
@@ -61,8 +61,6 @@ static inline void set_pmd_at(struct mm_
 extern void update_mmu_cache_pmd(struct vm_area_struct *vma, unsigned long addr,
 				 pmd_t *pmd);
 
-#define has_transparent_hugepage() 1
-
 /* Generic variants assume pgtable_t is struct page *, hence need for these */
 #define __HAVE_ARCH_PGTABLE_DEPOSIT
 extern void pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -281,11 +281,6 @@ static inline void set_pmd_at(struct mm_
 	flush_pmd_entry(pmdp);
 }
 
-static inline int has_transparent_hugepage(void)
-{
-	return 1;
-}
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_PGTABLE_3LEVEL_H */
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -304,11 +304,6 @@ static inline pgprot_t mk_sect_prot(pgpr
 
 #define set_pmd_at(mm, addr, pmdp, pmd)	set_pte_at(mm, addr, (pte_t *)pmdp, pmd_pte(pmd))
 
-static inline int has_transparent_hugepage(void)
-{
-	return 1;
-}
-
 #define __pgprot_modify(prot,mask,bits) \
 	__pgprot((pgprot_val(prot) & ~(mask)) | (bits))
 
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -468,6 +468,7 @@ static inline int io_remap_pfn_range(str
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
+#define has_transparent_hugepage has_transparent_hugepage
 extern int has_transparent_hugepage(void);
 
 static inline int pmd_trans_huge(pmd_t pmd)
--- a/arch/mips/mm/tlb-r4k.c
+++ b/arch/mips/mm/tlb-r4k.c
@@ -399,19 +399,20 @@ void add_wired_entry(unsigned long entry
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-int __init has_transparent_hugepage(void)
+int has_transparent_hugepage(void)
 {
-	unsigned int mask;
-	unsigned long flags;
+	static unsigned int mask = -1;
 
-	local_irq_save(flags);
-	write_c0_pagemask(PM_HUGE_MASK);
-	back_to_back_c0_hazard();
-	mask = read_c0_pagemask();
-	write_c0_pagemask(PM_DEFAULT_MASK);
-
-	local_irq_restore(flags);
+	if (mask == -1) {	/* first call comes during __init */
+		unsigned long flags;
 
+		local_irq_save(flags);
+		write_c0_pagemask(PM_HUGE_MASK);
+		back_to_back_c0_hazard();
+		mask = read_c0_pagemask();
+		write_c0_pagemask(PM_DEFAULT_MASK);
+		local_irq_restore(flags);
+	}
 	return mask == PM_HUGE_MASK;
 }
 
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -219,6 +219,7 @@ extern void set_pmd_at(struct mm_struct
 		       pmd_t *pmdp, pmd_t pmd);
 extern void update_mmu_cache_pmd(struct vm_area_struct *vma, unsigned long addr,
 				 pmd_t *pmd);
+#define has_transparent_hugepage has_transparent_hugepage
 extern int has_transparent_hugepage(void);
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -65,7 +65,6 @@ extern int gup_hugepte(pte_t *ptep, unsi
 		       struct page **pages, int *nr);
 #ifndef CONFIG_TRANSPARENT_HUGEPAGE
 #define pmd_large(pmd)		0
-#define has_transparent_hugepage() 0
 #endif
 pte_t *__find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
 				   bool *is_thp, unsigned *shift);
--- a/arch/s390/include/asm/pgtable.h
+++ b/arch/s390/include/asm/pgtable.h
@@ -1223,6 +1223,7 @@ static inline int pmd_trans_huge(pmd_t p
 	return pmd_val(pmd) & _SEGMENT_ENTRY_LARGE;
 }
 
+#define has_transparent_hugepage has_transparent_hugepage
 static inline int has_transparent_hugepage(void)
 {
 	return MACHINE_HAS_HPAGE ? 1 : 0;
--- a/arch/sparc/include/asm/pgtable_64.h
+++ b/arch/sparc/include/asm/pgtable_64.h
@@ -681,8 +681,6 @@ static inline unsigned long pmd_trans_hu
 	return pte_val(pte) & _PAGE_PMD_HUGE;
 }
 
-#define has_transparent_hugepage() 1
-
 static inline pmd_t pmd_mkold(pmd_t pmd)
 {
 	pte_t pte = __pte(pmd_val(pmd));
--- a/arch/tile/include/asm/pgtable.h
+++ b/arch/tile/include/asm/pgtable.h
@@ -487,7 +487,6 @@ static inline pmd_t pmd_modify(pmd_t pmd
 }
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-#define has_transparent_hugepage() 1
 #define pmd_trans_huge pmd_huge_page
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -181,6 +181,7 @@ static inline int pmd_trans_huge(pmd_t p
 	return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE;
 }
 
+#define has_transparent_hugepage has_transparent_hugepage
 static inline int has_transparent_hugepage(void)
 {
 	return cpu_has_pse;
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -806,4 +806,12 @@ static inline int pmd_clear_huge(pmd_t *
 #define io_remap_pfn_range remap_pfn_range
 #endif
 
+#ifndef has_transparent_hugepage
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+#define has_transparent_hugepage() 1
+#else
+#define has_transparent_hugepage() 0
+#endif
+#endif
+
 #endif /* _ASM_GENERIC_PGTABLE_H */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-05 21:02 ` [PATCH 10/10] arch: fix has_transparent_hugepage() Hugh Dickins
  2016-04-05 21:02   ` Hugh Dickins
@ 2016-04-05 23:25   ` David Miller
  2016-04-05 23:25     ` David Miller
  2016-04-06  3:22   ` Vineet Gupta
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2016-04-05 23:25 UTC (permalink / raw)
  To: hughd
  Cc: akpm, kirill.shutemov, aarcange, andreslc, yang.shi, quning, arnd,
	ralf, vgupta, linux, will.deacon, mpe, aneesh.kumar, schwidefsky,
	gerald.schaefer, cmetcalf, mingo, linux-kernel, linux-arch,
	linux-mm

From: Hugh Dickins <hughd@google.com>
Date: Tue, 5 Apr 2016 14:02:49 -0700 (PDT)

> I've just discovered that the useful-sounding has_transparent_hugepage()
> is actually an architecture-dependent minefield: on some arches it only
> builds if CONFIG_TRANSPARENT_HUGEPAGE=y, on others it's also there when
> not, but on some of those (arm and arm64) it then gives the wrong answer;
> and on mips alone it's marked __init, which would crash if called later
> (but so far it has not been called later).
> 
> Straighten this out: make it available to all configs, with a sensible
> default in asm-generic/pgtable.h, removing its definitions from those
> arches (arc, arm, arm64, sparc, tile) which are served by the default,
> adding #define has_transparent_hugepage has_transparent_hugepage to those
> (mips, powerpc, s390, x86) which need to override the default at runtime,
> and removing the __init from mips (but maybe that kind of code should be
> avoided after init: set a static variable the first time it's called).
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>

Acked-by: David S. Miller <davem@davemloft.net>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-05 23:25   ` David Miller
@ 2016-04-05 23:25     ` David Miller
  0 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2016-04-05 23:25 UTC (permalink / raw)
  To: hughd
  Cc: akpm, kirill.shutemov, aarcange, andreslc, yang.shi, quning, arnd,
	ralf, vgupta, linux, will.deacon, mpe, aneesh.kumar, schwidefsky,
	gerald.schaefer, cmetcalf, mingo, linux-kernel, linux-arch,
	linux-mm

From: Hugh Dickins <hughd@google.com>
Date: Tue, 5 Apr 2016 14:02:49 -0700 (PDT)

> I've just discovered that the useful-sounding has_transparent_hugepage()
> is actually an architecture-dependent minefield: on some arches it only
> builds if CONFIG_TRANSPARENT_HUGEPAGE=y, on others it's also there when
> not, but on some of those (arm and arm64) it then gives the wrong answer;
> and on mips alone it's marked __init, which would crash if called later
> (but so far it has not been called later).
> 
> Straighten this out: make it available to all configs, with a sensible
> default in asm-generic/pgtable.h, removing its definitions from those
> arches (arc, arm, arm64, sparc, tile) which are served by the default,
> adding #define has_transparent_hugepage has_transparent_hugepage to those
> (mips, powerpc, s390, x86) which need to override the default at runtime,
> and removing the __init from mips (but maybe that kind of code should be
> avoided after init: set a static variable the first time it's called).
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-05 21:02 ` [PATCH 10/10] arch: fix has_transparent_hugepage() Hugh Dickins
  2016-04-05 21:02   ` Hugh Dickins
  2016-04-05 23:25   ` David Miller
@ 2016-04-06  3:22   ` Vineet Gupta
  2016-04-06  6:58   ` Ingo Molnar
  2016-04-06 11:56   ` Gerald Schaefer
  4 siblings, 0 replies; 11+ messages in thread
From: Vineet Gupta @ 2016-04-06  3:22 UTC (permalink / raw)
  To: Hugh Dickins, Andrew Morton
  Cc: Kirill A. Shutemov, Andrea Arcangeli, Andres Lagar-Cavilla,
	Yang Shi, Ning Qu, Arnd Bergman, Ralf Baechle, Russell King,
	Will Deacon, Michael Ellerman, Aneesh Kumar K.V,
	Martin Schwidefsky, Gerald Schaefer, David Miller, Chris Metcalf,
	Ingo Molnar, linux-kernel@vger.kernel.org,
	linux-arch@vger.kernel.org, linux-mm@kvack.org

On Wednesday 06 April 2016 02:32 AM, Hugh Dickins wrote:
> I've just discovered that the useful-sounding has_transparent_hugepage()
> is actually an architecture-dependent minefield: on some arches it only
> builds if CONFIG_TRANSPARENT_HUGEPAGE=y, on others it's also there when
> not, but on some of those (arm and arm64) it then gives the wrong answer;
> and on mips alone it's marked __init, which would crash if called later
> (but so far it has not been called later).
>
> Straighten this out: make it available to all configs, with a sensible
> default in asm-generic/pgtable.h, removing its definitions from those
> arches (arc, arm, arm64, sparc, tile) which are served by the default,
> adding #define has_transparent_hugepage has_transparent_hugepage to those
> (mips, powerpc, s390, x86) which need to override the default at runtime,
> and removing the __init from mips (but maybe that kind of code should be
> avoided after init: set a static variable the first time it's called).
>
> Signed-off-by: Hugh Dickins <hughd@google.com>

Acked-by: Vineet Gupta <vgupta@synopsys.com> # for arch/arc bits

Thx,
-Vineet

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-05 21:02 ` [PATCH 10/10] arch: fix has_transparent_hugepage() Hugh Dickins
                     ` (2 preceding siblings ...)
  2016-04-06  3:22   ` Vineet Gupta
@ 2016-04-06  6:58   ` Ingo Molnar
  2016-04-06  6:58     ` Ingo Molnar
  2016-04-06 12:29     ` Chris Metcalf
  2016-04-06 11:56   ` Gerald Schaefer
  4 siblings, 2 replies; 11+ messages in thread
From: Ingo Molnar @ 2016-04-06  6:58 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Andrew Morton, Kirill A. Shutemov, Andrea Arcangeli,
	Andres Lagar-Cavilla, Yang Shi, Ning Qu, Arnd Bergman,
	Ralf Baechle, Vineet Gupta, Russell King, Will Deacon,
	Michael Ellerman, Aneesh Kumar K.V, Martin Schwidefsky,
	Gerald Schaefer, David Miller, Chris Metcalf, linux-kernel,
	linux-arch, linux-mm


* Hugh Dickins <hughd@google.com> wrote:

> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -181,6 +181,7 @@ static inline int pmd_trans_huge(pmd_t p
>  	return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE;
>  }
>  
> +#define has_transparent_hugepage has_transparent_hugepage
>  static inline int has_transparent_hugepage(void)
>  {
>  	return cpu_has_pse;

Small nit, just writing:

  #define has_transparent_hugepage

ought to be enough, right?

In any case:

  Acked-by: Ingo Molnar <mingo@kernel.org>

Another nit, this:

> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -806,4 +806,12 @@ static inline int pmd_clear_huge(pmd_t *
>  #define io_remap_pfn_range remap_pfn_range
>  #endif
>  
> +#ifndef has_transparent_hugepage
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +#define has_transparent_hugepage() 1
> +#else
> +#define has_transparent_hugepage() 0
> +#endif
> +#endif

Looks a bit more structured as:

  #ifndef has_transparent_hugepage
  # ifdef CONFIG_TRANSPARENT_HUGEPAGE
  #  define has_transparent_hugepage() 1
  # else
  #  define has_transparent_hugepage() 0
  # endif
  #endif

BYMMV.

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-06  6:58   ` Ingo Molnar
@ 2016-04-06  6:58     ` Ingo Molnar
  2016-04-06 12:29     ` Chris Metcalf
  1 sibling, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2016-04-06  6:58 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Andrew Morton, Kirill A. Shutemov, Andrea Arcangeli,
	Andres Lagar-Cavilla, Yang Shi, Ning Qu, Arnd Bergman,
	Ralf Baechle, Vineet Gupta, Russell King, Will Deacon,
	Michael Ellerman, Aneesh Kumar K.V, Martin Schwidefsky,
	Gerald Schaefer, David Miller, Chris Metcalf, linux-kernel,
	linux-arch, linux-mm


* Hugh Dickins <hughd@google.com> wrote:

> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -181,6 +181,7 @@ static inline int pmd_trans_huge(pmd_t p
>  	return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE;
>  }
>  
> +#define has_transparent_hugepage has_transparent_hugepage
>  static inline int has_transparent_hugepage(void)
>  {
>  	return cpu_has_pse;

Small nit, just writing:

  #define has_transparent_hugepage

ought to be enough, right?

In any case:

  Acked-by: Ingo Molnar <mingo@kernel.org>

Another nit, this:

> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -806,4 +806,12 @@ static inline int pmd_clear_huge(pmd_t *
>  #define io_remap_pfn_range remap_pfn_range
>  #endif
>  
> +#ifndef has_transparent_hugepage
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +#define has_transparent_hugepage() 1
> +#else
> +#define has_transparent_hugepage() 0
> +#endif
> +#endif

Looks a bit more structured as:

  #ifndef has_transparent_hugepage
  # ifdef CONFIG_TRANSPARENT_HUGEPAGE
  #  define has_transparent_hugepage() 1
  # else
  #  define has_transparent_hugepage() 0
  # endif
  #endif

BYMMV.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-05 21:02 ` [PATCH 10/10] arch: fix has_transparent_hugepage() Hugh Dickins
                     ` (3 preceding siblings ...)
  2016-04-06  6:58   ` Ingo Molnar
@ 2016-04-06 11:56   ` Gerald Schaefer
  2016-04-06 11:56     ` Gerald Schaefer
  4 siblings, 1 reply; 11+ messages in thread
From: Gerald Schaefer @ 2016-04-06 11:56 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Andrew Morton, Kirill A. Shutemov, Andrea Arcangeli,
	Andres Lagar-Cavilla, Yang Shi, Ning Qu, Arnd Bergman,
	Ralf Baechle, Vineet Gupta, Russell King, Will Deacon,
	Michael Ellerman, Aneesh Kumar K.V, Martin Schwidefsky,
	David Miller, Chris Metcalf, Ingo Molnar, linux-kernel,
	linux-arch, linux-mm

On Tue, 5 Apr 2016 14:02:49 -0700 (PDT)
Hugh Dickins <hughd@google.com> wrote:

> I've just discovered that the useful-sounding has_transparent_hugepage()
> is actually an architecture-dependent minefield: on some arches it only
> builds if CONFIG_TRANSPARENT_HUGEPAGE=y, on others it's also there when
> not, but on some of those (arm and arm64) it then gives the wrong answer;
> and on mips alone it's marked __init, which would crash if called later
> (but so far it has not been called later).
> 
> Straighten this out: make it available to all configs, with a sensible
> default in asm-generic/pgtable.h, removing its definitions from those
> arches (arc, arm, arm64, sparc, tile) which are served by the default,
> adding #define has_transparent_hugepage has_transparent_hugepage to those
> (mips, powerpc, s390, x86) which need to override the default at runtime,
> and removing the __init from mips (but maybe that kind of code should be
> avoided after init: set a static variable the first time it's called).
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> ---

Acked-by: Gerald Schaefer <gerald.schaefer@de.ibm.com> # for arch/s390 bits

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-06 11:56   ` Gerald Schaefer
@ 2016-04-06 11:56     ` Gerald Schaefer
  0 siblings, 0 replies; 11+ messages in thread
From: Gerald Schaefer @ 2016-04-06 11:56 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Andrew Morton, Kirill A. Shutemov, Andrea Arcangeli,
	Andres Lagar-Cavilla, Yang Shi, Ning Qu, Arnd Bergman,
	Ralf Baechle, Vineet Gupta, Russell King, Will Deacon,
	Michael Ellerman, Aneesh Kumar K.V, Martin Schwidefsky,
	David Miller, Chris Metcalf, Ingo Molnar, linux-kernel,
	linux-arch, linux-mm

On Tue, 5 Apr 2016 14:02:49 -0700 (PDT)
Hugh Dickins <hughd@google.com> wrote:

> I've just discovered that the useful-sounding has_transparent_hugepage()
> is actually an architecture-dependent minefield: on some arches it only
> builds if CONFIG_TRANSPARENT_HUGEPAGE=y, on others it's also there when
> not, but on some of those (arm and arm64) it then gives the wrong answer;
> and on mips alone it's marked __init, which would crash if called later
> (but so far it has not been called later).
> 
> Straighten this out: make it available to all configs, with a sensible
> default in asm-generic/pgtable.h, removing its definitions from those
> arches (arc, arm, arm64, sparc, tile) which are served by the default,
> adding #define has_transparent_hugepage has_transparent_hugepage to those
> (mips, powerpc, s390, x86) which need to override the default at runtime,
> and removing the __init from mips (but maybe that kind of code should be
> avoided after init: set a static variable the first time it's called).
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> ---

Acked-by: Gerald Schaefer <gerald.schaefer@de.ibm.com> # for arch/s390 bits


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-06  6:58   ` Ingo Molnar
  2016-04-06  6:58     ` Ingo Molnar
@ 2016-04-06 12:29     ` Chris Metcalf
  2016-04-06 21:58       ` Ingo Molnar
  1 sibling, 1 reply; 11+ messages in thread
From: Chris Metcalf @ 2016-04-06 12:29 UTC (permalink / raw)
  To: Ingo Molnar, Hugh Dickins
  Cc: Andrew Morton, Kirill A. Shutemov, Andrea Arcangeli,
	Andres Lagar-Cavilla, Yang Shi, Ning Qu, Arnd Bergman,
	Ralf Baechle, Vineet Gupta, Russell King, Will Deacon,
	Michael Ellerman, Aneesh Kumar K.V, Martin Schwidefsky,
	Gerald Schaefer, David Miller, linux-kernel, linux-arch, linux-mm

On 4/6/2016 2:58 AM, Ingo Molnar wrote:
> * Hugh Dickins <hughd@google.com> wrote:
>
>> --- a/arch/x86/include/asm/pgtable.h
>> +++ b/arch/x86/include/asm/pgtable.h
>> @@ -181,6 +181,7 @@ static inline int pmd_trans_huge(pmd_t p
>>   	return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE;
>>   }
>>   
>> +#define has_transparent_hugepage has_transparent_hugepage
>>   static inline int has_transparent_hugepage(void)
>>   {
>>   	return cpu_has_pse;
> Small nit, just writing:
>
>    #define has_transparent_hugepage
>
> ought to be enough, right?

No, since then in hugepage_init() the has_transparent_hugepage() call site
would be left with just a stray pair of parentheses instead of a call.

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 10/10] arch: fix has_transparent_hugepage()
  2016-04-06 12:29     ` Chris Metcalf
@ 2016-04-06 21:58       ` Ingo Molnar
  0 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2016-04-06 21:58 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Hugh Dickins, Andrew Morton, Kirill A. Shutemov, Andrea Arcangeli,
	Andres Lagar-Cavilla, Yang Shi, Ning Qu, Arnd Bergman,
	Ralf Baechle, Vineet Gupta, Russell King, Will Deacon,
	Michael Ellerman, Aneesh Kumar K.V, Martin Schwidefsky,
	Gerald Schaefer, David Miller, linux-kernel, linux-arch, linux-mm


* Chris Metcalf <cmetcalf@mellanox.com> wrote:

> On 4/6/2016 2:58 AM, Ingo Molnar wrote:
> >* Hugh Dickins <hughd@google.com> wrote:
> >
> >>--- a/arch/x86/include/asm/pgtable.h
> >>+++ b/arch/x86/include/asm/pgtable.h
> >>@@ -181,6 +181,7 @@ static inline int pmd_trans_huge(pmd_t p
> >>  	return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE;
> >>  }
> >>+#define has_transparent_hugepage has_transparent_hugepage
> >>  static inline int has_transparent_hugepage(void)
> >>  {
> >>  	return cpu_has_pse;
> >Small nit, just writing:
> >
> >   #define has_transparent_hugepage
> >
> >ought to be enough, right?
> 
> No, since then in hugepage_init() the has_transparent_hugepage() call site
> would be left with just a stray pair of parentheses instead of a call.

Yes, indeed ...

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-04-06 21:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <alpine.LSU.2.11.1604051329480.5965@eggly.anvils>
2016-04-05 21:02 ` [PATCH 10/10] arch: fix has_transparent_hugepage() Hugh Dickins
2016-04-05 21:02   ` Hugh Dickins
2016-04-05 23:25   ` David Miller
2016-04-05 23:25     ` David Miller
2016-04-06  3:22   ` Vineet Gupta
2016-04-06  6:58   ` Ingo Molnar
2016-04-06  6:58     ` Ingo Molnar
2016-04-06 12:29     ` Chris Metcalf
2016-04-06 21:58       ` Ingo Molnar
2016-04-06 11:56   ` Gerald Schaefer
2016-04-06 11:56     ` Gerald Schaefer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox