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

Heiko Stuebner posted 2 patches 2 years, 8 months ago
[PATCH v3 1/2] riscv: don't include kernel.h into alternative.h
Posted by Heiko Stuebner 2 years, 8 months ago
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

Re: [PATCH v3 1/2] riscv: don't include kernel.h into alternative.h
Posted by Andrew Jones 2 years, 8 months ago
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