On Sat, Mar 30, 2024 at 12:57 AM Charlemagne Lasse
<[email protected]> wrote:
>
> After switching to linux 6.9-rc1, I get a lot of these errors (when
> compiling with cgcc/sparse):
>
> ./include/linux/netdevice.h:4033:17: warning: cast removes address
> space '__percpu' of expression
>
> This is around code which wasn't changed and which correctly uses the
> per cpu helper. Sparse flags were -Wsparse-all for sparse 0.6.4
> (latest release). Sparse was enabled via C=1 parameter and sparse was
> configured using CHECK="sparse -Wsparse-all"
>
> Problem was introduced between commit 8ae292c66dcb ("x86/lib: Address
> kernel-doc warnings") and 3a1d3829e193 ("x86/percpu: Avoid sparse
> warning with cast to named address space").
>
> I would even go as far as saying that 1ca3683cc6d2 ("x86/percpu:
> Enable named address spaces with known compiler version") together
> with 3a1d3829e193 ("x86/percpu: Avoid sparse warning with cast to
> named address space") triggered this problem
Are you sure this is the offending commit?
The complexity of x86 low-level code forced the whole x86 percpu
rewrite to be designed and written in such a way that it can be
disabled in a single place. So, using the attached patch will switch
x86 percpu functionality back to the previous implementation - and I
get the same warnings from the old implementation as when new named
address spaces are enabled:
security/selinux/netif.c: note: in included file:
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
/include/linux/netdevice.h:4041:17: warning: cast removes address
space '__percpu' of expression
4037 static inline void __dev_put(struct net_device *dev)
4038 {
4039 if (dev) {
4040 #ifdef CONFIG_PCPU_DEV_REFCNT
4041 this_cpu_dec(*dev->pcpu_refcnt);
4042 #else
4043 refcount_dec(&dev->dev_refcnt);
4044 #endif
4045 }
4046 }
Uros.
Am Mo., 1. Apr. 2024 um 21:16 Uhr schrieb Uros Bizjak <[email protected]>:
> > I would even go as far as saying that 1ca3683cc6d2 ("x86/percpu:
> > Enable named address spaces with known compiler version") together
> > with 3a1d3829e193 ("x86/percpu: Avoid sparse warning with cast to
> > named address space") triggered this problem
I think 1ca3683cc6d2 was wrong and is the last working one.
Just switch to 1ca3683cc6d2c2ce4204df519c4e4730d037905a and you won't
see the messages.
```
git reset --hard 1ca3683cc6d2c2ce4204df519c4e4730d037905a
git clean -dfx
make allnoconfig -j$(nproc)
make kvm_guest.config
make prepare -j$(nproc)
touch include/linux/netdevice.h
make C=1 net/core/dev.o CHECK="sparse -Wcast-from-as"
```
Go to 9a462b9eafa6dda16ea8429b151edb1fb535d744 and cherry-pick
3a1d3829e193c091475ceab481c5f8deab385023 and you would see the error.
On amd64 with 12.2.0, this would look like this:
```
git reset --hard 9a462b9eafa6dda16ea8429b151edb1fb535d744
git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
git clean -dfx
make allnoconfig -j$(nproc)
make kvm_guest.config
make prepare -j$(nproc)
touch include/linux/netdevice.h
make C=1 net/core/dev.o CHECK="sparse -Wcast-from-as"
```
I would recommend to use `-Wsparse-all` for testing but for this
demonstration, it is easier to use `-Wcast-from-as` to reduce the
amount of noise in the demonstrator.
On Tue, Apr 2, 2024 at 9:56 AM Charlemagne Lasse
<[email protected]> wrote:
>
> Am Mo., 1. Apr. 2024 um 21:16 Uhr schrieb Uros Bizjak <[email protected]>:
> > > I would even go as far as saying that 1ca3683cc6d2 ("x86/percpu:
> > > Enable named address spaces with known compiler version") together
> > > with 3a1d3829e193 ("x86/percpu: Avoid sparse warning with cast to
> > > named address space") triggered this problem
>
> I think 1ca3683cc6d2 was wrong and is the last working one.
>
>
> Just switch to 1ca3683cc6d2c2ce4204df519c4e4730d037905a and you won't
> see the messages.
>
> ```
> git reset --hard 1ca3683cc6d2c2ce4204df519c4e4730d037905a
> git clean -dfx
> make allnoconfig -j$(nproc)
> make kvm_guest.config
> make prepare -j$(nproc)
> touch include/linux/netdevice.h
> make C=1 net/core/dev.o CHECK="sparse -Wcast-from-as"
> ```
>
> Go to 9a462b9eafa6dda16ea8429b151edb1fb535d744 and cherry-pick
> 3a1d3829e193c091475ceab481c5f8deab385023 and you would see the error.
> On amd64 with 12.2.0, this would look like this:
>
> ```
> git reset --hard 9a462b9eafa6dda16ea8429b151edb1fb535d744
> git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
> git clean -dfx
> make allnoconfig -j$(nproc)
> make kvm_guest.config
> make prepare -j$(nproc)
> touch include/linux/netdevice.h
> make C=1 net/core/dev.o CHECK="sparse -Wcast-from-as"
> ```
>
> I would recommend to use `-Wsparse-all` for testing but for this
> demonstration, it is easier to use `-Wcast-from-as` to reduce the
> amount of noise in the demonstrator.
Oh, I see the problem now. We *do* cast away from __percpu space, this
is how we switch between GCC's named address spaces [1]:
--q--
6.17.5 x86 Named Address Spaces
..
The respective segment base must be set via some method specific to
the operating system. Rather than require an expensive system call to
retrieve the segment base, these address spaces are not considered to
be subspaces of the generic (flat) address space. This means that
explicit casts are required to convert pointers between these address
spaces and the generic address space. In practice the application
should cast to uintptr_t and apply the segment base offset that it
installed previously.
--/q--
Please try the attached patch that informs sparse about this action.
BTW: Please also note recent discussion about different checks for
__percpu name space that can be implemented using GCC's named address
spaces feature [2].
[1] https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html
[2[ https://lore.kernel.org/lkml/87bk7ux4e9.ffs@tglx/#t
Thanks,
Uros.
Am Di., 2. Apr. 2024 um 11:43 Uhr schrieb Uros Bizjak <[email protected]>:
[snip]
> Please try the attached patch that informs sparse about this action.
I've tested it using
```
git reset --hard v6.9-rc2
patch -p1 -i ~/p.diff.txt
git clean -dfx
make allnoconfig -j$(nproc)
make kvm_guest.config
make prepare -j$(nproc)
touch include/linux/netdevice.h
make C=1 net/core/dev.o CHECK="sparse -Wcast-from-as"
```
And it seems to work. Thanks
Am Di., 2. Apr. 2024 um 11:53 Uhr schrieb Charlemagne Lasse
<[email protected]>:
>
> Am Di., 2. Apr. 2024 um 11:43 Uhr schrieb Uros Bizjak <[email protected]>:
> [snip]
> > Please try the attached patch that informs sparse about this action.
>
> I've tested it using
>
> ```
> git reset --hard v6.9-rc2
> patch -p1 -i ~/p.diff.txt
> git clean -dfx
> make allnoconfig -j$(nproc)
> make kvm_guest.config
> make prepare -j$(nproc)
> touch include/linux/netdevice.h
> make C=1 net/core/dev.o CHECK="sparse -Wcast-from-as"
> ```
>
> And it seems to work. Thanks
But I found another problem which seem to have been introduced by
commit ed2f752e0e0a ("x86/percpu: Introduce const-qualified
const_pcpu_hot to micro-optimize code generation")
```
git reset --hard ed2f752e0e0a21d941ca0ee539ef3d4cd576bc5e
git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
patch -p1 -i ~/p.diff.txt
git clean -dfx
make allnoconfig -j$(nproc)
make kvm_guest.config
echo CONFIG_MODULES=y >> .config
echo CONFIG_NET_9P_VIRTIO=m >> .config
make olddefconfig
make prepare -j$(nproc)
touch net/9p/trans_virtio.o
make C=1 M=net/9p/ trans_virtio.o CHECK="sparse -Wconstexpr-not-const"
```
This now shows the warning:
```
net/9p/trans_virtio.c:831:1: warning: non-constant initializer for static object
net/9p/trans_virtio.c:832:1: warning: non-constant initializer for static object
```
Which is from
```
module_init(p9_virtio_init);
module_exit(p9_virtio_cleanup);
```
The same happens when directly switching to the mentioned commit:
```
git reset --hard ed2f752e0e0a21d941ca0ee539ef3d4cd576bc5e
git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
patch -p1 -i ~/p.diff.txt
git clean -dfx
make allnoconfig -j$(nproc)
make kvm_guest.config
echo CONFIG_MODULES=y >> .config
echo CONFIG_NET_9P_VIRTIO=m >> .config
make olddefconfig
make prepare -j$(nproc)
touch net/9p/trans_virtio.o
make C=1 M=net/9p/ trans_virtio.o CHECK="sparse -Wconstexpr-not-const"
```
So something for module_init and module_exit changed with this commit.
I can't see this when switching to a version before this commit:
```
git reset --hard ed2f752e0e0a21d941ca0ee539ef3d4cd576bc5e~1
git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
git clean -dfx
make allnoconfig -j$(nproc)
make kvm_guest.config
echo CONFIG_MODULES=y >> .config
echo CONFIG_NET_9P_VIRTIO=m >> .config
make olddefconfig
make prepare -j$(nproc)
touch net/9p/trans_virtio.o
make C=1 M=net/9p/ trans_virtio.o CHECK="sparse -Wconstexpr-not-const"
```
On Tue, Apr 2, 2024 at 10:10 PM Charlemagne Lasse
<[email protected]> wrote:
>
> Am Di., 2. Apr. 2024 um 11:53 Uhr schrieb Charlemagne Lasse
> <[email protected]>:
> >
> > Am Di., 2. Apr. 2024 um 11:43 Uhr schrieb Uros Bizjak <[email protected]>:
> > [snip]
> > > Please try the attached patch that informs sparse about this action.
> >
> > I've tested it using
> >
> > ```
> > git reset --hard v6.9-rc2
> > patch -p1 -i ~/p.diff.txt
> > git clean -dfx
> > make allnoconfig -j$(nproc)
> > make kvm_guest.config
> > make prepare -j$(nproc)
> > touch include/linux/netdevice.h
> > make C=1 net/core/dev.o CHECK="sparse -Wcast-from-as"
> > ```
> >
> > And it seems to work. Thanks
>
> But I found another problem which seem to have been introduced by
> commit ed2f752e0e0a ("x86/percpu: Introduce const-qualified
> const_pcpu_hot to micro-optimize code generation")
>
>
> ```
> git reset --hard ed2f752e0e0a21d941ca0ee539ef3d4cd576bc5e
> git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
> patch -p1 -i ~/p.diff.txt
> git clean -dfx
> make allnoconfig -j$(nproc)
> make kvm_guest.config
> echo CONFIG_MODULES=y >> .config
> echo CONFIG_NET_9P_VIRTIO=m >> .config
> make olddefconfig
> make prepare -j$(nproc)
> touch net/9p/trans_virtio.o
> make C=1 M=net/9p/ trans_virtio.o CHECK="sparse -Wconstexpr-not-const"
> ```
>
> This now shows the warning:
>
> ```
> net/9p/trans_virtio.c:831:1: warning: non-constant initializer for static object
> net/9p/trans_virtio.c:832:1: warning: non-constant initializer for static object
> ```
>
> Which is from
>
> ```
> module_init(p9_virtio_init);
> module_exit(p9_virtio_cleanup);
> ```
>
> The same happens when directly switching to the mentioned commit:
>
> ```
> git reset --hard ed2f752e0e0a21d941ca0ee539ef3d4cd576bc5e
> git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
> patch -p1 -i ~/p.diff.txt
> git clean -dfx
> make allnoconfig -j$(nproc)
> make kvm_guest.config
> echo CONFIG_MODULES=y >> .config
> echo CONFIG_NET_9P_VIRTIO=m >> .config
> make olddefconfig
> make prepare -j$(nproc)
> touch net/9p/trans_virtio.o
> make C=1 M=net/9p/ trans_virtio.o CHECK="sparse -Wconstexpr-not-const"
> ```
>
>
> So something for module_init and module_exit changed with this commit.
> I can't see this when switching to a version before this commit:
>
> ```
> git reset --hard ed2f752e0e0a21d941ca0ee539ef3d4cd576bc5e~1
> git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
> git clean -dfx
> make allnoconfig -j$(nproc)
> make kvm_guest.config
> echo CONFIG_MODULES=y >> .config
> echo CONFIG_NET_9P_VIRTIO=m >> .config
> make olddefconfig
> make prepare -j$(nproc)
> touch net/9p/trans_virtio.o
> make C=1 M=net/9p/ trans_virtio.o CHECK="sparse -Wconstexpr-not-const"
> ```
It's this part:
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index d7779a18b24fc3..bf9815eaf4aabf 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -212,7 +212,7 @@ void ftrace_likely_update(struct
ftrace_likely_data *f, int val,
*/
#define ___ADDRESSABLE(sym, __attrs) \
static void * __used __attrs \
- __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)&sym;
+ __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
#define __ADDRESSABLE(sym) \
___ADDRESSABLE(sym, __section(".discard.addressable"))
But ... how is this not const?
Uros.
Am Di., 2. Apr. 2024 um 22:40 Uhr schrieb Uros Bizjak <[email protected]>:
[snip]
> > ```
> > git reset --hard ed2f752e0e0a21d941ca0ee539ef3d4cd576bc5e
> > git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
> > patch -p1 -i ~/p.diff.txt
> > git clean -dfx
> > make allnoconfig -j$(nproc)
> > make kvm_guest.config
> > echo CONFIG_MODULES=y >> .config
> > echo CONFIG_NET_9P_VIRTIO=m >> .config
> > make olddefconfig
> > make prepare -j$(nproc)
> > touch net/9p/trans_virtio.c
> > make C=1 M=net/9p/ trans_virtio.o CHECK="sparse -Wconstexpr-not-const"
> > ```
> >
> > This now shows the warning:
> >
> > ```
> > net/9p/trans_virtio.c:831:1: warning: non-constant initializer for static object
> > net/9p/trans_virtio.c:832:1: warning: non-constant initializer for static object
> > ```
[snip]
> It's this part:
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index d7779a18b24fc3..bf9815eaf4aabf 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -212,7 +212,7 @@ void ftrace_likely_update(struct
> ftrace_likely_data *f, int val,
> */
> #define ___ADDRESSABLE(sym, __attrs) \
> static void * __used __attrs \
> - __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)&sym;
> + __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
> #define __ADDRESSABLE(sym) \
> ___ADDRESSABLE(sym, __section(".discard.addressable"))
>
> But ... how is this not const?
@Luc Van Oostenryck Do you have any idea how to correctly implement it
to make sparse happy?
Am Mi., 3. Apr. 2024 um 09:14 Uhr schrieb Charlemagne Lasse
<[email protected]>:
>
> Am Di., 2. Apr. 2024 um 22:40 Uhr schrieb Uros Bizjak <[email protected]>:
> [snip]
> > > ```
> > > git reset --hard ed2f752e0e0a21d941ca0ee539ef3d4cd576bc5e
> > > git cherry-pick 3a1d3829e193c091475ceab481c5f8deab385023
> > > patch -p1 -i ~/p.diff.txt
> > > git clean -dfx
> > > make allnoconfig -j$(nproc)
> > > make kvm_guest.config
> > > echo CONFIG_MODULES=y >> .config
> > > echo CONFIG_NET_9P_VIRTIO=m >> .config
> > > make olddefconfig
> > > make prepare -j$(nproc)
> > > touch net/9p/trans_virtio.c
> > > make C=1 M=net/9p/ trans_virtio.o CHECK="sparse -Wconstexpr-not-const"
> > > ```
> > >
> > > This now shows the warning:
> > >
> > > ```
> > > net/9p/trans_virtio.c:831:1: warning: non-constant initializer for static object
> > > net/9p/trans_virtio.c:832:1: warning: non-constant initializer for static object
> > > ```
> [snip]
> > It's this part:
> >
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > index d7779a18b24fc3..bf9815eaf4aabf 100644
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -212,7 +212,7 @@ void ftrace_likely_update(struct
> > ftrace_likely_data *f, int val,
> > */
> > #define ___ADDRESSABLE(sym, __attrs) \
> > static void * __used __attrs \
> > - __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)&sym;
> > + __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
> > #define __ADDRESSABLE(sym) \
> > ___ADDRESSABLE(sym, __section(".discard.addressable"))
> >
> > But ... how is this not const?
>
> @Luc Van Oostenryck Do you have any idea how to correctly implement it
> to make sparse happy?
ping?