All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
@ 2021-05-02  4:56 Nicholas Piggin
  2021-05-02  7:34 ` Christophe Leroy
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Nicholas Piggin @ 2021-05-02  4:56 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

This reduces TLB misses by nearly 30x on a `git diff` workload on a
2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
to vfs hashes being allocated with 2MB pages.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
This was in the -mm tree but was dropped at the last minute after
clashing with a patch in powerpc next.

Now all prerequisites are upstream, this can be merged as is. Probably
makes sense now to go via powerpc tree.

This is rebased and retested on upstream.

 Documentation/admin-guide/kernel-parameters.txt |  2 ++
 arch/powerpc/Kconfig                            |  1 +
 arch/powerpc/include/asm/pgtable.h              |  5 +++++
 arch/powerpc/kernel/module.c                    | 16 +++++++++++++---
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1c0a3cf6fcc9..1be38b25c485 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3250,6 +3250,8 @@
 
 	nohugeiomap	[KNL,X86,PPC,ARM64] Disable kernel huge I/O mappings.
 
+	nohugevmalloc	[PPC] Disable kernel huge vmalloc mappings.
+
 	nosmt		[KNL,S390] Disable symmetric multithreading (SMT).
 			Equivalent to smt=1.
 
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1e6230bea09d..c547a9d6a2dd 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -185,6 +185,7 @@ config PPC
 	select GENERIC_VDSO_TIME_NS
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_HUGE_VMAP		if PPC_BOOK3S_64 && PPC_RADIX_MMU
+	select HAVE_ARCH_HUGE_VMALLOC		if HAVE_ARCH_HUGE_VMAP
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_JUMP_LABEL_RELATIVE
 	select HAVE_ARCH_KASAN			if PPC32 && PPC_PAGE_SHIFT <= 14
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index c6a676714f04..1678e4b08fc3 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -39,6 +39,11 @@ struct mm_struct;
 #define __S110	PAGE_SHARED_X
 #define __S111	PAGE_SHARED_X
 
+#ifndef MODULES_VADDR
+#define MODULES_VADDR	VMALLOC_START
+#define MODULES_END	VMALLOC_END
+#endif
+
 #ifndef __ASSEMBLY__
 
 /* Keep these as a macros to avoid include dependency mess */
diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
index fab84024650c..77aefcbbd276 100644
--- a/arch/powerpc/kernel/module.c
+++ b/arch/powerpc/kernel/module.c
@@ -8,6 +8,7 @@
 #include <linux/moduleloader.h>
 #include <linux/err.h>
 #include <linux/vmalloc.h>
+#include <linux/mm.h>
 #include <linux/bug.h>
 #include <asm/module.h>
 #include <linux/uaccess.h>
@@ -88,17 +89,24 @@ int module_finalize(const Elf_Ehdr *hdr,
 	return 0;
 }
 
-#ifdef MODULES_VADDR
 static __always_inline void *
 __module_alloc(unsigned long size, unsigned long start, unsigned long end)
 {
+	/*
+	 * Don't do huge page allocations for modules yet until more testing
+	 * is done. STRICT_MODULE_RWX may require extra work to support this
+	 * too.
+	 */
 	return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
-				    PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
+				    PAGE_KERNEL_EXEC,
+				    VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
+				    NUMA_NO_NODE,
 				    __builtin_return_address(0));
 }
 
 void *module_alloc(unsigned long size)
 {
+#ifdef CONFIG_PPC32
 	unsigned long limit = (unsigned long)_etext - SZ_32M;
 	void *ptr = NULL;
 
@@ -112,5 +120,7 @@ void *module_alloc(unsigned long size)
 		ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
 
 	return ptr;
-}
+#else
+	return __module_alloc(size, MODULES_VADDR, MODULES_END);
 #endif
+}
-- 
2.23.0


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

* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
  2021-05-02  4:56 [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings Nicholas Piggin
@ 2021-05-02  7:34 ` Christophe Leroy
  2021-05-02 10:41   ` Nicholas Piggin
  2021-05-02  8:03   ` kernel test robot
  2021-05-02  8:03   ` kernel test robot
  2 siblings, 1 reply; 7+ messages in thread
From: Christophe Leroy @ 2021-05-02  7:34 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev



Le 02/05/2021 à 06:56, Nicholas Piggin a écrit :
> This reduces TLB misses by nearly 30x on a `git diff` workload on a
> 2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
> to vfs hashes being allocated with 2MB pages.
> 
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> This was in the -mm tree but was dropped at the last minute after
> clashing with a patch in powerpc next.
> 
> Now all prerequisites are upstream, this can be merged as is. Probably
> makes sense now to go via powerpc tree.
> 
> This is rebased and retested on upstream.
> 
>   Documentation/admin-guide/kernel-parameters.txt |  2 ++
>   arch/powerpc/Kconfig                            |  1 +
>   arch/powerpc/include/asm/pgtable.h              |  5 +++++
>   arch/powerpc/kernel/module.c                    | 16 +++++++++++++---
>   4 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1c0a3cf6fcc9..1be38b25c485 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3250,6 +3250,8 @@
>   
>   	nohugeiomap	[KNL,X86,PPC,ARM64] Disable kernel huge I/O mappings.
>   
> +	nohugevmalloc	[PPC] Disable kernel huge vmalloc mappings.
> +
>   	nosmt		[KNL,S390] Disable symmetric multithreading (SMT).
>   			Equivalent to smt=1.
>   
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 1e6230bea09d..c547a9d6a2dd 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -185,6 +185,7 @@ config PPC
>   	select GENERIC_VDSO_TIME_NS
>   	select HAVE_ARCH_AUDITSYSCALL
>   	select HAVE_ARCH_HUGE_VMAP		if PPC_BOOK3S_64 && PPC_RADIX_MMU
> +	select HAVE_ARCH_HUGE_VMALLOC		if HAVE_ARCH_HUGE_VMAP
>   	select HAVE_ARCH_JUMP_LABEL
>   	select HAVE_ARCH_JUMP_LABEL_RELATIVE
>   	select HAVE_ARCH_KASAN			if PPC32 && PPC_PAGE_SHIFT <= 14
> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index c6a676714f04..1678e4b08fc3 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -39,6 +39,11 @@ struct mm_struct;
>   #define __S110	PAGE_SHARED_X
>   #define __S111	PAGE_SHARED_X
>   
> +#ifndef MODULES_VADDR
> +#define MODULES_VADDR	VMALLOC_START
> +#define MODULES_END	VMALLOC_END
> +#endif

This will also require some changes in a few places, see 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210429031602.2606654-4-jniethe5@gmail.com/

> +
>   #ifndef __ASSEMBLY__
>   
>   /* Keep these as a macros to avoid include dependency mess */
> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> index fab84024650c..77aefcbbd276 100644
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -8,6 +8,7 @@
>   #include <linux/moduleloader.h>
>   #include <linux/err.h>
>   #include <linux/vmalloc.h>
> +#include <linux/mm.h>
>   #include <linux/bug.h>
>   #include <asm/module.h>
>   #include <linux/uaccess.h>
> @@ -88,17 +89,24 @@ int module_finalize(const Elf_Ehdr *hdr,
>   	return 0;
>   }
>   
> -#ifdef MODULES_VADDR
>   static __always_inline void *
>   __module_alloc(unsigned long size, unsigned long start, unsigned long end)
>   {
> +	/*
> +	 * Don't do huge page allocations for modules yet until more testing
> +	 * is done. STRICT_MODULE_RWX may require extra work to support this
> +	 * too.
> +	 */
>   	return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
> -				    PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
> +				    PAGE_KERNEL_EXEC,
> +				    VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
> +				    NUMA_NO_NODE,
>   				    __builtin_return_address(0));

Can we avoid so many lines ? Doesn't it fit on 3 lines now that 100 chars per line are tolerated ?

>   }
>   
>   void *module_alloc(unsigned long size)
>   {
> +#ifdef CONFIG_PPC32

Can we just add an IS_ENABLED(CONFIG_PPC32) in the 'if' instead of this #ifdef/#else ?

>   	unsigned long limit = (unsigned long)_etext - SZ_32M;
>   	void *ptr = NULL;
>   
> @@ -112,5 +120,7 @@ void *module_alloc(unsigned long size)
>   		ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>   
>   	return ptr;
> -}
> +#else
> +	return __module_alloc(size, MODULES_VADDR, MODULES_END);
>   #endif
> +}
> 

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

* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
  2021-05-02  4:56 [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings Nicholas Piggin
@ 2021-05-02  8:03   ` kernel test robot
  2021-05-02  8:03   ` kernel test robot
  2021-05-02  8:03   ` kernel test robot
  2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-05-02  8:03 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: kbuild-all, Nicholas Piggin

[-- Attachment #1: Type: text/plain, Size: 3162 bytes --]

Hi Nicholas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master next-20210430]
[cannot apply to v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r013-20210502 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1c8b90cff6679d7690891101efc07a1edd98e9a7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
        git checkout 1c8b90cff6679d7690891101efc07a1edd98e9a7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/nohash/mmu.h:16,
                    from arch/powerpc/include/asm/mmu.h:423,
                    from arch/powerpc/kernel/head_8xx.S:23:
>> arch/powerpc/include/asm/nohash/32/mmu-8xx.h:175: warning: "MODULES_VADDR" redefined
     175 | #define MODULES_VADDR (PAGE_OFFSET - SZ_256M)
         | 
   In file included from include/linux/pgtable.h:6,
                    from arch/powerpc/kernel/head_8xx.S:19:
   arch/powerpc/include/asm/pgtable.h:43: note: this is the location of the previous definition
      43 | #define MODULES_VADDR VMALLOC_START
         | 
   In file included from arch/powerpc/include/asm/nohash/mmu.h:16,
                    from arch/powerpc/include/asm/mmu.h:423,
                    from arch/powerpc/kernel/head_8xx.S:23:
>> arch/powerpc/include/asm/nohash/32/mmu-8xx.h:176: warning: "MODULES_END" redefined
     176 | #define MODULES_END PAGE_OFFSET
         | 
   In file included from include/linux/pgtable.h:6,
                    from arch/powerpc/kernel/head_8xx.S:19:
   arch/powerpc/include/asm/pgtable.h:44: note: this is the location of the previous definition
      44 | #define MODULES_END VMALLOC_END
         | 


vim +/MODULES_VADDR +175 arch/powerpc/include/asm/nohash/32/mmu-8xx.h

fca5c1e9eb5e26 Christophe Leroy 2019-04-25  174  
9132a2e82adc6e Christophe Leroy 2021-04-01 @175  #define MODULES_VADDR	(PAGE_OFFSET - SZ_256M)
9132a2e82adc6e Christophe Leroy 2021-04-01 @176  #define MODULES_END	PAGE_OFFSET
9132a2e82adc6e Christophe Leroy 2021-04-01  177  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38771 bytes --]

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

* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
@ 2021-05-02  8:03   ` kernel test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-05-02  8:03 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]

Hi Nicholas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master next-20210430]
[cannot apply to v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r013-20210502 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1c8b90cff6679d7690891101efc07a1edd98e9a7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
        git checkout 1c8b90cff6679d7690891101efc07a1edd98e9a7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/nohash/mmu.h:16,
                    from arch/powerpc/include/asm/mmu.h:423,
                    from arch/powerpc/kernel/head_8xx.S:23:
>> arch/powerpc/include/asm/nohash/32/mmu-8xx.h:175: warning: "MODULES_VADDR" redefined
     175 | #define MODULES_VADDR (PAGE_OFFSET - SZ_256M)
         | 
   In file included from include/linux/pgtable.h:6,
                    from arch/powerpc/kernel/head_8xx.S:19:
   arch/powerpc/include/asm/pgtable.h:43: note: this is the location of the previous definition
      43 | #define MODULES_VADDR VMALLOC_START
         | 
   In file included from arch/powerpc/include/asm/nohash/mmu.h:16,
                    from arch/powerpc/include/asm/mmu.h:423,
                    from arch/powerpc/kernel/head_8xx.S:23:
>> arch/powerpc/include/asm/nohash/32/mmu-8xx.h:176: warning: "MODULES_END" redefined
     176 | #define MODULES_END PAGE_OFFSET
         | 
   In file included from include/linux/pgtable.h:6,
                    from arch/powerpc/kernel/head_8xx.S:19:
   arch/powerpc/include/asm/pgtable.h:44: note: this is the location of the previous definition
      44 | #define MODULES_END VMALLOC_END
         | 


vim +/MODULES_VADDR +175 arch/powerpc/include/asm/nohash/32/mmu-8xx.h

fca5c1e9eb5e26 Christophe Leroy 2019-04-25  174  
9132a2e82adc6e Christophe Leroy 2021-04-01 @175  #define MODULES_VADDR	(PAGE_OFFSET - SZ_256M)
9132a2e82adc6e Christophe Leroy 2021-04-01 @176  #define MODULES_END	PAGE_OFFSET
9132a2e82adc6e Christophe Leroy 2021-04-01  177  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38771 bytes --]

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

* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
  2021-05-02  4:56 [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings Nicholas Piggin
@ 2021-05-02  8:03   ` kernel test robot
  2021-05-02  8:03   ` kernel test robot
  2021-05-02  8:03   ` kernel test robot
  2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-05-02  8:03 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: kbuild-all, Nicholas Piggin

[-- Attachment #1: Type: text/plain, Size: 3548 bytes --]

Hi Nicholas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master next-20210430]
[cannot apply to scottwood/next v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1c8b90cff6679d7690891101efc07a1edd98e9a7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
        git checkout 1c8b90cff6679d7690891101efc07a1edd98e9a7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/proc/kcore.c: In function 'add_modules_range':
>> fs/proc/kcore.c:626:20: warning: self-comparison always evaluates to false [-Wtautological-compare]
     626 |  if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
         |                    ^~
   fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to false [-Wtautological-compare]
     626 |  if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
         |                                                    ^~


vim +626 fs/proc/kcore.c

9492587cf35d37 KAMEZAWA Hiroyuki 2009-09-22  618  
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  619  #if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  620  /*
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  621   * MODULES_VADDR has no intersection with VMALLOC_ADDR.
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  622   */
eebf36480678f9 YueHaibing        2019-03-28  623  static struct kcore_list kcore_modules;
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  624  static void __init add_modules_range(void)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  625  {
bf3e2692468fe4 Baoquan He        2014-10-09 @626  	if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  627  		kclist_add(&kcore_modules, (void *)MODULES_VADDR,
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  628  			MODULES_END - MODULES_VADDR, KCORE_VMALLOC);
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  629  	}
bf3e2692468fe4 Baoquan He        2014-10-09  630  }
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  631  #else
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  632  static void __init add_modules_range(void)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  633  {
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  634  }
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  635  #endif
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  636  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26774 bytes --]

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

* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
@ 2021-05-02  8:03   ` kernel test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-05-02  8:03 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3614 bytes --]

Hi Nicholas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master next-20210430]
[cannot apply to scottwood/next v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1c8b90cff6679d7690891101efc07a1edd98e9a7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
        git checkout 1c8b90cff6679d7690891101efc07a1edd98e9a7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/proc/kcore.c: In function 'add_modules_range':
>> fs/proc/kcore.c:626:20: warning: self-comparison always evaluates to false [-Wtautological-compare]
     626 |  if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
         |                    ^~
   fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to false [-Wtautological-compare]
     626 |  if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
         |                                                    ^~


vim +626 fs/proc/kcore.c

9492587cf35d37 KAMEZAWA Hiroyuki 2009-09-22  618  
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  619  #if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  620  /*
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  621   * MODULES_VADDR has no intersection with VMALLOC_ADDR.
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  622   */
eebf36480678f9 YueHaibing        2019-03-28  623  static struct kcore_list kcore_modules;
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  624  static void __init add_modules_range(void)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  625  {
bf3e2692468fe4 Baoquan He        2014-10-09 @626  	if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  627  		kclist_add(&kcore_modules, (void *)MODULES_VADDR,
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  628  			MODULES_END - MODULES_VADDR, KCORE_VMALLOC);
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  629  	}
bf3e2692468fe4 Baoquan He        2014-10-09  630  }
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  631  #else
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  632  static void __init add_modules_range(void)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  633  {
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  634  }
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  635  #endif
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22  636  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 26774 bytes --]

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

* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
  2021-05-02  7:34 ` Christophe Leroy
@ 2021-05-02 10:41   ` Nicholas Piggin
  0 siblings, 0 replies; 7+ messages in thread
From: Nicholas Piggin @ 2021-05-02 10:41 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev

Excerpts from Christophe Leroy's message of May 2, 2021 5:34 pm:
> 
> 
> Le 02/05/2021 à 06:56, Nicholas Piggin a écrit :
>> This reduces TLB misses by nearly 30x on a `git diff` workload on a
>> 2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
>> to vfs hashes being allocated with 2MB pages.
>> 
>> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> This was in the -mm tree but was dropped at the last minute after
>> clashing with a patch in powerpc next.
>> 
>> Now all prerequisites are upstream, this can be merged as is. Probably
>> makes sense now to go via powerpc tree.
>> 
>> This is rebased and retested on upstream.
>> 
>>   Documentation/admin-guide/kernel-parameters.txt |  2 ++
>>   arch/powerpc/Kconfig                            |  1 +
>>   arch/powerpc/include/asm/pgtable.h              |  5 +++++
>>   arch/powerpc/kernel/module.c                    | 16 +++++++++++++---
>>   4 files changed, 21 insertions(+), 3 deletions(-)
>> 
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 1c0a3cf6fcc9..1be38b25c485 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -3250,6 +3250,8 @@
>>   
>>   	nohugeiomap	[KNL,X86,PPC,ARM64] Disable kernel huge I/O mappings.
>>   
>> +	nohugevmalloc	[PPC] Disable kernel huge vmalloc mappings.
>> +
>>   	nosmt		[KNL,S390] Disable symmetric multithreading (SMT).
>>   			Equivalent to smt=1.
>>   
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 1e6230bea09d..c547a9d6a2dd 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -185,6 +185,7 @@ config PPC
>>   	select GENERIC_VDSO_TIME_NS
>>   	select HAVE_ARCH_AUDITSYSCALL
>>   	select HAVE_ARCH_HUGE_VMAP		if PPC_BOOK3S_64 && PPC_RADIX_MMU
>> +	select HAVE_ARCH_HUGE_VMALLOC		if HAVE_ARCH_HUGE_VMAP
>>   	select HAVE_ARCH_JUMP_LABEL
>>   	select HAVE_ARCH_JUMP_LABEL_RELATIVE
>>   	select HAVE_ARCH_KASAN			if PPC32 && PPC_PAGE_SHIFT <= 14
>> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
>> index c6a676714f04..1678e4b08fc3 100644
>> --- a/arch/powerpc/include/asm/pgtable.h
>> +++ b/arch/powerpc/include/asm/pgtable.h
>> @@ -39,6 +39,11 @@ struct mm_struct;
>>   #define __S110	PAGE_SHARED_X
>>   #define __S111	PAGE_SHARED_X
>>   
>> +#ifndef MODULES_VADDR
>> +#define MODULES_VADDR	VMALLOC_START
>> +#define MODULES_END	VMALLOC_END
>> +#endif
> 
> This will also require some changes in a few places, see 
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210429031602.2606654-4-jniethe5@gmail.com/

I see.

I'll just make the PPC64 version use VMALLOC_START/VMALLOC_END, which
avoids that stupid compiler warning found by kbuild robot as well.

> 
>> +
>>   #ifndef __ASSEMBLY__
>>   
>>   /* Keep these as a macros to avoid include dependency mess */
>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
>> index fab84024650c..77aefcbbd276 100644
>> --- a/arch/powerpc/kernel/module.c
>> +++ b/arch/powerpc/kernel/module.c
>> @@ -8,6 +8,7 @@
>>   #include <linux/moduleloader.h>
>>   #include <linux/err.h>
>>   #include <linux/vmalloc.h>
>> +#include <linux/mm.h>
>>   #include <linux/bug.h>
>>   #include <asm/module.h>
>>   #include <linux/uaccess.h>
>> @@ -88,17 +89,24 @@ int module_finalize(const Elf_Ehdr *hdr,
>>   	return 0;
>>   }
>>   
>> -#ifdef MODULES_VADDR
>>   static __always_inline void *
>>   __module_alloc(unsigned long size, unsigned long start, unsigned long end)
>>   {
>> +	/*
>> +	 * Don't do huge page allocations for modules yet until more testing
>> +	 * is done. STRICT_MODULE_RWX may require extra work to support this
>> +	 * too.
>> +	 */
>>   	return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
>> -				    PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>> +				    PAGE_KERNEL_EXEC,
>> +				    VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
>> +				    NUMA_NO_NODE,
>>   				    __builtin_return_address(0));
> 
> Can we avoid so many lines ? Doesn't it fit on 3 lines now that 100 chars per line are tolerated ?

It could be squashed onto fewer lines. Is it better?

> 
>>   }
>>   
>>   void *module_alloc(unsigned long size)
>>   {
>> +#ifdef CONFIG_PPC32
> 
> Can we just add an IS_ENABLED(CONFIG_PPC32) in the 'if' instead of this #ifdef/#else ?

I guess not if I don't define MODULES_VADDR. Jordan's clenup patch could 
change it to IS_ENABLED.

Thanks,
Nick

> 
>>   	unsigned long limit = (unsigned long)_etext - SZ_32M;
>>   	void *ptr = NULL;
>>   
>> @@ -112,5 +120,7 @@ void *module_alloc(unsigned long size)
>>   		ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>>   
>>   	return ptr;
>> -}
>> +#else
>> +	return __module_alloc(size, MODULES_VADDR, MODULES_END);
>>   #endif
>> +}
>> 
> 

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

end of thread, other threads:[~2021-05-02 10:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-02  4:56 [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings Nicholas Piggin
2021-05-02  7:34 ` Christophe Leroy
2021-05-02 10:41   ` Nicholas Piggin
2021-05-02  8:03 ` kernel test robot
2021-05-02  8:03   ` kernel test robot
2021-05-02  8:03 ` kernel test robot
2021-05-02  8:03   ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.