Re: [PATCH v13 14/14] powerpc/64s/radix: Enable huge vmalloc mappings

[Nicholas Piggin]

Excerpts from Andrew Morton's message of April 16, 2021 4:55 am:
> On Thu, 15 Apr 2021 12:23:55 +0200 Christophe Leroy 
>  wrote:
>> > +   * is done. STRICT_MODULE_RWX may require extra work to support this
>> > +   * too.
>> > +   */
>> >   
>> > -  return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, 
>> > GFP_KERNEL,
>> > -  PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, 
>> > NUMA_NO_NODE,
>> 
>> 
>> I think you should add the following in 
>> 
>> #ifndef MODULES_VADDR
>> #define MODULES_VADDR VMALLOC_START
>> #define MODULES_END VMALLOC_END
>> #endif
>> 
>> And leave module_alloc() as is (just removing the enclosing #ifdef 
>> MODULES_VADDR and adding the 
>> VM_NO_HUGE_VMAP  flag)
>> 
>> This would minimise the conflits with the changes I did in powerpc/next 
>> reported by Stephen R.
>> 
> 
> I'll drop powerpc-64s-radix-enable-huge-vmalloc-mappings.patch for now,
> make life simpler.

Yeah that's fine.

> Nick, a redo on top of Christophe's changes in linux-next would be best
> please.

Will do.

Thanks,
Nick

Re: [PATCH v13 14/14] powerpc/64s/radix: Enable huge vmalloc mappings

[Stephen Rothwell]

Hi all,

On Thu, 15 Apr 2021 11:55:29 -0700 Andrew Morton  
wrote:
>
> On Thu, 15 Apr 2021 12:23:55 +0200 Christophe Leroy 
>  wrote:
> > > +  * is done. STRICT_MODULE_RWX may require extra work to support this
> > > +  * too.
> > > +  */
> > >   
> > > - return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, 
> > > GFP_KERNEL,
> > > - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, 
> > > NUMA_NO_NODE,  
> > 
> > 
> > I think you should add the following in 
> > 
> > #ifndef MODULES_VADDR
> > #define MODULES_VADDR VMALLOC_START
> > #define MODULES_END VMALLOC_END
> > #endif
> > 
> > And leave module_alloc() as is (just removing the enclosing #ifdef 
> > MODULES_VADDR and adding the 
> > VM_NO_HUGE_VMAP  flag)
> > 
> > This would minimise the conflits with the changes I did in powerpc/next 
> > reported by Stephen R.
> >   
> 
> I'll drop powerpc-64s-radix-enable-huge-vmalloc-mappings.patch for now,
> make life simpler.

I have dropped that patch from linux-next.
-- 
Cheers,
Stephen Rothwell


pgpxAl10ZqUKJ.pgp
Description: OpenPGP digital signature

Re: [PATCH v13 14/14] powerpc/64s/radix: Enable huge vmalloc mappings

[Andrew Morton]

On Thu, 15 Apr 2021 12:23:55 +0200 Christophe Leroy 
 wrote:
> > +* is done. STRICT_MODULE_RWX may require extra work to support this
> > +* too.
> > +*/
> >   
> > -   return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, 
> > GFP_KERNEL,
> > -   PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, 
> > NUMA_NO_NODE,
> 
> 
> I think you should add the following in 
> 
> #ifndef MODULES_VADDR
> #define MODULES_VADDR VMALLOC_START
> #define MODULES_END VMALLOC_END
> #endif
> 
> And leave module_alloc() as is (just removing the enclosing #ifdef 
> MODULES_VADDR and adding the 
> VM_NO_HUGE_VMAP  flag)
> 
> This would minimise the conflits with the changes I did in powerpc/next 
> reported by Stephen R.
> 

I'll drop powerpc-64s-radix-enable-huge-vmalloc-mappings.patch for now,
make life simpler.

Nick, a redo on top of Christophe's changes in linux-next would be best
please.

Re: [PATCH v13 14/14] powerpc/64s/radix: Enable huge vmalloc mappings

[Christophe Leroy]

Hi Nick,

Le 17/03/2021 à 07:24, 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.

Cc: linuxppc-...@lists.ozlabs.org
Acked-by: Michael Ellerman 
Signed-off-by: Nicholas Piggin 
---
  .../admin-guide/kernel-parameters.txt |  2 ++
  arch/powerpc/Kconfig  |  1 +
  arch/powerpc/kernel/module.c  | 22 +++
  3 files changed, 21 insertions(+), 4 deletions(-)

--- a/arch/powerpc/kernel/module.c
+++ b/arch/powerpc/kernel/module.c
@@ -8,6 +8,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -87,13 +88,26 @@ int module_finalize(const Elf_Ehdr *hdr,
return 0;
  }
  
-#ifdef MODULES_VADDR

  void *module_alloc(unsigned long size)
  {
+   unsigned long start = VMALLOC_START;
+   unsigned long end = VMALLOC_END;
+
+#ifdef MODULES_VADDR
BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
+   start = MODULES_VADDR;
+   end = MODULES_END;
+#endif
+
+   /*
+* 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, MODULES_VADDR, MODULES_END, GFP_KERNEL,

-   PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, 
NUMA_NO_NODE,



I think you should add the following in 

#ifndef MODULES_VADDR
#define MODULES_VADDR VMALLOC_START
#define MODULES_END VMALLOC_END
#endif

And leave module_alloc() as is (just removing the enclosing #ifdef MODULES_VADDR and adding the 
VM_NO_HUGE_VMAP  flag)


This would minimise the conflits with the changes I did in powerpc/next 
reported by Stephen R.


+   return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
+   PAGE_KERNEL_EXEC,
+   VM_NO_HUGE_VMAP | VM_FLUSH_RESET_PERMS,
+   NUMA_NO_NODE,
__builtin_return_address(0));
  }
-#endif