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

[Andrew Jones]

On Sun, May 21, 2023 at 01:47:14PM +0200, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@xxxxxxxx>
> 
> 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@xxxxxxxx>
> ---
>  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