Re: [PATCH 1/3] mm: vmalloc: Let user to control huge vmalloc default behavior

From: Christophe Leroy
Date: Sun Dec 26 2021 - 12:37:14 EST




Le 26/12/2021 à 09:39, Kefeng Wang a écrit :
> Add HUGE_VMALLOC_DEFAULT_ENABLED to let user to choose whether or
> not enable huge vmalloc mappings by default, and this could make
> more architectures to enable huge vmalloc mappings feature but
> don't want to enable it by default.
>
> Add hugevmalloc=on/off parameter to enable or disable this feature
> at boot time, nohugevmalloc is still supported and equivalent to
> hugevmalloc=off.


Is there a real added value to have the user be able to select that ?

If the architecture supports it, is there any good reason to not use it ?

Why not just do like PPC and enable it by default ? Why should it be
enabled by default on PPC but disabled by default on ARM64 and X86 ?


>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> ---
> .../admin-guide/kernel-parameters.txt | 12 ++++++++++++
> arch/powerpc/Kconfig | 1 +
> mm/Kconfig | 7 +++++++
> mm/vmalloc.c | 18 +++++++++++++++++-
> 4 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2fba82431efb..4107136097a6 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1629,6 +1629,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/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index dea74d7717c0..d59b221be264 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -246,6 +246,7 @@ config PPC
> select HAVE_STATIC_CALL if PPC32
> select HAVE_SYSCALL_TRACEPOINTS
> select HAVE_VIRT_CPU_ACCOUNTING
> + select HUGE_VMALLOC_DEFAULT_ENABLED if HAVE_ARCH_HUGE_VMALLOC
> select HUGETLB_PAGE_SIZE_VARIABLE if PPC_BOOK3S_64 && HUGETLB_PAGE
> select IOMMU_HELPER if PPC64
> select IRQ_DOMAIN
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 356f4f2c779e..4ba91c0359bd 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -262,6 +262,13 @@ 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"
> + 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 d2a00ad4e1dd..3b6f99753816 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -58,7 +58,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)
> {
> @@ -66,6 +66,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 = true;
> + 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 */