From: Kefeng Wang <hidden> Date: 2021-12-27 14:49:48
Huge vmalloc mappings is supported on PPC[1], but this feature should
be not only used on PPC, it could be used on arch support HAVE_ARCH_HUGE_VMAP
and PMD sized vmap mappings. this patchset is to enable this feature
on arm64/x86.
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
Based on the above considerations, we add the first patch is to let
user to control huge vmalloc mapping default behavior. Meanwhile,
add new kernel parameter hugevmalloc=on/off to enable/disable this
feature at boot time, nohugevmalloc parameter is still supported.
The later two patches to enable this feature on arm64/x86, select
HAVE_ARCH_HUGE_VMALLOC and mark VM_NO_HUGE_VMAP in arch's module_alloc().
This patchset based on next-20211224.
v2:
- Default y for HUGE_VMALLOC_DEFAULT_ENABLED, not only select it on PPC
- Fix copy/type error
- Mark VM_NO_HUGE_VMAP in module_alloc() on arm64/x86
[1] https://lore.kernel.org/linux-mm/20210317062402.533919-1-npiggin@gmail.com/
[2] https://lore.kernel.org/linux-mm/1616036421.amjz2efujj.astroid@bobo.none/
Kefeng Wang (3):
mm: vmalloc: Let user to control huge vmalloc default behavior
arm64: Support huge vmalloc mappings
x86: Support huge vmalloc mappings
.../admin-guide/kernel-parameters.txt | 14 +++++++++++++-
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/module.c | 5 +++--
arch/x86/Kconfig | 1 +
arch/x86/kernel/module.c | 4 ++--
mm/Kconfig | 8 ++++++++
mm/vmalloc.c | 18 +++++++++++++++++-
7 files changed, 45 insertions(+), 6 deletions(-)
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -1639,7 +1639,7 @@ precedence over memory_hotplug.memmap_on_memory.- hugevmalloc= [PPC] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC+ hugevmalloc= [KNL,PPC,ARM64] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC Format: { on | off } Default set by CONFIG_HUGE_VMALLOC_DEFAULT_ENABLED.
From: Kefeng Wang <hidden> Date: 2021-12-27 14:49:52
Introduce HUGE_VMALLOC_DEFAULT_ENABLED and make it default y, this
let user to choose whether or not enable huge vmalloc mappings by
default.
Meanwhile, add new hugevmalloc=on/off parameter to enable or disable
this feature at boot time, nohugevmalloc is still supported and
equivalent to hugevmalloc=off.
Signed-off-by: Kefeng Wang <redacted>
---
.../admin-guide/kernel-parameters.txt | 12 ++++++++++++
mm/Kconfig | 8 ++++++++
mm/vmalloc.c | 18 +++++++++++++++++-
3 files changed, 37 insertions(+), 1 deletion(-)
@@ -1638,6 +1638,18 @@ If both parameters are enabled, hugetlb_free_vmemmap takes precedence over memory_hotplug.memmap_on_memory.++ hugevmalloc= [PPC] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC+ Format: { on | off }+ Default set by CONFIG_HUGE_VMALLOC_DEFAULT_ENABLED.++ This parameter enables/disables kernel huge vmalloc+ mappings at boot time.++ on: Enable the feature+ off: Disable the feature+ Equivalent to: nohugevmalloc+ hung_task_panic= [KNL] Should the hung task detector generate panics. Format: 0 | 1
@@ -1639,7 +1639,7 @@ precedence over memory_hotplug.memmap_on_memory.- hugevmalloc= [KNL,PPC,ARM64] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC+ hugevmalloc= [KNL,PPC,ARM64,X86] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC Format: { on | off } Default set by CONFIG_HUGE_VMALLOC_DEFAULT_ENABLED.
From: Dave Hansen <hidden> Date: 2021-12-27 16:00:42
On 12/27/21 6:59 AM, Kefeng Wang wrote:
This patch select HAVE_ARCH_HUGE_VMALLOC to let X86_64 and X86_PAE
support huge vmalloc mappings.
In general, this seems interesting and the diff is simple. But, I don't
see _any_ x86-specific data. I think the bare minimum here would be a
few kernel compiles and some 'perf stat' data for some TLB events.
@@ -75,8 +75,8 @@ void *module_alloc(unsigned long size)p=__vmalloc_node_range(size,MODULE_ALIGN,MODULES_VADDR+get_module_load_offset(),-MODULES_END,gfp_mask,-PAGE_KERNEL,VM_DEFER_KMEMLEAK,NUMA_NO_NODE,+MODULES_END,gfp_mask,PAGE_KERNEL,+VM_DEFER_KMEMLEAK|VM_NO_HUGE_VMAP,NUMA_NO_NODE,__builtin_return_address(0));if(p&&(kasan_module_alloc(p,size,gfp_mask)<0)){vfree(p);
To figure out what's going on in this hunk, I had to look at the cover
letter (which I wasn't cc'd on). That's not great and it means that
somebody who stumbles upon this in the code is going to have a really
hard time figuring out what is going on. Cover letters don't make it
into git history.
This desperately needs a comment and some changelog material in *this*
patch.
But, even the description from the cover letter is sparse:
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
That just says that x86 *needs* PAGE_SIZE allocations. But, what
happens if VM_NO_HUGE_VMAP is not passed (like it was in v1)? Will the
subsequent permission changes just fragment the 2M mapping?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -1639,7 +1639,7 @@ precedence over memory_hotplug.memmap_on_memory.- hugevmalloc= [PPC] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC+ hugevmalloc= [KNL,PPC,ARM64] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC Format: { on | off } Default set by CONFIG_HUGE_VMALLOC_DEFAULT_ENABLED.
@@ -1639,7 +1639,7 @@ precedence over memory_hotplug.memmap_on_memory.- hugevmalloc= [PPC] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC+ hugevmalloc= [KNL,PPC,ARM64] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC Format: { on | off } Default set by CONFIG_HUGE_VMALLOC_DEFAULT_ENABLED.
From: Kefeng Wang <hidden> Date: 2021-12-28 10:26:46
On 2021/12/27 23:56, Dave Hansen wrote:
On 12/27/21 6:59 AM, Kefeng Wang wrote:
quoted
This patch select HAVE_ARCH_HUGE_VMALLOC to let X86_64 and X86_PAE
support huge vmalloc mappings.
In general, this seems interesting and the diff is simple. But, I don't
see _any_ x86-specific data. I think the bare minimum here would be a
few kernel compiles and some 'perf stat' data for some TLB events.
When the feature supported on ppc,
commit 8abddd968a303db75e4debe77a3df484164f1f33
Author: Nicholas Piggin [off-list ref]
Date: Mon May 3 19:17:55 2021 +1000
powerpc/64s/radix: Enable huge vmalloc mappings
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.
But the data could be different on different machine/arch.
@@ -75,8 +75,8 @@ void *module_alloc(unsigned long size)p=__vmalloc_node_range(size,MODULE_ALIGN,MODULES_VADDR+get_module_load_offset(),-MODULES_END,gfp_mask,-PAGE_KERNEL,VM_DEFER_KMEMLEAK,NUMA_NO_NODE,+MODULES_END,gfp_mask,PAGE_KERNEL,+VM_DEFER_KMEMLEAK|VM_NO_HUGE_VMAP,NUMA_NO_NODE,__builtin_return_address(0));if(p&&(kasan_module_alloc(p,size,gfp_mask)<0)){vfree(p);
To figure out what's going on in this hunk, I had to look at the cover
letter (which I wasn't cc'd on). That's not great and it means that
somebody who stumbles upon this in the code is going to have a really
hard time figuring out what is going on. Cover letters don't make it
into git history.
Sorry for that, will add more into arch's patch changelog.
This desperately needs a comment and some changelog material in *this*
patch.
But, even the description from the cover letter is sparse:
quoted
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
That just says that x86 *needs* PAGE_SIZE allocations. But, what
happens if VM_NO_HUGE_VMAP is not passed (like it was in v1)? Will the
subsequent permission changes just fragment the 2M mapping?
.
Yes, without VM_NO_HUGE_VMAP, it could fragment the 2M mapping.
When module alloc with STRICT_MODULE_RWX on x86, it calls
__change_page_attr()
from set_memory_ro/rw/nx which will split large page, so there is no
need to make
module alloc with HUGE_VMALLOC.
From: Dave Hansen <hidden> Date: 2021-12-28 16:15:07
On 12/28/21 2:26 AM, Kefeng Wang wrote:
quoted
quoted
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
That just says that x86 *needs* PAGE_SIZE allocations. But, what
happens if VM_NO_HUGE_VMAP is not passed (like it was in v1)? Will the
subsequent permission changes just fragment the 2M mapping?
Yes, without VM_NO_HUGE_VMAP, it could fragment the 2M mapping.
When module alloc with STRICT_MODULE_RWX on x86, it calls
__change_page_attr()
from set_memory_ro/rw/nx which will split large page, so there is no
need to make
module alloc with HUGE_VMALLOC.
This all sounds very fragile to me. Every time a new architecture would
get added for huge vmalloc() support, the developer needs to know to go
find that architecture's module_alloc() and add this flag. They next
guy is going to forget, just like you did.
Considering that this is not a hot path, a weak function would be a nice
choice:
/* vmalloc() flags used for all module allocations. */
unsigned long __weak arch_module_vm_flags()
{
/*
* Modules use a single, large vmalloc(). Different
* permissions are applied later and will fragment
* huge mappings. Avoid using huge pages for modules.
*/
return VM_NO_HUGE_VMAP;
}
Stick that in some the common module code, next to:
...
Then, put arch_module_vm_flags() in *all* of the module_alloc()
implementations, including the generic one. That way (even with a new
architecture) whoever copies-and-pastes their module_alloc()
implementation is likely to get it right. The next guy who just does a
"select HAVE_ARCH_HUGE_VMALLOC" will hopefully just work.
VM_FLUSH_RESET_PERMS could probably be dealt with in the same way.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Kefeng Wang <hidden> Date: 2021-12-29 11:01:24
On 2021/12/29 0:14, Dave Hansen wrote:
On 12/28/21 2:26 AM, Kefeng Wang wrote:
quoted
quoted
quoted
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
That just says that x86 *needs* PAGE_SIZE allocations. But, what
happens if VM_NO_HUGE_VMAP is not passed (like it was in v1)? Will the
subsequent permission changes just fragment the 2M mapping?
Yes, without VM_NO_HUGE_VMAP, it could fragment the 2M mapping.
When module alloc with STRICT_MODULE_RWX on x86, it calls
__change_page_attr()
from set_memory_ro/rw/nx which will split large page, so there is no
need to make
module alloc with HUGE_VMALLOC.
This all sounds very fragile to me. Every time a new architecture would
get added for huge vmalloc() support, the developer needs to know to go
find that architecture's module_alloc() and add this flag. They next
guy is going to forget, just like you did.
Considering that this is not a hot path, a weak function would be a nice
choice:
/* vmalloc() flags used for all module allocations. */
unsigned long __weak arch_module_vm_flags()
{
/*
* Modules use a single, large vmalloc(). Different
* permissions are applied later and will fragment
* huge mappings. Avoid using huge pages for modules.
*/
return VM_NO_HUGE_VMAP;
For x86, it only fragment, but for arm64, due to apply_to_page_range() in
set_memory_*, without this flag maybe crash. Whatever, we need this
flag for module.
}
Stick that in some the common module code, next to:
...
Then, put arch_module_vm_flags() in *all* of the module_alloc()
implementations, including the generic one. That way (even with a new
architecture) whoever copies-and-pastes their module_alloc()
implementation is likely to get it right. The next guy who just does a
"select HAVE_ARCH_HUGE_VMALLOC" will hopefully just work.
OK, Let me check the VM_FLUSH_RESET_PERMS and try about this way.
Thanks.
VM_FLUSH_RESET_PERMS could probably be dealt with in the same way.
.
@@ -1639,7 +1639,7 @@ precedence over memory_hotplug.memmap_on_memory.- hugevmalloc= [PPC] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC+ hugevmalloc= [KNL,PPC,ARM64] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC Format: { on | off } Default set by CONFIG_HUGE_VMALLOC_DEFAULT_ENABLED.
@@ -36,7 +36,8 @@ void *module_alloc(unsigned long size)module_alloc_end=MODULES_END;p=__vmalloc_node_range(size,MODULE_ALIGN,module_alloc_base,-module_alloc_end,gfp_mask,PAGE_KERNEL,VM_DEFER_KMEMLEAK,+module_alloc_end,gfp_mask,PAGE_KERNEL,+VM_DEFER_KMEMLEAK|VM_NO_HUGE_VMAP,
you should add a comment like powerpc (commit 8abddd968a30
("powerpc/64s/radix: Enable huge vmalloc mappings")) to explain why this
requires VM_NO_HUGE_VMAP
quoted hunk
NUMA_NO_NODE, __builtin_return_address(0));
if (!p && IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) &&
@@ -1639,7 +1639,7 @@ precedence over memory_hotplug.memmap_on_memory.- hugevmalloc= [KNL,PPC,ARM64] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC+ hugevmalloc= [KNL,PPC,ARM64,X86] Reguires CONFIG_HAVE_ARCH_HUGE_VMALLOC Format: { on | off } Default set by CONFIG_HUGE_VMALLOC_DEFAULT_ENABLED.
@@ -75,8 +75,8 @@ void *module_alloc(unsigned long size)p=__vmalloc_node_range(size,MODULE_ALIGN,MODULES_VADDR+get_module_load_offset(),-MODULES_END,gfp_mask,-PAGE_KERNEL,VM_DEFER_KMEMLEAK,NUMA_NO_NODE,+MODULES_END,gfp_mask,PAGE_KERNEL,+VM_DEFER_KMEMLEAK|VM_NO_HUGE_VMAP,NUMA_NO_NODE,
you should add a comment like powerpc (commit 8abddd968a30
("powerpc/64s/radix: Enable huge vmalloc mappings")) to explain why this
requires VM_NO_HUGE_VMAP
Huge vmalloc mappings is supported on PPC[1], but this feature should
be not only used on PPC, it could be used on arch support HAVE_ARCH_HUGE_VMAP
and PMD sized vmap mappings. this patchset is to enable this feature
on arm64/x86.
There are some disadvantages about this feature[2], one of the main
There are some disadvantage, ok, so are there advantages as well ?
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
Based on the above considerations, we add the first patch is to let
user to control huge vmalloc mapping default behavior. Meanwhile,
add new kernel parameter hugevmalloc=on/off to enable/disable this
feature at boot time, nohugevmalloc parameter is still supported.
The later two patches to enable this feature on arm64/x86, select
HAVE_ARCH_HUGE_VMALLOC and mark VM_NO_HUGE_VMAP in arch's module_alloc().
This patchset based on next-20211224.
v2:
- Default y for HUGE_VMALLOC_DEFAULT_ENABLED, not only select it on PPC
- Fix copy/type error
- Mark VM_NO_HUGE_VMAP in module_alloc() on arm64/x86
[1] https://lore.kernel.org/linux-mm/20210317062402.533919-1-npiggin@gmail.com/
[2] https://lore.kernel.org/linux-mm/1616036421.amjz2efujj.astroid@bobo.none/
Kefeng Wang (3):
mm: vmalloc: Let user to control huge vmalloc default behavior
arm64: Support huge vmalloc mappings
x86: Support huge vmalloc mappings
.../admin-guide/kernel-parameters.txt | 14 +++++++++++++-
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/module.c | 5 +++--
arch/x86/Kconfig | 1 +
arch/x86/kernel/module.c | 4 ++--
mm/Kconfig | 8 ++++++++
mm/vmalloc.c | 18 +++++++++++++++++-
7 files changed, 45 insertions(+), 6 deletions(-)
This patch select HAVE_ARCH_HUGE_VMALLOC to let X86_64 and X86_PAE
support huge vmalloc mappings.
In general, this seems interesting and the diff is simple. But, I don't
see _any_ x86-specific data. I think the bare minimum here would be a
few kernel compiles and some 'perf stat' data for some TLB events.
When the feature supported on ppc,
commit 8abddd968a303db75e4debe77a3df484164f1f33
Author: Nicholas Piggin [off-list ref]
Date: Mon May 3 19:17:55 2021 +1000
powerpc/64s/radix: Enable huge vmalloc mappings
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.
But the data could be different on different machine/arch.
To figure out what's going on in this hunk, I had to look at the cover
letter (which I wasn't cc'd on). That's not great and it means that
somebody who stumbles upon this in the code is going to have a really
hard time figuring out what is going on. Cover letters don't make it
into git history.
Sorry for that, will add more into arch's patch changelog.
quoted
This desperately needs a comment and some changelog material in *this*
patch.
But, even the description from the cover letter is sparse:
quoted
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
That just says that x86 *needs* PAGE_SIZE allocations. But, what
happens if VM_NO_HUGE_VMAP is not passed (like it was in v1)? Will the
subsequent permission changes just fragment the 2M mapping?
.
Yes, without VM_NO_HUGE_VMAP, it could fragment the 2M mapping.
When module alloc with STRICT_MODULE_RWX on x86, it calls
__change_page_attr()
from set_memory_ro/rw/nx which will split large page, so there is no
need to make
module alloc with HUGE_VMALLOC.
Maybe there is no need to perform the module alloc with HUGE_VMALLOC,
but it least it would still work if you do so.
Powerpc did add VM_NO_HUGE_VMAP temporarily and for some reason which is
explained in a comment.
If x86 already has the necessary logic to handle it, why add
VM_NO_HUGE_VMAP ?
Christophe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
That just says that x86 *needs* PAGE_SIZE allocations. But, what
happens if VM_NO_HUGE_VMAP is not passed (like it was in v1)? Will the
subsequent permission changes just fragment the 2M mapping?
Yes, without VM_NO_HUGE_VMAP, it could fragment the 2M mapping.
When module alloc with STRICT_MODULE_RWX on x86, it calls
__change_page_attr()
from set_memory_ro/rw/nx which will split large page, so there is no
need to make
module alloc with HUGE_VMALLOC.
This all sounds very fragile to me. Every time a new architecture would
get added for huge vmalloc() support, the developer needs to know to go
find that architecture's module_alloc() and add this flag. They next
guy is going to forget, just like you did.
That's not correct from my point of view.
When powerpc added that, a clear comment explains why:
+ /*
+ * 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.
+ */
So as you can see, this is something specific to powerpc and temporary.
Considering that this is not a hot path, a weak function would be a nice
choice:
/* vmalloc() flags used for all module allocations. */
unsigned long __weak arch_module_vm_flags()
{
/*
* Modules use a single, large vmalloc(). Different
* permissions are applied later and will fragment
* huge mappings. Avoid using huge pages for modules.
*/
Why ? Not everybody use STRICT_MODULES_RWX.
Even if you do so, you can still benefit from huge pages for modules.
Why make what was initially a temporary precaution for powerpc become a
definitive default limitation for all ?
return VM_NO_HUGE_VMAP;
}
Stick that in some the common module code, next to:
...
Then, put arch_module_vm_flags() in *all* of the module_alloc()
implementations, including the generic one. That way (even with a new
architecture) whoever copies-and-pastes their module_alloc()
implementation is likely to get it right. The next guy who just does a
"select HAVE_ARCH_HUGE_VMALLOC" will hopefully just work.
VM_FLUSH_RESET_PERMS could probably be dealt with in the same way.
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
That just says that x86 *needs* PAGE_SIZE allocations. But, what
happens if VM_NO_HUGE_VMAP is not passed (like it was in v1)? Will the
subsequent permission changes just fragment the 2M mapping?
Yes, without VM_NO_HUGE_VMAP, it could fragment the 2M mapping.
When module alloc with STRICT_MODULE_RWX on x86, it calls
__change_page_attr()
from set_memory_ro/rw/nx which will split large page, so there is no
need to make
module alloc with HUGE_VMALLOC.
This all sounds very fragile to me. Every time a new architecture would
get added for huge vmalloc() support, the developer needs to know to go
find that architecture's module_alloc() and add this flag. They next
guy is going to forget, just like you did.
Considering that this is not a hot path, a weak function would be a nice
choice:
/* vmalloc() flags used for all module allocations. */
unsigned long __weak arch_module_vm_flags()
{
/*
* Modules use a single, large vmalloc(). Different
* permissions are applied later and will fragment
* huge mappings. Avoid using huge pages for modules.
*/
return VM_NO_HUGE_VMAP;
For x86, it only fragment, but for arm64, due to apply_to_page_range() in
set_memory_*, without this flag maybe crash. Whatever, we need this
flag for module.
I see no reason to have this flag by default.
Only ARM should have it if necessary, with a comment explaining why just
like powerpc.
And maybe the flag should be there only when STRICT_MODULE_RWX is selected.
quoted
}
Stick that in some the common module code, next to:
...
Then, put arch_module_vm_flags() in *all* of the module_alloc()
implementations, including the generic one. That way (even with a new
architecture) whoever copies-and-pastes their module_alloc()
implementation is likely to get it right. The next guy who just does a
"select HAVE_ARCH_HUGE_VMALLOC" will hopefully just work.
OK, Let me check the VM_FLUSH_RESET_PERMS and try about this way.
Thanks.
quoted
VM_FLUSH_RESET_PERMS could probably be dealt with in the same way.
.
From: Nicholas Piggin <npiggin@gmail.com> Date: 2022-01-18 03:13:19
Excerpts from Dave Hansen's message of December 29, 2021 2:14 am:
On 12/28/21 2:26 AM, Kefeng Wang wrote:
quoted
quoted
quoted
There are some disadvantages about this feature[2], one of the main
concerns is the possible memory fragmentation/waste in some scenarios,
also archs must ensure that any arch specific vmalloc allocations that
require PAGE_SIZE mappings(eg, module alloc with STRICT_MODULE_RWX)
use the VM_NO_HUGE_VMAP flag to inhibit larger mappings.
That just says that x86 *needs* PAGE_SIZE allocations. But, what
happens if VM_NO_HUGE_VMAP is not passed (like it was in v1)? Will the
subsequent permission changes just fragment the 2M mapping?
Yes, without VM_NO_HUGE_VMAP, it could fragment the 2M mapping.
When module alloc with STRICT_MODULE_RWX on x86, it calls
__change_page_attr()
from set_memory_ro/rw/nx which will split large page, so there is no
need to make
module alloc with HUGE_VMALLOC.
This all sounds very fragile to me. Every time a new architecture would
get added for huge vmalloc() support, the developer needs to know to go
find that architecture's module_alloc() and add this flag.
This is documented in the Kconfig.
#
# Archs that select this would be capable of PMD-sized vmaps (i.e.,
# arch_vmap_pmd_supported() returns true), and they must make no assumptions
# that vmalloc memory is mapped with PAGE_SIZE ptes. The VM_NO_HUGE_VMAP flag
# can be used to prohibit arch-specific allocations from using hugepages to
# help with this (e.g., modules may require it).
#
config HAVE_ARCH_HUGE_VMALLOC
depends on HAVE_ARCH_HUGE_VMAP
bool
Is it really fair to say it's *very* fragile? Surely it's reasonable to
read the (not very long) documentation ad understand the consequences for
the arch code before enabling it.
They next
guy is going to forget, just like you did.
The miss here could just be a simple oversight or thinko, and caught by
review, as happens to a lot of things.
Considering that this is not a hot path, a weak function would be a nice
choice:
/* vmalloc() flags used for all module allocations. */
unsigned long __weak arch_module_vm_flags()
{
/*
* Modules use a single, large vmalloc(). Different
* permissions are applied later and will fragment
* huge mappings. Avoid using huge pages for modules.
*/
return VM_NO_HUGE_VMAP;
}
Stick that in some the common module code, next to:
Then they have to think about it even less, so I don't know if that's an
improvement. I don't know what else an arch might be doing with these
allocations, at least modules will blow up pretty quickly, who knows
what other rare code relies on 4k vmalloc mappings?
The huge vmalloc option is not supposed to be easy to enable. This is
the same problem Andy was having with the TLB shootdown patches, he
didn't read the documentation and thought it was supposed to be a
trivial thing anybody could enable without thinking about it, and was
dutifully pointing out the the nasty "bugs" the feature has in it if
x86 were to enable it improperly.
Thanks,
Nick
...
Then, put arch_module_vm_flags() in *all* of the module_alloc()
implementations, including the generic one. That way (even with a new
architecture) whoever copies-and-pastes their module_alloc()
implementation is likely to get it right. The next guy who just does a
"select HAVE_ARCH_HUGE_VMALLOC" will hopefully just work.
VM_FLUSH_RESET_PERMS could probably be dealt with in the same way.
From: Nicholas Piggin <npiggin@gmail.com> Date: 2022-01-18 03:26:13
Excerpts from Kefeng Wang's message of December 28, 2021 12:59 am:
Introduce HUGE_VMALLOC_DEFAULT_ENABLED and make it default y, this
let user to choose whether or not enable huge vmalloc mappings by
default.
Meanwhile, add new hugevmalloc=on/off parameter to enable or disable
this feature at boot time, nohugevmalloc is still supported and
equivalent to hugevmalloc=off.
Runtime options are bad enough, Kconfig and boot options are even worse.
The 'nohugevmalloc' option mirrors 'nohugeiomap' and is not expected to
ever be understood by an administrator unless a kernel developer is
working with them to hunt down a regression.
IMO there should be no new options. You could switch it off for
CONFIG_BASE_SMALL perhaps, and otherwise just try to work on heuristics
first. Bring in new options once it's proven they're needed.
Aside from that, thanks for working on these ports, great work.
Thanks,
Nick
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Dave Hansen <hidden> Date: 2022-01-18 17:28:58
On 1/17/22 6:46 PM, Nicholas Piggin wrote:
quoted
This all sounds very fragile to me. Every time a new architecture would
get added for huge vmalloc() support, the developer needs to know to go
find that architecture's module_alloc() and add this flag.
This is documented in the Kconfig.
#
# Archs that select this would be capable of PMD-sized vmaps (i.e.,
# arch_vmap_pmd_supported() returns true), and they must make no assumptions
# that vmalloc memory is mapped with PAGE_SIZE ptes. The VM_NO_HUGE_VMAP flag
# can be used to prohibit arch-specific allocations from using hugepages to
# help with this (e.g., modules may require it).
#
config HAVE_ARCH_HUGE_VMALLOC
depends on HAVE_ARCH_HUGE_VMAP
bool
Is it really fair to say it's *very* fragile? Surely it's reasonable to
read the (not very long) documentation ad understand the consequences for
the arch code before enabling it.
Very fragile or not, I think folks are likely to get it wrong. It would
be nice to have it default *everyone* to safe and slow and make *sure*
they go look at the architecture modules code itself before enabling
this for modules.
Just from that Kconfig text, I don't think I'd know off the top of my
head what do do for x86, or what code I needed to go touch.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Nicholas Piggin <npiggin@gmail.com> Date: 2022-01-19 04:17:49
Excerpts from Dave Hansen's message of January 19, 2022 3:28 am:
On 1/17/22 6:46 PM, Nicholas Piggin wrote:
quoted
quoted
This all sounds very fragile to me. Every time a new architecture would
get added for huge vmalloc() support, the developer needs to know to go
find that architecture's module_alloc() and add this flag.
This is documented in the Kconfig.
#
# Archs that select this would be capable of PMD-sized vmaps (i.e.,
# arch_vmap_pmd_supported() returns true), and they must make no assumptions
# that vmalloc memory is mapped with PAGE_SIZE ptes. The VM_NO_HUGE_VMAP flag
# can be used to prohibit arch-specific allocations from using hugepages to
# help with this (e.g., modules may require it).
#
config HAVE_ARCH_HUGE_VMALLOC
depends on HAVE_ARCH_HUGE_VMAP
bool
Is it really fair to say it's *very* fragile? Surely it's reasonable to
read the (not very long) documentation ad understand the consequences for
the arch code before enabling it.
Very fragile or not, I think folks are likely to get it wrong. It would
be nice to have it default *everyone* to safe and slow and make *sure*
It's not safe to enable though. That's the problem. If it was just
modules then you'd have a point but it could be anything.
they go look at the architecture modules code itself before enabling
this for modules.
This is required not just for modules for the whole arch code, it
has to be looked at and decided this will work.
Just from that Kconfig text, I don't think I'd know off the top of my
head what do do for x86, or what code I needed to go touch.
You have to make sure arch/x86 makes no assumptions that vmalloc memory
is backed by PAGE_SIZE ptes. If you can't do that then you shouldn't
enable the option. The option can not explain it any more because any
arch could do anything with its mappings. The module code is an example,
not the recipe.
Thanks,
Nick
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Kefeng Wang <hidden> Date: 2022-01-19 12:58:04
On 2022/1/18 10:52, Nicholas Piggin wrote:
Excerpts from Kefeng Wang's message of December 28, 2021 12:59 am:
quoted
Introduce HUGE_VMALLOC_DEFAULT_ENABLED and make it default y, this
let user to choose whether or not enable huge vmalloc mappings by
default.
Meanwhile, add new hugevmalloc=on/off parameter to enable or disable
this feature at boot time, nohugevmalloc is still supported and
equivalent to hugevmalloc=off.
Runtime options are bad enough, Kconfig and boot options are even worse.
nohugevmalloc is like blacklists, on the other hand, Add a
HUGE_VMALLOC_DEFAULT_ENABLED
to close hugevmalloc default and enable it only via hugevmalloc=on is
whiteList.
Only parts of our products wants this feature, we add some interfaces
which only
alloc hugevmalloc for them, eg,
vmap_hugepage/vmalloc_hugepage/remap_vmalloc_hugepage_range..
for our products, but it's not the choice of most products, also add
nohugevmalloc
for most products is expensive, so this is the reason for adding the patch.
more config/cmdline are more flexible for test/products,
The 'nohugevmalloc' option mirrors 'nohugeiomap' and is not expected to
ever be understood by an administrator unless a kernel developer is
working with them to hunt down a regression.
IMO there should be no new options. You could switch it off for
CONFIG_BASE_SMALL perhaps, and otherwise just try to work on heuristics
first. Bring in new options once it's proven they're needed.
but yes, this patch is optional, could others give some more comments
about this way?
Thanks.
Aside from that, thanks for working on these ports, great work.
Thanks,
Nick
.
From: Matthew Wilcox <willy@infradead.org> Date: 2022-01-19 13:22:25
On Wed, Jan 19, 2022 at 08:57:58PM +0800, Kefeng Wang wrote:
Only parts of our products wants this feature, we add some interfaces which
only
alloc hugevmalloc for them, eg,
vmap_hugepage/vmalloc_hugepage/remap_vmalloc_hugepage_range..
for our products, but it's not the choice of most products, also add
nohugevmalloc
for most products is expensive, so this is the reason for adding the patch.
more config/cmdline are more flexible for test/products,
But why do only some products want it? What goes wrong if all products
enable it? Features should be auto-tuning, not relying on admins to
understand them.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Kefeng Wang <hidden> Date: 2022-01-19 13:32:21
On 2022/1/19 12:17, Nicholas Piggin wrote:
Excerpts from Dave Hansen's message of January 19, 2022 3:28 am:
quoted
On 1/17/22 6:46 PM, Nicholas Piggin wrote:
quoted
quoted
This all sounds very fragile to me. Every time a new architecture would
get added for huge vmalloc() support, the developer needs to know to go
find that architecture's module_alloc() and add this flag.
This is documented in the Kconfig.
#
# Archs that select this would be capable of PMD-sized vmaps (i.e.,
# arch_vmap_pmd_supported() returns true), and they must make no assumptions
# that vmalloc memory is mapped with PAGE_SIZE ptes. The VM_NO_HUGE_VMAP flag
# can be used to prohibit arch-specific allocations from using hugepages to
# help with this (e.g., modules may require it).
#
config HAVE_ARCH_HUGE_VMALLOC
depends on HAVE_ARCH_HUGE_VMAP
bool
Is it really fair to say it's *very* fragile? Surely it's reasonable to
read the (not very long) documentation ad understand the consequences for
the arch code before enabling it.
Very fragile or not, I think folks are likely to get it wrong. It would
be nice to have it default *everyone* to safe and slow and make *sure*
It's not safe to enable though. That's the problem. If it was just
modules then you'd have a point but it could be anything.
quoted
they go look at the architecture modules code itself before enabling
this for modules.
This is required not just for modules for the whole arch code, it
has to be looked at and decided this will work.
quoted
Just from that Kconfig text, I don't think I'd know off the top of my
head what do do for x86, or what code I needed to go touch.
You have to make sure arch/x86 makes no assumptions that vmalloc memory
is backed by PAGE_SIZE ptes. If you can't do that then you shouldn't
enable the option. The option can not explain it any more because any
arch could do anything with its mappings. The module code is an example,
not the recipe.
Hi Nick, Dave and Christophe,thanks for your review, a little
confused, I think,
1) for ppc/arm64 module_alloc(), it must set VM_NO_HUGE_VMAP because the
arch's set_memory_* funcitons can only support PAGE_SIZE mapping, due to the
limit of apply_to_page_range().
2) but for x86's module_alloc(), add VM_NO_HUGE_VMAP is to avoid
fragmentation,
x86's __change_page_attr functions will split the huge mapping. this
flags is not a must.
and the behavior above occurred when STRICT_MODULE_RWX enabled, so
1) add a unified function to set vm flags(suggested by Dave ) or
2) add vm flags with some comments to per-arch's module_alloc()
are both acceptable, for the way of unified function , we could make
this a default recipe
with STRICT_MODULE_RWX, also make two more vm flags into it, eg,
+unsigned long module_alloc_vm_flags(bool need_flush_reset_perms)
+{
+ unsigned long vm_flags = VM_DEFER_KMEMLEAK;
+
+ if (need_flush_reset_perms)
+ vm_flags |= VM_FLUSH_RESET_PERMS;
+ /*
+ * Modules use a single, large vmalloc(). Different permissions
+ * are applied later and will fragment huge mappings or even
+ * fails in set_memory_* on some architectures. Avoid using
+ * huge pages for modules.
+ */
+ if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
+ vm_flags |= VM_NO_HUGE_VMAP;
+
+ return vm_flags;
+}
then called each arch's module_alloc().
Any suggestion, many thanks.
From: Kefeng Wang <hidden> Date: 2022-01-19 13:44:28
On 2022/1/19 21:22, Matthew Wilcox wrote:
On Wed, Jan 19, 2022 at 08:57:58PM +0800, Kefeng Wang wrote:
quoted
Only parts of our products wants this feature, we add some interfaces which
only
alloc hugevmalloc for them, eg,
vmap_hugepage/vmalloc_hugepage/remap_vmalloc_hugepage_range..
for our products, but it's not the choice of most products, also add
nohugevmalloc
for most products is expensive, so this is the reason for adding the patch.
more config/cmdline are more flexible for test/products,
But why do only some products want it? What goes wrong if all products
enable it? Features should be auto-tuning, not relying on admins to
understand them.
Because this feature will use more memory for vmalloc/vmap, that's why
we add some explicit
interfaces as said above in our kernel to control the user.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Matthew Wilcox <willy@infradead.org> Date: 2022-01-19 13:48:30
On Wed, Jan 19, 2022 at 09:44:20PM +0800, Kefeng Wang wrote:
On 2022/1/19 21:22, Matthew Wilcox wrote:
quoted
On Wed, Jan 19, 2022 at 08:57:58PM +0800, Kefeng Wang wrote:
quoted
Only parts of our products wants this feature, we add some interfaces which
only
alloc hugevmalloc for them, eg,
vmap_hugepage/vmalloc_hugepage/remap_vmalloc_hugepage_range..
for our products, but it's not the choice of most products, also add
nohugevmalloc
for most products is expensive, so this is the reason for adding the patch.
more config/cmdline are more flexible for test/products,
But why do only some products want it? What goes wrong if all products
enable it? Features should be auto-tuning, not relying on admins to
understand them.
Because this feature will use more memory for vmalloc/vmap, that's why we
add some explicit
interfaces as said above in our kernel to control the user.
Have you validated that? What sort of performance penalty do you see?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel