Re: [PATCH] cpumask: Switch from inline to __always_inline
From: Brian Norris
Date: Tue Jun 25 2024 - 14:28:09 EST
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