Re: [PATCH v2 2/5] powerpc/mm: drop powerpc vmemmap_can_optimize()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] mm: undeclared vmemmap in section_vmemmap_optimization_disable()
- [Low] mm: define vmemmap_optimizable_order() before section_vmemmap_optimizable()
--

commit e51c76b2ae2d335e25edd03780c815eb467b564b
Author: Muchun Song <muchun.song@linux.dev>

powerpc/mm: drop powerpc vmemmap_can_optimize()

This commit adds section_vmemmap_optimization_disable() to disable vmemmap
optimization in architecture fallback paths such as PowerPC Hash MMU, and
removes the PowerPC-specific vmemmap_can_optimize() override.

[ ... ]

> diff --git a/include/linux/vmemmap-optimization.h b/include/linux/vmemmap-optimization.h
> index 858311a71b0cc..5c09da4df4d83 100644
> --- a/include/linux/vmemmap-optimization.h
> +++ b/include/linux/vmemmap-optimization.h
> @@ -57,6 +57,27 @@ static inline void section_set_compound_order_range(unsigned long pfn,
>  		section_set_compound_order(__nr_to_section(section_nr + i), order);
>  }
>  
> +static inline bool section_vmemmap_optimizable(const struct mem_section *section)
> +{
> +	return vmemmap_optimizable_order(section_compound_order(section));
> +}

[Severity: Low]
Will moving section_vmemmap_optimizable() here break the build at this
commit when CONFIG_VMEMMAP_OPTIMIZATION is enabled?

At this point in include/linux/vmemmap-optimization.h,
vmemmap_optimizable_order() has not been declared yet and is only defined at
the end of the header after the #endif for CONFIG_VMEMMAP_OPTIMIZATION:

include/linux/vmemmap-optimization.h:
    static inline bool vmemmap_optimizable_order(unsigned int order)
    {
        if (!IS_ENABLED(CONFIG_VMEMMAP_OPTIMIZATION))
            return false;
        ...
    }

Compiling translation units that include linux/vmemmap-optimization.h with
CONFIG_VMEMMAP_OPTIMIZATION=y fails with an implicit declaration of
vmemmap_optimizable_order() followed by a static declaration following a
non-static declaration.

A later commit in the series ("mm/sparse-vmemmap: drop
vmemmap_can_optimize()") moves vmemmap_optimizable_order() above the
#ifdef CONFIG_VMEMMAP_OPTIMIZATION block. Could that move of
vmemmap_optimizable_order() be done in this patch instead to keep the series
bisectable?

> +
> +static inline void section_vmemmap_optimization_disable(unsigned long start,
> +		unsigned long end)
> +{
> +	unsigned long pfn = page_to_pfn((struct page *)start);

[Severity: Low]
Could calling page_to_pfn() inside section_vmemmap_optimization_disable()
cause a build failure on architectures such as LoongArch that include
linux/vmemmap-optimization.h before defining vmemmap?

When CONFIG_VMEMMAP_OPTIMIZATION (and therefore CONFIG_SPARSEMEM_VMEMMAP) is
enabled, page_to_pfn() in include/asm-generic/memory_model.h expands to
reference vmemmap:

    #define __pfn_to_page(pfn)	(vmemmap + (pfn))
    #define __page_to_pfn(page)	(unsigned long)((page) - vmemmap)

However, include/linux/vmemmap-optimization.h does not include a header that
defines vmemmap, and arch/loongarch/include/asm/pgtable.h includes
linux/vmemmap-optimization.h to obtain MAX_FOLIO_VMEMMAP_ALIGN before it
defines the vmemmap macro:

arch/loongarch/include/asm/pgtable.h:
    #include <linux/mm_types.h>
    #include <linux/mmzone.h>
    #include <linux/vmemmap-optimization.h>
    #include <asm/fixmap.h>
    #include <asm/sparsemem.h>
    ...
    #define VMEMMAP_ALIGN	max(PMD_SIZE, MAX_FOLIO_VMEMMAP_ALIGN)
    #define vmemmap		((struct page *)(ALIGN(VMALLOC_END, VMEMMAP_ALIGN)))

Because section_vmemmap_optimization_disable() is a static inline function
parsed before the vmemmap macro is defined, doesn't vmemmap remain an
undeclared identifier in page_to_pfn() when compiling on LoongArch with
CONFIG_VMEMMAP_OPTIMIZATION enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105061217.13302-1-songmuchun@bytedance.com?part=2