Re: [PATCH v3 1/2] riscv: don't include kernel.h into alternative.h

Andrew Jones
2023-05-24T14:02:05Z

On Sun, May 21, 2023 at 01:47:14PM +0200, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> This include is not currently needed for alternatives and creates
> possible issues when we want to add alternatives to deeper kernel
> infrastructure.
>
> The issue in question came from trying to introduce Zawrs alternatives,
> which resulted in a somewhat circular dependency like:
>
> In file included from ../include/linux/bitops.h:34,
> from ../include/linux/kernel.h:22,
> from ../arch/riscv/include/asm/alternative.h:16,
> from ../arch/riscv/include/asm/errata_list.h:8,
> from ../arch/riscv/include/asm/barrier.h:15,
> from ../include/linux/list.h:11,
> from ../include/linux/preempt.h:11,
> from ../include/linux/spinlock.h:56,
> from ../include/linux/mmzone.h:8,
> from ../include/linux/gfp.h:7,
> from ../include/linux/mm.h:7,
> from ../arch/riscv/kernel/asm-offsets.c:10:
> ../include/asm-generic/bitops/generic-non-atomic.h: In function ‘generic_test_bit_acquire’:
> ../include/asm-generic/bitops/generic-non-atomic.h:140:23: error: implicit declaration of function ‘smp_load_acquire’ [-Werror=implicit-function-declaration]
> 140 | return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1)));
> | ^~~~~~~~~~~~~~~~
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
> arch/riscv/include/asm/alternative.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index 6a41537826a7..05885de6048c 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -13,7 +13,6 @@
> #ifdef CONFIG_RISCV_ALTERNATIVE
>
> #include <linux/init.h>
> -#include <linux/kernel.h>
> #include <linux/types.h>
> #include <linux/stddef.h>
> #include <asm/hwcap.h>
> --
> 2.39.0
>

Removing this include doesn't break compilation because the only callers
of PATCH_ID_CPUFEATURE_ID() and PATCH_ID_CPUFEATURE_VALUE(), which are
defined with lower/upper_16_bits(), are in arch/riscv/kernel/cpufeature.c,
which includes at least one thing which eventually includes linux/kernel.h
(the first path I found was linux/module.h -> linux/moduleparam.h ->
linux/kernel.h). Ideally we wouldn't rely on that luck. We can open
code the PATCH_ID_* macros to drop the lower/upper_16_bits() dependencies
or move the macros elsewhere, maybe, for now, just to
arch/riscv/kernel/cpufeature.c

Thanks,
drew