Messages in this thread | | | Date | Wed, 3 Apr 2024 20:09:21 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH 1/8] sparc32: make __cmpxchg_u32() return u32 |
| |
On Wed, Apr 03, 2024 at 11:20:53PM +0100, Al Viro wrote: > On Tue, Apr 02, 2024 at 01:03:13PM -0700, Paul E. McKenney wrote: > > On Tue, Apr 02, 2024 at 12:28:28AM -0400, Al Viro wrote: > > > Conversion between u32 and unsigned long is tautological there, > > > and the only use of return value is to return it from > > > __cmpxchg() (which return unsigned long). > > > > > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > > > > I have pulled these in as replacements for my patches in the meantime. > > > > Thank you! > > FWIW, updated branch force-pushed; the difference is that __cmpxchg() > on sparc32 went > - switch (size) { > - case 1: > - return __cmpxchg_u8((u8 *)ptr, (u8)old, (u8)new_); > - case 2: > - return __cmpxchg_u16((u16 *)ptr, (u16)old, (u16)new_); > - case 4: > - return __cmpxchg_u32((u32 *)ptr, (u32)old, (u32)new_); > - default: > - __cmpxchg_called_with_bad_pointer(); > - break; > - } > - return old; > + return > + size == 1 ? __cmpxchg_u8(ptr, old, new_) : > + size == 2 ? __cmpxchg_u16(ptr, old, new_) : > + size == 4 ? __cmpxchg_u32(ptr, old, new_) : > + (__cmpxchg_called_with_bad_pointer(), old); > > (and similar for parisc). Rationale: sparse does generate constant > truncation warnings in unreachable statements, but not in never-evaluated > subexpressions. Alternative would be what parisc used to do in mainline: > case 1: return __cmpxchg_u8((u8 *)ptr, old & 0xff, new_ & 0xff); > and we'd need the same in 16bit case (both on parisc and sparc32). > Explicit (and rather mysterious) & 0xff for passing unsigned long to > a function that takes u8 was there to tell sparse that e.g. > cmpxchg(&int_var, 0, 0x12345678) was *not* trying to feed > 0x12345678 to a __cmpxchg_u8(), which would quietly truncate it had > it ever been reached. Use of conditional expression avoids that > without having to play with explicit (and utterly pointless from > C point of view) masking. IMO it's better that way, not to mention > being more concise than use of switch.
Cute! I replaced the old versions of your patches with this series.
However, I was too lazy to apply this transformation to the other cmpxchg() implementations. ;-)
Thanx, Paul
| |