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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6c4bd989-268e-5899-09a7-ac573bd8b4d9@csgroup.eu>
Date:   Sun, 26 Dec 2021 17:36:59 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Kefeng Wang <wangkefeng.wang@...wei.com>,
        Jonathan Corbet <corbet@....net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
CC:     Catalin Marinas <catalin.marinas@....com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Nicholas Piggin <npiggin@...il.com>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Paul Mackerras <paulus@...ba.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <will@...nel.org>
Subject: Re: [PATCH 1/3] mm: vmalloc: Let user to control huge vmalloc default
 behavior



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@...wei.com>
> ---
>   .../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 */