Re: [PATCH 1/1] hugetlb: fix CONFIG_PADATA dependency for non-SMP system

Muchun Song
2024-02-04T07:44:37Z

On 2024/2/4 15:25, Gang Li wrote:
> Randy Dunlap and kernel test robot reported a warning:
>
> ```
> WARNING: unmet direct dependencies detected for PADATA
> Depends on [n]: SMP [=n]
> Selected by [y]:
> - HUGETLBFS [=y] && (X86 [=y] || SPARC64 || ARCH_SUPPORTS_HUGETLBFS [=n] || BROKEN [=n]) && (SYSFS [=y] || SYSCTL [=n])
> ```
>
> hugetlb parallelization depends on PADATA, and PADATA depends on SMP, so
> when the SMP config is disabled, the dependency of hugetlb on padata
> should be downgraded to single thread.
>
> Fixes: f2f635264b98 ("hugetlb: have CONFIG_HUGETLBFS select CONFIG_PADATA")
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Closes: https://lore.kernel.org/lkml/ec5dc528-2c3c-4444-9e88-d2c48395b433@infradead.org/
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202402020454.6EPkP1hi-lkp@intel.com/
> Signed-off-by: Gang Li <ligang.bdlg@bytedance.com>
> ```
> Hi Andrew, this fix patch is based on mm/mm-unstable.
> Thanks!
> ```
> ---
> fs/Kconfig | 2 +-
> mm/hugetlb.c | 9 ++++++++-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 3abc107ab2fbd..f2bc73fc0417e 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -261,7 +261,7 @@ menuconfig HUGETLBFS
> depends on X86 || SPARC64 || ARCH_SUPPORTS_HUGETLBFS || BROKEN
> depends on (SYSFS || SYSCTL)
> select MEMFD_CREATE
> - select PADATA
> + select PADATA if SMP

I don't think it is a clear way to fix this. If someone want to
use PADATA in a non-SMP system, he should be carefully to handle
the non-SMP case himself. I think the better way is to make PADATA
handle the non-SMP case, I think it should be easy for it, which
could just call ->thread_fn() many times instead of creating many
threads in the non-SMP case.

Thanks.

> help
> hugetlbfs is a filesystem backing for HugeTLB pages, based on
> ramfs. For architectures that support it, say Y here and read
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index bf3d5dfb921e6..1b01b244fb50b 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3457,6 +3457,7 @@ static void __init gather_bootmem_prealloc_node(unsigned long start, unsigned lo
>
> static void __init gather_bootmem_prealloc(void)
> {
> +#ifdef CONFIG_PADATA
> struct padata_mt_job job = {
> .thread_fn = gather_bootmem_prealloc_node,
> .fn_arg = NULL,
> @@ -3469,6 +3470,9 @@ static void __init gather_bootmem_prealloc(void)
> };
>
> padata_do_multithreaded(&job);
> +#else
> + gather_bootmem_prealloc_node(0, 0, NULL);
> +#endif
> }
>
> static void __init hugetlb_hstate_alloc_pages_onenode(struct hstate *h, int nid)
> @@ -3568,6 +3572,7 @@ static unsigned long __init hugetlb_gigantic_pages_alloc_boot(struct hstate *h)
>
> static unsigned long __init hugetlb_pages_alloc_boot(struct hstate *h)
> {
> +#ifdef CONFIG_PADATA
> struct padata_mt_job job = {
> .fn_arg = h,
> .align = 1,
> @@ -3600,7 +3605,9 @@ static unsigned long __init hugetlb_pages_alloc_boot(struct hstate *h)
> job.max_threads = num_node_state(N_MEMORY) * 2;
> job.min_chunk = h->max_huge_pages / num_node_state(N_MEMORY) / 2;
> padata_do_multithreaded(&job);
> -
> +#else
> + hugetlb_pages_alloc_boot_node(0, h->max_huge_pages, h);
> +#endif
> return h->nr_huge_pages;
> }
>