2024-04-02 17:52:04

by Uros Bizjak

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

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 <[email protected]>
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 <[email protected]>
Cc: Nadav Amit <[email protected]>
Cc: Luc Van Oostenryck <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
---
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



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

The following commit has been merged into the x86/percpu branch of tip:

Commit-ID: a55c1fdad5f61b4bfe42319694b23671a758cb28
Gitweb: https://git.kernel.org/tip/a55c1fdad5f61b4bfe42319694b23671a758cb28
Author: Uros Bizjak <[email protected]>
AuthorDate: Tue, 02 Apr 2024 19:50:38 +02:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 03 Apr 2024 08:59:15 +02:00

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

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.

Fixes: 9a462b9eafa6 ("x86/percpu: Use compiler segment prefix qualifier")
Reported-by: Charlemagne Lasse <[email protected]>
Signed-off-by: Uros Bizjak <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Cc: Linus Torvalds <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

Closes: https://lore.kernel.org/lkml/CAFGhKbzev7W4aHwhFPWwMZQEHenVgZUj7=aunFieVqZg3mt14A@mail.gmail.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 1f6404e..20696df 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


2024-04-05 14:38:39

by Nadav Amit

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



> On 2 Apr 2024, at 20:50, Uros Bizjak <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Cc: Nadav Amit <[email protected]>
> Cc: Luc Van Oostenryck <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Brian Gerst <[email protected]>
> Cc: Denys Vlasenko <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Josh Poimboeuf <[email protected]>
> ---
> 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 <[email protected]>

2024-04-05 15:17:10

by Uros Bizjak

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

On Fri, Apr 5, 2024 at 4:38 PM Nadav Amit <[email protected]> wrote:
>
>
>
> > On 2 Apr 2024, at 20:50, Uros Bizjak <[email protected]> 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 <[email protected]>
> > 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 <[email protected]>
> > Cc: Nadav Amit <[email protected]>
> > Cc: Luc Van Oostenryck <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Andy Lutomirski <[email protected]>
> > Cc: Brian Gerst <[email protected]>
> > Cc: Denys Vlasenko <[email protected]>
> > Cc: H. Peter Anvin <[email protected]>
> > Cc: Linus Torvalds <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Josh Poimboeuf <[email protected]>
> > ---
> > 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/[email protected]/

Thanks,
Uros.