[PATCH] x86/percpu: fix cast in __pcpu_cast_* macros

[PATCH] x86/percpu: fix cast in __pcpu_cast_* macros

[Alexey Dobriyan]

Fix compile failure

	this_cpu_add(*p, x = 1);

kernel/test.c:7:29: error: lvalue required as left operand of assignment
    7 |         this_cpu_add(*p, x = 1);
      |                            ^
arch/x86/include/asm/percpu.h:134:51: note: in definition of macro '__pcpu_cast_1'
  134 | #define __pcpu_cast_1(val) ((u8)(((unsigned long) val) & 0xff))


This pattern is almost always wrong:

	#define M(x)	((T)x)

also because it can't be used with assignment expressions: cast makes
left part of it non-lvalue and assignment to non-lvalue has to break
compilation.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 arch/x86/include/asm/percpu.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -119,9 +119,9 @@
 #define __pcpu_type_4 u32
 #define __pcpu_type_8 u64
 
-#define __pcpu_cast_1(val) ((u8)(((unsigned long) val) & 0xff))
-#define __pcpu_cast_2(val) ((u16)(((unsigned long) val) & 0xffff))
-#define __pcpu_cast_4(val) ((u32)(((unsigned long) val) & 0xffffffff))
+#define __pcpu_cast_1(val) ((u8)((unsigned long)(val) & 0xff))
+#define __pcpu_cast_2(val) ((u16)((unsigned long)(val) & 0xffff))
+#define __pcpu_cast_4(val) ((u32)((unsigned long)(val) & 0xffffffff))
 #define __pcpu_cast_8(val) ((u64)(val))
 
 #define __pcpu_op1_1(op, dst) op "b " dst

Re: [PATCH] x86/percpu: fix cast in __pcpu_cast_* macros

[Dennis Zhou]

Hi Alexey,

On Tue, May 28, 2024 at 01:43:27PM -0700, Christoph Lameter (Ampere) wrote:
> On Mon, 27 May 2024, Alexey Dobriyan wrote:
> 
> > 	this_cpu_add(*p, x = 1);
> 
> A side effect in a this_cpu_op? Shudder....
> 

At the end of the day this is Thomas' code, but I can't imagine a
single use case where this is a good idea. Especially with how much fun
percpu macros end up being.

Thanks,
Dennis

Re: [PATCH] x86/percpu: fix cast in __pcpu_cast_* macros

[Thomas Gleixner]

On Mon, May 27 2024 at 08:51, Alexey Dobriyan wrote:
> Fix compile failure
>
> 	this_cpu_add(*p, x = 1);
>
> kernel/test.c:7:29: error: lvalue required as left operand of assignment
>     7 |         this_cpu_add(*p, x = 1);
>       |                            ^
> arch/x86/include/asm/percpu.h:134:51: note: in definition of macro '__pcpu_cast_1'
>   134 | #define __pcpu_cast_1(val) ((u8)(((unsigned long) val) & 0xff))
>
>
> This pattern is almost always wrong:
>
> 	#define M(x)	((T)x)
>
> also because it can't be used with assignment expressions: cast makes
> left part of it non-lvalue and assignment to non-lvalue has to break
> compilation.

In principle I agree with the change itself, but I really don't see the
point for

	 this_cpu_add(*p, x = 1);

Even if C supports it, it's just nasty and tasteless.

Thanks,

        tglx