date:20221229

Re: [PATCH 02/19] arch/arc: rename internal name xchg to arch_xchg

2022-12-29 Thread Arnd Bergmann

On Thu, Dec 22, 2022, at 12:46, Andrzej Hajda wrote:
> __xchg will be used for non-atomic xchg macro.
>
> Signed-off-by: Andrzej Hajda 
> ---
>  arch/arc/include/asm/cmpxchg.h | 4 ++--

Reviewed-by: Arnd Bergmann 

for all the arch/*/include/asm/cmpxchg.h changes.

Since these patches are all the same, and they have identical
subject and description texts, I would suggest combining them
into a single patch to keep the series more compact.

Having them separate would allow merging the patches through
the individual architecture maintainer trees, but that in turn
would mean waiting longer to get it all merged, but in this
case it seems way easier to go through the asm-generic
tree.

 Arnd

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg

2022-12-29 Thread Andrzej Hajda


Forgive me late response - Holidays,

On 22.12.2022 18:21, Andrew Morton wrote:

On Thu, 22 Dec 2022 12:46:16 +0100 Andrzej Hajda  
wrote:


Hi all,

I hope there will be place for such tiny helper in kernel.
Quick cocci analyze shows there is probably few thousands places
where it could be useful.

So to clarify, the intent here is a simple readability cleanup for
existing open-coded exchange operations.


And replace private helpers with common one, see the last patch - the 
ultimate goal

would be to replace all occurrences of fetch_and_zero with __xchg.


The intent is *not* to
identify existing xchg() sites which are unnecessarily atomic and to
optimize them by using the non-atomic version.

Have you considered the latter?


If you mean some way of (semi-)automatic detection of such cases, then 
no. Anyway this could be quite interesting challenge.





I am not sure who is good person to review/ack such patches,

I can take 'em.


so I've used my intuition to construct to/cc lists, sorry for mistakes.
This is the 2nd approach of the same idea, with comments addressed[0].

The helper is tiny and there are advices we can leave without it, so
I want to present few arguments why it would be good to have it:

1. Code readability/simplification/number of lines:

Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
-   previous_min_rate = evport->qos.min_rate;
-   evport->qos.min_rate = min_rate;
+   previous_min_rate = __xchg(evport->qos.min_rate, min_rate);

For sure the code is more compact, and IMHO more readable.

2. Presence of similar helpers in other somehow related languages/libs:

a) Rust[1]: 'replace' from std::mem module, there is also 'take'
 helper (__xchg(&x, 0)), which is the same as private helper in
 i915 - fetch_and_zero, see latest patch.
b) C++ [2]: 'exchange' from utility header.

If the idea is OK there are still 2 qestions to answer:

1. Name of the helper, __xchg follows kernel conventions,
 but for me Rust names are also OK.

I like replace(), or, shockingly, exchange().

But...   Can we simply make swap() return the previous value?

previous_min_rate = swap(&evport->qos.min_rate, min_rate);


As Alexander already pointed out, swap requires 'references' to two 
variables,

in contrast to xchg which requires reference to variable and value.
So we cannot use swap for cases:
    old_value = __xchg(&x, new_value);

Regards
Andrzej


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

[PATCH v2] arch: rename all internal names xchg to arch_xchg

2022-12-29 Thread Andrzej Hajda

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda 
Reviewed-by: Arnd Bergmann 
---
 arch/alpha/include/asm/cmpxchg.h | 6 +++---
 arch/arc/include/asm/cmpxchg.h   | 4 ++--
 arch/arm/include/asm/cmpxchg.h   | 4 ++--
 arch/arm64/include/asm/cmpxchg.h | 4 ++--
 arch/hexagon/include/asm/cmpxchg.h   | 6 +++---
 arch/ia64/include/asm/cmpxchg.h  | 2 +-
 arch/ia64/include/uapi/asm/cmpxchg.h | 4 ++--
 arch/loongarch/include/asm/cmpxchg.h | 4 ++--
 arch/m68k/include/asm/cmpxchg.h  | 6 +++---
 arch/mips/include/asm/cmpxchg.h  | 4 ++--
 arch/openrisc/include/asm/cmpxchg.h  | 4 ++--
 arch/parisc/include/asm/cmpxchg.h| 4 ++--
 arch/powerpc/include/asm/cmpxchg.h   | 4 ++--
 arch/riscv/include/asm/atomic.h  | 2 +-
 arch/riscv/include/asm/cmpxchg.h | 4 ++--
 arch/s390/include/asm/cmpxchg.h  | 4 ++--
 arch/sh/include/asm/cmpxchg.h| 4 ++--
 arch/sparc/include/asm/cmpxchg_32.h  | 4 ++--
 arch/sparc/include/asm/cmpxchg_64.h  | 4 ++--
 arch/xtensa/include/asm/cmpxchg.h| 4 ++--
 20 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
index 6e0a850aa9d38c..40e8159ef6e794 100644
--- a/arch/alpha/include/asm/cmpxchg.h
+++ b/arch/alpha/include/asm/cmpxchg.h
@@ -6,7 +6,7 @@
  * Atomic exchange routines.
  */
 
-#define xchg(type, args...)__xchg ## type ## _local(args)
+#define xchg(type, args...)__arch_xchg ## type ## 
_local(args)
 #define cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
 #include 
 
@@ -34,7 +34,7 @@
 
 #undef xchg
 #undef cmpxchg
-#define xchg(type, args...)__xchg ##type(args)
+#define xchg(type, args...)__arch_xchg ##type(args)
 #define cmpxchg(type, args...) __cmpxchg ##type(args)
 #include 
 
@@ -48,7 +48,7 @@
__typeof__(*(ptr)) _x_ = (x);   \
smp_mb();   \
__ret = (__typeof__(*(ptr)))\
-   __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr)));  \
+   __arch_xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
smp_mb();   \
__ret;  \
 })
diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
index c5b544a5fe8106..e138fde067dea5 100644
--- a/arch/arc/include/asm/cmpxchg.h
+++ b/arch/arc/include/asm/cmpxchg.h
@@ -85,7 +85,7 @@
  */
 #ifdef CONFIG_ARC_HAS_LLSC
 
-#define __xchg(ptr, val)   \
+#define __arch_xchg(ptr, val)  \
 ({ \
__asm__ __volatile__(   \
"   ex  %0, [%1]\n" /* set new value */ \
@@ -102,7 +102,7 @@
\
switch(sizeof(*(_p_))) {\
case 4: \
-   _val_ = __xchg(_p_, _val_); \
+   _val_ = __arch_xchg(_p_, _val_);\
break;  \
default:\
BUILD_BUG();\
diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 4dfe538dfc689b..6953fc05a97886 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -25,7 +25,7 @@
 #define swp_is_buggy
 #endif
 
-static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int 
size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void *ptr, 
int size)
 {
extern void __bad_xchg(volatile void *, int);
unsigned long ret;
@@ -115,7 +115,7 @@ static inline unsigned long __xchg(unsigned long x, 
volatile void *ptr, int size
 }
 
 #define arch_xchg_relaxed(ptr, x) ({   \
-   (__typeof__(*(ptr)))__xchg((unsigned long)(x), (ptr),   \
+   (__typeof__(*(ptr)))__arch_xchg((unsigned long)(x), (ptr),  
\
   sizeof(*(ptr))); \
 })
 
diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index 497acf134d9923..3a36ba58e8c2ef 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -62,7 +62,7 @@ __XCHG_CASE( ,  ,  mb_, 64, dmb ish, nop,  , a, l, "memory")
 #undef __XCHG_CASE
 
 #define __XCHG_GEN(sfx)
\
-static __always_inline  unsigned l

Re: [PATCH 13/19] arch/riscv: rename internal name xchg to arch_xchg

2022-12-29 Thread Palmer Dabbelt


On Thu, 22 Dec 2022 03:46:29 PST (-0800), andrzej.ha...@intel.com wrote:

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda 
---
 arch/riscv/include/asm/atomic.h  | 2 +-
 arch/riscv/include/asm/cmpxchg.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 0dfe9d857a762b..bba472928b5393 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -261,7 +261,7 @@ c_t arch_atomic##prefix##_xchg_release(atomic##prefix##_t 
*v, c_t n)\
 static __always_inline \
 c_t arch_atomic##prefix##_xchg(atomic##prefix##_t *v, c_t n)   \
 {  \
-   return __xchg(&(v->counter), n, size);   \
+   return __arch_xchg(&(v->counter), n, size);  \
 }  \
 static __always_inline \
 c_t arch_atomic##prefix##_cmpxchg_relaxed(atomic##prefix##_t *v,   \
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 12debce235e52d..2f4726d3cfcc25 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -114,7 +114,7 @@
_x_, sizeof(*(ptr)));   \
 })

-#define __xchg(ptr, new, size) \
+#define __arch_xchg(ptr, new, size)\
 ({ \
__typeof__(ptr) __ptr = (ptr);  \
__typeof__(new) __new = (new);  \
@@ -143,7 +143,7 @@
 #define arch_xchg(ptr, x)  \
 ({ \
__typeof__(*(ptr)) _x_ = (x);   \
-   (__typeof__(*(ptr))) __xchg((ptr), _x_, sizeof(*(ptr)));\
+   (__typeof__(*(ptr))) __arch_xchg((ptr), _x_, sizeof(*(ptr)));   \
 })

 #define xchg32(ptr, x) \


Acked-by: Palmer Dabbelt 

Thanks!

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 02/19] arch/arc: rename internal name xchg to arch_xchg

Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg

[PATCH v2] arch: rename all internal names xchg to arch_xchg

Re: [PATCH 13/19] arch/riscv: rename internal name xchg to arch_xchg

4 matches

Site Navigation

Mail list logo

Footer information

Re: [PATCH 02/19] arch/arc: rename internal name __xchg to __arch_xchg

Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg

[PATCH v2] arch: rename all internal names __xchg to __arch_xchg

Re: [PATCH 13/19] arch/riscv: rename internal name __xchg to __arch_xchg

4 matches

Mail list logo

Re: [PATCH 02/19] arch/arc: rename internal name xchg to arch_xchg

[PATCH v2] arch: rename all internal names xchg to arch_xchg

Re: [PATCH 13/19] arch/riscv: rename internal name xchg to arch_xchg