Re: [PATCH] cpumask: Switch from inline to __always_inline

[Brian Norris]

Hi Yuri, Rasmus,

On Tue, May 14, 2024 at 01:49:01PM -0700, Brian Norris wrote:
> On recent (v6.6+) builds with Clang (based on Clang 18.0.0) and certain
> configurations [0], I'm finding that (lack of) inlining decisions may
> lead to section mismatch warnings like the following:
> 
>   WARNING: modpost: vmlinux.o: section mismatch in reference:
>   cpumask_andnot (section: .text) ->
>   cpuhp_bringup_cpus_parallel.tmp_mask (section: .init.data) ERROR:
>   modpost: Section mismatches detected.
> 
> or more confusingly:
> 
>   WARNING: modpost: vmlinux: section mismatch in reference:
>   cpumask_andnot+0x5f (section: .text) -> efi_systab_phys (section:
>   .init.data)
> 
> The first warning makes a little sense, because
> cpuhp_bringup_cpus_parallel() (an __init function) calls
> cpumask_andnot() on tmp_mask (an __initdata symbol). If the compiler
> doesn't inline cpumask_andnot(), this may appear like a mismatch.
> 
> The second warning makes less sense, but might be because efi_systab_phys
> and cpuhp_bringup_cpus_parallel.tmp_mask are laid out near each other,
> and the latter isn't a proper C symbol definition.
> 
> In any case, it seems a reasonable solution to suggest more strongly to
> the compiler that these cpumask macros *must* be inlined, as 'inline' is
> just a recommendation.
> 
> This change (plus more) has been previously proposed for other reasons
> -- that some of the bitmask 'const' machinery doesn't work without
> inlining -- in the past as:
> 
>   Subject: [PATCH 1/3] bitmap: switch from inline to __always_inline
>   https://lore.kernel.org/all/20221027043810.350460-2-yury.norov@xxxxxxxxx/
> 
> It seems like a good idea to at least make all cpumask functions use
> __always_inline; several already do.
> 
> According to bloat-o-meter, my ~29MB vmlinux increases by a total of 61
> bytes (0.00%) with this change.
> 
> [0] CONFIG_HOTPLUG_PARALLEL=y ('select'ed for x86 as of [1]) and
>     CONFIG_GCOV_PROFILE_ALL.
> 
> [1] commit 0c7ffa32dbd6 ("x86/smpboot/64: Implement
>     arch_cpuhp_init_parallel_bringup() and enable it")
> 
> Cc: Yury Norov <yury.norov@xxxxxxxxx>
> Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
> ---
> 
>  include/linux/cpumask.h | 214 +++++++++++++++++++++-------------------
>  1 file changed, 113 insertions(+), 101 deletions(-)

Any thoughts here? scripts/get_maintainer.pl suggests you are
maintainer/reviewer here.

Regards,
Brian