diff mbox series

[v2,1/3] mm: vmalloc: Let user to control huge vmalloc default behavior

Message ID 20211227145903.187152-2-wangkefeng.wang@huawei.com (mailing list archive)
State Handled Elsewhere
Headers show
Series mm: support huge vmalloc mapping on arm64/x86 | expand

Commit Message

Kefeng Wang Dec. 27, 2021, 2:59 p.m. UTC
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 <wangkefeng.wang@huawei.com>
---
 .../admin-guide/kernel-parameters.txt          | 12 ++++++++++++
 mm/Kconfig                                     |  8 ++++++++
 mm/vmalloc.c                                   | 18 +++++++++++++++++-
 3 files changed, 37 insertions(+), 1 deletion(-)

Comments

Nicholas Piggin Jan. 18, 2022, 2:52 a.m. UTC | #1
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
Kefeng Wang Jan. 19, 2022, 12:57 p.m. UTC | #2
On 2022/1/18 10:52, Nicholas Piggin wrote:
> 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.

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
> .
Matthew Wilcox Jan. 19, 2022, 1:22 p.m. UTC | #3
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.
Kefeng Wang Jan. 19, 2022, 1:44 p.m. UTC | #4
On 2022/1/19 21:22, Matthew Wilcox wrote:
> 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.

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.
Matthew Wilcox Jan. 19, 2022, 1:48 p.m. UTC | #5
On Wed, Jan 19, 2022 at 09:44:20PM +0800, Kefeng Wang wrote:
> 
> On 2022/1/19 21:22, Matthew Wilcox wrote:
> > 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.
> 
> 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?
diff mbox series

Patch

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a069d8fe2fee..7b2f900fd243 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -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
diff --git a/mm/Kconfig b/mm/Kconfig
index a99bd499ef51..8d8a92f22905 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -262,6 +262,14 @@  config HUGETLB_PAGE_SIZE_VARIABLE
 	  HUGETLB_PAGE_ORDER when there are multiple HugeTLB page sizes available
 	  on a platform.
 
+config HUGE_VMALLOC_DEFAULT_ENABLED
+	bool "Enable huge vmalloc mappings by default"
+	default y
+	depends on HAVE_ARCH_HUGE_VMALLOC
+	help
+	  Enable huge vmalloc mappings by default, this value could be overridden
+	  by hugevmalloc=off|on.
+
 config CONTIG_ALLOC
 	def_bool (MEMORY_ISOLATION && COMPACTION) || CMA
 
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 9bf838817a47..0d0f8deb5639 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -60,7 +60,7 @@  static const unsigned int ioremap_max_page_shift = PAGE_SHIFT;
 #endif	/* CONFIG_HAVE_ARCH_HUGE_VMAP */
 
 #ifdef CONFIG_HAVE_ARCH_HUGE_VMALLOC
-static bool __ro_after_init vmap_allow_huge = true;
+static bool __ro_after_init vmap_allow_huge = IS_ENABLED(CONFIG_HUGE_VMALLOC_DEFAULT_ENABLED);
 
 static int __init set_nohugevmalloc(char *str)
 {
@@ -68,6 +68,22 @@  static int __init set_nohugevmalloc(char *str)
 	return 0;
 }
 early_param("nohugevmalloc", set_nohugevmalloc);
+
+static int __init set_hugevmalloc(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!strcmp(str, "on"))
+		vmap_allow_huge = true;
+	else if (!strcmp(str, "off"))
+		vmap_allow_huge = false;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+early_param("hugevmalloc", set_hugevmalloc);
 #else /* CONFIG_HAVE_ARCH_HUGE_VMALLOC */
 static const bool vmap_allow_huge = false;
 #endif	/* CONFIG_HAVE_ARCH_HUGE_VMALLOC */