[PATCH] x86/percpu: Use __force to cast from __percpu address space

[PATCH] x86/percpu: Use __force to cast from __percpu address space

[Uros Bizjak]

commit a55c1fdad5f61b4bfe42319694b23671a758cb28 upstream.

Fix Sparse warning when casting from __percpu address space by using
__force in the cast. x86 named address spaces are not considered to
be subspaces of the generic (flat) address space, so explicit casts
are required to convert pointers between these address spaces and the
generic address space (the application should cast to uintptr_t and
apply the segment base offset). The cast to uintptr_t removes
__percpu address space tag and Sparse reports:

  warning: cast removes address space '__percpu' of expression

Use __force to inform Sparse that the cast is intentional.

The patch deviates from upstream commit due to the unification of
arch_raw_cpu_ptr() defines in the commit:

  4e5b0e8003df ("x86/percpu: Unify arch_raw_cpu_ptr() defines").

Fixes: 9a462b9eafa6 ("x86/percpu: Use compiler segment prefix qualifier")
Reported-by: Charlemagne Lasse <charlemagnelasse@gmail.com>
Closes: https://lore.kernel.org/lkml/CAFGhKbzev7W4aHwhFPWwMZQEHenVgZUj7=aunFieVqZg3mt14A@mail.gmail.com/
Cc: stable@vger.kernel.org # v6.8
Link: https://lore.kernel.org/r/20240402175058.52649-1-ubizjak@gmail.com
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
---
 arch/x86/include/asm/percpu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 44958ebaf626..66ed36b8cdb4 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -70,7 +70,7 @@
 	unsigned long tcp_ptr__;				\
 	tcp_ptr__ = __raw_cpu_read(, this_cpu_off);		\
 								\
-	tcp_ptr__ += (unsigned long)(ptr);			\
+	tcp_ptr__ += (__force unsigned long)(ptr);		\
 	(typeof(*(ptr)) __kernel __force *)tcp_ptr__;		\
 })
 #else /* CONFIG_USE_X86_SEG_SUPPORT */
@@ -85,7 +85,7 @@
 	     : "=r" (tcp_ptr__)					\
 	     : "m" (__my_cpu_var(this_cpu_off)));		\
 								\
-	tcp_ptr__ += (unsigned long)(ptr);			\
+	tcp_ptr__ += (__force unsigned long)(ptr);		\
 	(typeof(*(ptr)) __kernel __force *)tcp_ptr__;		\
 })
 #endif /* CONFIG_USE_X86_SEG_SUPPORT */
@@ -102,8 +102,8 @@
 #endif /* CONFIG_SMP */
 
 #define __my_cpu_type(var)	typeof(var) __percpu_seg_override
-#define __my_cpu_ptr(ptr)	(__my_cpu_type(*ptr) *)(uintptr_t)(ptr)
-#define __my_cpu_var(var)	(*__my_cpu_ptr(&var))
+#define __my_cpu_ptr(ptr)	(__my_cpu_type(*(ptr))*)(__force uintptr_t)(ptr)
+#define __my_cpu_var(var)	(*__my_cpu_ptr(&(var)))
 #define __percpu_arg(x)		__percpu_prefix "%" #x
 #define __force_percpu_arg(x)	__force_percpu_prefix "%" #x
 
-- 
2.45.0

Re: [PATCH] x86/percpu: Use __force to cast from __percpu address space

[Greg KH]

On Tue, May 14, 2024 at 10:39:18AM +0200, Uros Bizjak wrote:
> commit a55c1fdad5f61b4bfe42319694b23671a758cb28 upstream.
> 
> Fix Sparse warning when casting from __percpu address space by using
> __force in the cast. x86 named address spaces are not considered to
> be subspaces of the generic (flat) address space, so explicit casts
> are required to convert pointers between these address spaces and the
> generic address space (the application should cast to uintptr_t and
> apply the segment base offset). The cast to uintptr_t removes
> __percpu address space tag and Sparse reports:
> 
>   warning: cast removes address space '__percpu' of expression
> 
> Use __force to inform Sparse that the cast is intentional.

Why is a fix for sparse required for stable kernels?

> The patch deviates from upstream commit due to the unification of
> arch_raw_cpu_ptr() defines in the commit:
> 
>   4e5b0e8003df ("x86/percpu: Unify arch_raw_cpu_ptr() defines").
> 
> Fixes: 9a462b9eafa6 ("x86/percpu: Use compiler segment prefix qualifier")
> Reported-by: Charlemagne Lasse <charlemagnelasse@gmail.com>
> Closes: https://lore.kernel.org/lkml/CAFGhKbzev7W4aHwhFPWwMZQEHenVgZUj7=aunFieVqZg3mt14A@mail.gmail.com/
> Cc: stable@vger.kernel.org # v6.8
> Link: https://lore.kernel.org/r/20240402175058.52649-1-ubizjak@gmail.com
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> ---
>  arch/x86/include/asm/percpu.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

And also, what kernel version(s) is this for?

I don't see this in any released kernels yet either, is that
intentional?

thanks,

greg k-h

Re: [PATCH] x86/percpu: Use __force to cast from __percpu address space

[Uros Bizjak]

On Wed, May 15, 2024 at 9:32 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, May 14, 2024 at 10:39:18AM +0200, Uros Bizjak wrote:
> > commit a55c1fdad5f61b4bfe42319694b23671a758cb28 upstream.
> >
> > Fix Sparse warning when casting from __percpu address space by using
> > __force in the cast. x86 named address spaces are not considered to
> > be subspaces of the generic (flat) address space, so explicit casts
> > are required to convert pointers between these address spaces and the
> > generic address space (the application should cast to uintptr_t and
> > apply the segment base offset). The cast to uintptr_t removes
> > __percpu address space tag and Sparse reports:
> >
> >   warning: cast removes address space '__percpu' of expression
> >
> > Use __force to inform Sparse that the cast is intentional.
>
> Why is a fix for sparse required for stable kernels?

Named address spaces is a new feature in the 6.8 kernel. When someone
compiles this version with Sparse (and certain sparse parameters), it
will spew many sparse warnings. We have fixed this in the tip tree
(so, the fix will be in v6.9), but the tip tree diverted from the
mainline in this area, so it was not possible to fix the issue in 6.8
via "urgent" tip branches.

I thought that the fix falls into "some “oh, that’s not good” issue"
category (due to many sparse warnings). Also, the fix is
straightforward with a low possibility of breaking something.

> > The patch deviates from upstream commit due to the unification of
> > arch_raw_cpu_ptr() defines in the commit:
> >
> >   4e5b0e8003df ("x86/percpu: Unify arch_raw_cpu_ptr() defines").
> >
> > Fixes: 9a462b9eafa6 ("x86/percpu: Use compiler segment prefix qualifier")
> > Reported-by: Charlemagne Lasse <charlemagnelasse@gmail.com>
> > Closes: https://lore.kernel.org/lkml/CAFGhKbzev7W4aHwhFPWwMZQEHenVgZUj7=aunFieVqZg3mt14A@mail.gmail.com/
> > Cc: stable@vger.kernel.org # v6.8
> > Link: https://lore.kernel.org/r/20240402175058.52649-1-ubizjak@gmail.com
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > ---
> >  arch/x86/include/asm/percpu.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
>
> And also, what kernel version(s) is this for?

As instructed in the "Procedure for submitting patches to the -stable
tree", it is stated at:

Cc: stable@vger.kernel.org # v6.8

>
> I don't see this in any released kernels yet either, is that
> intentional?

The original fix was committed to the current mainline and will be in
v6.9, but please also see the above reasoning. However, it is your
call, so if you think that this issue is not problematic for a stable
kernel, it's also OK with me.

Thanks,
Uros.

Re: [PATCH] x86/percpu: Use __force to cast from __percpu address space

[Uros Bizjak]

On Wed, May 15, 2024 at 10:03 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Wed, May 15, 2024 at 9:32 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, May 14, 2024 at 10:39:18AM +0200, Uros Bizjak wrote:
> > > commit a55c1fdad5f61b4bfe42319694b23671a758cb28 upstream.
> > >
> > > Fix Sparse warning when casting from __percpu address space by using
> > > __force in the cast. x86 named address spaces are not considered to
> > > be subspaces of the generic (flat) address space, so explicit casts
> > > are required to convert pointers between these address spaces and the
> > > generic address space (the application should cast to uintptr_t and
> > > apply the segment base offset). The cast to uintptr_t removes
> > > __percpu address space tag and Sparse reports:
> > >
> > >   warning: cast removes address space '__percpu' of expression
> > >
> > > Use __force to inform Sparse that the cast is intentional.
> >
> > Why is a fix for sparse required for stable kernels?
>
> Named address spaces is a new feature in the 6.8 kernel. When someone
> compiles this version with Sparse (and certain sparse parameters), it
> will spew many sparse warnings. We have fixed this in the tip tree
> (so, the fix will be in v6.9), but the tip tree diverted from the
> mainline in this area, so it was not possible to fix the issue in 6.8
> via "urgent" tip branches.
>
> I thought that the fix falls into "some “oh, that’s not good” issue"
> category (due to many sparse warnings). Also, the fix is
> straightforward with a low possibility of breaking something.
>
> > > The patch deviates from upstream commit due to the unification of
> > > arch_raw_cpu_ptr() defines in the commit:
> > >
> > >   4e5b0e8003df ("x86/percpu: Unify arch_raw_cpu_ptr() defines").
> > >
> > > Fixes: 9a462b9eafa6 ("x86/percpu: Use compiler segment prefix qualifier")
> > > Reported-by: Charlemagne Lasse <charlemagnelasse@gmail.com>
> > > Closes: https://lore.kernel.org/lkml/CAFGhKbzev7W4aHwhFPWwMZQEHenVgZUj7=aunFieVqZg3mt14A@mail.gmail.com/
> > > Cc: stable@vger.kernel.org # v6.8
> > > Link: https://lore.kernel.org/r/20240402175058.52649-1-ubizjak@gmail.com
> > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > > ---
> > >  arch/x86/include/asm/percpu.h | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > And also, what kernel version(s) is this for?
>
> As instructed in the "Procedure for submitting patches to the -stable
> tree", it is stated at:
>
> Cc: stable@vger.kernel.org # v6.8

Oh, I mixed up kernel versions... this should apply to just released v6.9.

> >
> > I don't see this in any released kernels yet either, is that
> > intentional?
>
> The original fix was committed to the current mainline and will be in
> v6.9, but please also see the above reasoning. However, it is your

The original fix will be in v6.10, I mixed up the number ...

> call, so if you think that this issue is not problematic for a stable
> kernel, it's also OK with me.

Thanks,
Uros.

[PATCH] x86/percpu: Use __force to cast from __percpu address space

[Uros Bizjak]

Fix sparse warning when casting from __percpu address space by using
__force in the cast. x86 named address spaces are not considered to
be subspaces of the generic (flat) address space, so explicit casts
are required to convert pointers between these address spaces and the
generic address space (the application should cast to uintptr_t and
apply the segment base offset). The cast to uintptr_t removes
__percpu address space tag and sparse reports:

  warning: cast removes address space '__percpu' of expression

Use __force to inform sparse that the cast is intentional.

Reported-by: Charlemagne Lasse <charlemagnelasse@gmail.com>
Closes: https://lore.kernel.org/lkml/CAFGhKbzev7W4aHwhFPWwMZQEHenVgZUj7=aunFieVqZg3mt14A@mail.gmail.com/
Fixes: 9a462b9eafa6 ("x86/percpu: Use compiler segment prefix qualifier")
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Nadav Amit <nadav.amit@gmail.com>
Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/percpu.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 1f6404e0c428..20696df5d567 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -77,7 +77,7 @@
 #define arch_raw_cpu_ptr(_ptr)					\
 ({								\
 	unsigned long tcp_ptr__ = __raw_my_cpu_offset;		\
-	tcp_ptr__ += (unsigned long)(_ptr);			\
+	tcp_ptr__ += (__force unsigned long)(_ptr);		\
 	(typeof(*(_ptr)) __kernel __force *)tcp_ptr__;		\
 })
 #else
@@ -96,8 +96,8 @@
 #endif /* CONFIG_SMP */
 
 #define __my_cpu_type(var)	typeof(var) __percpu_seg_override
-#define __my_cpu_ptr(ptr)	(__my_cpu_type(*ptr) *)(uintptr_t)(ptr)
-#define __my_cpu_var(var)	(*__my_cpu_ptr(&var))
+#define __my_cpu_ptr(ptr)	(__my_cpu_type(*ptr)*)(__force uintptr_t)(ptr)
+#define __my_cpu_var(var)	(*__my_cpu_ptr(&(var)))
 #define __percpu_arg(x)		__percpu_prefix "%" #x
 #define __force_percpu_arg(x)	__force_percpu_prefix "%" #x
 
-- 
2.42.0

Re: [PATCH] x86/percpu: Use __force to cast from __percpu address space

[Nadav Amit]

> On 2 Apr 2024, at 20:50, Uros Bizjak <ubizjak@gmail.com> wrote:
> 
> Fix sparse warning when casting from __percpu address space by using
> __force in the cast. x86 named address spaces are not considered to
> be subspaces of the generic (flat) address space, so explicit casts
> are required to convert pointers between these address spaces and the
> generic address space (the application should cast to uintptr_t and
> apply the segment base offset). The cast to uintptr_t removes
> __percpu address space tag and sparse reports:
> 
>  warning: cast removes address space '__percpu' of expression
> 
> Use __force to inform sparse that the cast is intentional.
> 
> Reported-by: Charlemagne Lasse <charlemagnelasse@gmail.com>
> Closes: https://lore.kernel.org/lkml/CAFGhKbzev7W4aHwhFPWwMZQEHenVgZUj7=aunFieVqZg3mt14A@mail.gmail.com/
> Fixes: 9a462b9eafa6 ("x86/percpu: Use compiler segment prefix qualifier")
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Cc: Nadav Amit <nadav.amit@gmail.com>
> Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: Denys Vlasenko <dvlasenk@redhat.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
> arch/x86/include/asm/percpu.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> index 1f6404e0c428..20696df5d567 100644
> --- a/arch/x86/include/asm/percpu.h
> +++ b/arch/x86/include/asm/percpu.h
> @@ -77,7 +77,7 @@
> #define arch_raw_cpu_ptr(_ptr) \
> ({ \
> unsigned long tcp_ptr__ = __raw_my_cpu_offset; \
> - tcp_ptr__ += (unsigned long)(_ptr); \
> + tcp_ptr__ += (__force unsigned long)(_ptr); \
> (typeof(*(_ptr)) __kernel __force *)tcp_ptr__; \
> })
> #else
> @@ -96,8 +96,8 @@
> #endif /* CONFIG_SMP */
> 
> #define __my_cpu_type(var) typeof(var) __percpu_seg_override
> -#define __my_cpu_ptr(ptr) (__my_cpu_type(*ptr) *)(uintptr_t)(ptr)
> -#define __my_cpu_var(var) (*__my_cpu_ptr(&var))
> +#define __my_cpu_ptr(ptr) (__my_cpu_type(*ptr)*)(__force uintptr_t)(ptr)
> +#define __my_cpu_var(var) (*__my_cpu_ptr(&(var)))

If you use "(var)” in __my_cpu_var(),, you might just as well change the first to:

  #define __my_cpu_ptr(ptr) (__my_cpu_type(*(ptr))*)(__force uintptr_t)(ptr)

[ not that I think of any real issue it might cause - just for consistency]

Anyhow,

Asked-by: Nadav Amit <nadav.amit@gmail.com>

Re: [PATCH] x86/percpu: Use __force to cast from __percpu address space

[Uros Bizjak]

On Fri, Apr 5, 2024 at 4:38 PM Nadav Amit <nadav.amit@gmail.com> wrote:
>
>
>
> > On 2 Apr 2024, at 20:50, Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > Fix sparse warning when casting from __percpu address space by using
> > __force in the cast. x86 named address spaces are not considered to
> > be subspaces of the generic (flat) address space, so explicit casts
> > are required to convert pointers between these address spaces and the
> > generic address space (the application should cast to uintptr_t and
> > apply the segment base offset). The cast to uintptr_t removes
> > __percpu address space tag and sparse reports:
> >
> >  warning: cast removes address space '__percpu' of expression
> >
> > Use __force to inform sparse that the cast is intentional.
> >
> > Reported-by: Charlemagne Lasse <charlemagnelasse@gmail.com>
> > Closes: https://lore.kernel.org/lkml/CAFGhKbzev7W4aHwhFPWwMZQEHenVgZUj7=aunFieVqZg3mt14A@mail.gmail.com/
> > Fixes: 9a462b9eafa6 ("x86/percpu: Use compiler segment prefix qualifier")
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Cc: Nadav Amit <nadav.amit@gmail.com>
> > Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Brian Gerst <brgerst@gmail.com>
> > Cc: Denys Vlasenko <dvlasenk@redhat.com>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> > ---
> > arch/x86/include/asm/percpu.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> > index 1f6404e0c428..20696df5d567 100644
> > --- a/arch/x86/include/asm/percpu.h
> > +++ b/arch/x86/include/asm/percpu.h
> > @@ -77,7 +77,7 @@
> > #define arch_raw_cpu_ptr(_ptr) \
> > ({ \
> > unsigned long tcp_ptr__ = __raw_my_cpu_offset; \
> > - tcp_ptr__ += (unsigned long)(_ptr); \
> > + tcp_ptr__ += (__force unsigned long)(_ptr); \
> > (typeof(*(_ptr)) __kernel __force *)tcp_ptr__; \
> > })
> > #else
> > @@ -96,8 +96,8 @@
> > #endif /* CONFIG_SMP */
> >
> > #define __my_cpu_type(var) typeof(var) __percpu_seg_override
> > -#define __my_cpu_ptr(ptr) (__my_cpu_type(*ptr) *)(uintptr_t)(ptr)
> > -#define __my_cpu_var(var) (*__my_cpu_ptr(&var))
> > +#define __my_cpu_ptr(ptr) (__my_cpu_type(*ptr)*)(__force uintptr_t)(ptr)
> > +#define __my_cpu_var(var) (*__my_cpu_ptr(&(var)))
>
> If you use "(var)” in __my_cpu_var(),, you might just as well change the first to:
>
>   #define __my_cpu_ptr(ptr) (__my_cpu_type(*(ptr))*)(__force uintptr_t)(ptr)
>
> [ not that I think of any real issue it might cause - just for consistency]

Yes, I have changed it in one of the follow-up percpu patches [1].

[1] https://lore.kernel.org/lkml/20240404094218.448963-2-ubizjak@gmail.com/

Thanks,
Uros.