2023-05-21 12:30:26

by Heiko Stuebner

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

From: Heiko Stuebner <[email protected]>

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 <[email protected]>
---
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



2023-05-24 14:12:14

by Andrew Jones

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

On Sun, May 21, 2023 at 01:47:14PM +0200, Heiko Stuebner wrote:
> From: Heiko Stuebner <[email protected]>
>
> 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 <[email protected]>
> ---
> 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