2019-12-04 01:08:25

by Luc Van Oostenryck

[permalink] [raw]
Subject: [PATCH v2] fix __percpu annotation in asm-generic

The generic implementation of raw_cpu_generic_add_return() is:

#define raw_cpu_generic_add_return(pcp, val) \
({ \
typeof(&(pcp)) __p = raw_cpu_ptr(&(pcp)); \
\
*__p += val; \
*__p; \
})

where the 'pcp' argument is a __percpu lvalue.
There, the variable '__p' is declared as a __percpu pointer
the type of the address of 'pcp') but:
1) the value assigned to it, the return value of raw_cpu_ptr(), is
a plain (__kernel) pointer, not a __percpu one.
2) this variable is dereferenced just after while a __percpu
pointer is implicitly __noderef.

So, fix the declaration of the 'pcp' variable to its correct type:
the plain (non-percpu) pointer corresponding to pcp's address,
using the fact that typeof() ignores the address space and the
'noderef' attribute of its agument.

Same for raw_cpu_generic_xchg(), raw_cpu_generic_cmpxchg() &
raw_cpu_generic_cmpxchg_double().

This removes 209 warnings on ARM, 525 on ARM64, 220 on x86 &
more than 2600 on ppc64 (all of them with the default config).

Cc: Dennis Zhou <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Reported-by: Ben Dooks <[email protected]>
Signed-off-by: Luc Van Oostenryck <[email protected]>
---

Change since v1:
* use the fact that typeof() ignore the address space of its argument.

include/asm-generic/percpu.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/asm-generic/percpu.h b/include/asm-generic/percpu.h
index c2de013b2cf4..35e4a53b83e6 100644
--- a/include/asm-generic/percpu.h
+++ b/include/asm-generic/percpu.h
@@ -74,7 +74,7 @@ do { \

#define raw_cpu_generic_add_return(pcp, val) \
({ \
- typeof(&(pcp)) __p = raw_cpu_ptr(&(pcp)); \
+ typeof(pcp) *__p = raw_cpu_ptr(&(pcp)); \
\
*__p += val; \
*__p; \
@@ -82,7 +82,7 @@ do { \

#define raw_cpu_generic_xchg(pcp, nval) \
({ \
- typeof(&(pcp)) __p = raw_cpu_ptr(&(pcp)); \
+ typeof(pcp) *__p = raw_cpu_ptr(&(pcp)); \
typeof(pcp) __ret; \
__ret = *__p; \
*__p = nval; \
@@ -91,7 +91,7 @@ do { \

#define raw_cpu_generic_cmpxchg(pcp, oval, nval) \
({ \
- typeof(&(pcp)) __p = raw_cpu_ptr(&(pcp)); \
+ typeof(pcp) *__p = raw_cpu_ptr(&(pcp)); \
typeof(pcp) __ret; \
__ret = *__p; \
if (__ret == (oval)) \
@@ -101,8 +101,8 @@ do { \

#define raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
({ \
- typeof(&(pcp1)) __p1 = raw_cpu_ptr(&(pcp1)); \
- typeof(&(pcp2)) __p2 = raw_cpu_ptr(&(pcp2)); \
+ typeof(pcp1) *__p1 = raw_cpu_ptr(&(pcp1)); \
+ typeof(pcp2) *__p2 = raw_cpu_ptr(&(pcp2)); \
int __ret = 0; \
if (*__p1 == (oval1) && *__p2 == (oval2)) { \
*__p1 = nval1; \
--
2.24.0


Subject: Re: [PATCH v2] fix __percpu annotation in asm-generic

On Wed, 4 Dec 2019, Luc Van Oostenryck wrote:

> using the fact that typeof() ignores the address space and the
> 'noderef' attribute of its agument.

Acked-by: Christoph Lameter <[email protected]>

2019-12-04 22:30:19

by Dennis Zhou

[permalink] [raw]
Subject: Re: [PATCH v2] fix __percpu annotation in asm-generic

On Wed, Dec 04, 2019 at 02:06:23AM +0100, Luc Van Oostenryck wrote:
> The generic implementation of raw_cpu_generic_add_return() is:
>
> #define raw_cpu_generic_add_return(pcp, val) \
> ({ \
> typeof(&(pcp)) __p = raw_cpu_ptr(&(pcp)); \
> \
> *__p += val; \
> *__p; \
> })
>
> where the 'pcp' argument is a __percpu lvalue.
> There, the variable '__p' is declared as a __percpu pointer
> the type of the address of 'pcp') but:
> 1) the value assigned to it, the return value of raw_cpu_ptr(), is
> a plain (__kernel) pointer, not a __percpu one.
> 2) this variable is dereferenced just after while a __percpu
> pointer is implicitly __noderef.
>
> So, fix the declaration of the 'pcp' variable to its correct type:
> the plain (non-percpu) pointer corresponding to pcp's address,
> using the fact that typeof() ignores the address space and the
> 'noderef' attribute of its agument.
>
> Same for raw_cpu_generic_xchg(), raw_cpu_generic_cmpxchg() &
> raw_cpu_generic_cmpxchg_double().
>
> This removes 209 warnings on ARM, 525 on ARM64, 220 on x86 &
> more than 2600 on ppc64 (all of them with the default config).
>
> Cc: Dennis Zhou <[email protected]>
> Cc: Christoph Lameter <[email protected]>
> Cc: Tejun Heo <[email protected]>
> Cc: Nicholas Piggin <[email protected]>
> Reported-by: Ben Dooks <[email protected]>
> Signed-off-by: Luc Van Oostenryck <[email protected]>
> ---
>
> Change since v1:
> * use the fact that typeof() ignore the address space of its argument.
>
> include/asm-generic/percpu.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/asm-generic/percpu.h b/include/asm-generic/percpu.h
> index c2de013b2cf4..35e4a53b83e6 100644
> --- a/include/asm-generic/percpu.h
> +++ b/include/asm-generic/percpu.h
> @@ -74,7 +74,7 @@ do { \
>
> #define raw_cpu_generic_add_return(pcp, val) \
> ({ \
> - typeof(&(pcp)) __p = raw_cpu_ptr(&(pcp)); \
> + typeof(pcp) *__p = raw_cpu_ptr(&(pcp)); \
> \
> *__p += val; \
> *__p; \
> @@ -82,7 +82,7 @@ do { \
>
> #define raw_cpu_generic_xchg(pcp, nval) \
> ({ \
> - typeof(&(pcp)) __p = raw_cpu_ptr(&(pcp)); \
> + typeof(pcp) *__p = raw_cpu_ptr(&(pcp)); \
> typeof(pcp) __ret; \
> __ret = *__p; \
> *__p = nval; \
> @@ -91,7 +91,7 @@ do { \
>
> #define raw_cpu_generic_cmpxchg(pcp, oval, nval) \
> ({ \
> - typeof(&(pcp)) __p = raw_cpu_ptr(&(pcp)); \
> + typeof(pcp) *__p = raw_cpu_ptr(&(pcp)); \
> typeof(pcp) __ret; \
> __ret = *__p; \
> if (__ret == (oval)) \
> @@ -101,8 +101,8 @@ do { \
>
> #define raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
> ({ \
> - typeof(&(pcp1)) __p1 = raw_cpu_ptr(&(pcp1)); \
> - typeof(&(pcp2)) __p2 = raw_cpu_ptr(&(pcp2)); \
> + typeof(pcp1) *__p1 = raw_cpu_ptr(&(pcp1)); \
> + typeof(pcp2) *__p2 = raw_cpu_ptr(&(pcp2)); \
> int __ret = 0; \
> if (*__p1 == (oval1) && *__p2 == (oval2)) { \
> *__p1 = nval1; \
> --
> 2.24.0
>

I've applied this for-5.6.

Thanks,
Dennis