Messages in this thread | | | Subject | Re: [PATCH] qspinlock: use signed temporaries for cmpxchg | From | Waiman Long <> | Date | Mon, 26 Oct 2020 14:03:06 -0400 |
| |
On 10/26/20 12:57 PM, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > When building with W=2, the build log is flooded with > > include/asm-generic/qrwlock.h:65:56: warning: pointer targets in passing argument 2 of 'atomic_try_cmpxchg_acquire' differ in signedness [-Wpointer-sign] > include/asm-generic/qrwlock.h:92:53: warning: pointer targets in passing argument 2 of 'atomic_try_cmpxchg_acquire' differ in signedness [-Wpointer-sign] > include/asm-generic/qspinlock.h:68:55: warning: pointer targets in passing argument 2 of 'atomic_try_cmpxchg_acquire' differ in signedness [-Wpointer-sign] > include/asm-generic/qspinlock.h:82:52: warning: pointer targets in passing argument 2 of 'atomic_try_cmpxchg_acquire' differ in signedness [-Wpointer-sign] > > The atomics are built on top of signed integers, but the caller > doesn't actually care. Just use signed types as well. > > Fixes: 27df89689e25 ("locking/spinlocks: Remove an instruction from spin and write locks") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > include/asm-generic/qrwlock.h | 8 ++++---- > include/asm-generic/qspinlock.h | 4 ++-- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/include/asm-generic/qrwlock.h b/include/asm-generic/qrwlock.h > index 3aefde23dcea..84ce841ce735 100644 > --- a/include/asm-generic/qrwlock.h > +++ b/include/asm-generic/qrwlock.h > @@ -37,7 +37,7 @@ extern void queued_write_lock_slowpath(struct qrwlock *lock); > */ > static inline int queued_read_trylock(struct qrwlock *lock) > { > - u32 cnts; > + int cnts; > > cnts = atomic_read(&lock->cnts); > if (likely(!(cnts & _QW_WMASK))) { > @@ -56,7 +56,7 @@ static inline int queued_read_trylock(struct qrwlock *lock) > */ > static inline int queued_write_trylock(struct qrwlock *lock) > { > - u32 cnts; > + int cnts; > > cnts = atomic_read(&lock->cnts); > if (unlikely(cnts)) > @@ -71,7 +71,7 @@ static inline int queued_write_trylock(struct qrwlock *lock) > */ > static inline void queued_read_lock(struct qrwlock *lock) > { > - u32 cnts; > + int cnts; > > cnts = atomic_add_return_acquire(_QR_BIAS, &lock->cnts); > if (likely(!(cnts & _QW_WMASK))) > @@ -87,7 +87,7 @@ static inline void queued_read_lock(struct qrwlock *lock) > */ > static inline void queued_write_lock(struct qrwlock *lock) > { > - u32 cnts = 0; > + int cnts = 0; > /* Optimize for the unfair lock case where the fair flag is 0. */ > if (likely(atomic_try_cmpxchg_acquire(&lock->cnts, &cnts, _QW_LOCKED))) > return; > diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h > index 4fe7fd0fe834..d74b13825501 100644 > --- a/include/asm-generic/qspinlock.h > +++ b/include/asm-generic/qspinlock.h > @@ -60,7 +60,7 @@ static __always_inline int queued_spin_is_contended(struct qspinlock *lock) > */ > static __always_inline int queued_spin_trylock(struct qspinlock *lock) > { > - u32 val = atomic_read(&lock->val); > + int val = atomic_read(&lock->val); > > if (unlikely(val)) > return 0; > @@ -77,7 +77,7 @@ extern void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val); > */ > static __always_inline void queued_spin_lock(struct qspinlock *lock) > { > - u32 val = 0; > + int val = 0; > > if (likely(atomic_try_cmpxchg_acquire(&lock->val, &val, _Q_LOCKED_VAL))) > return;
Yes, it shouldn't really matter if the value is defined as int or u32. However, the only caveat that I see is queued_spin_lock_slowpath() is expecting a u32 argument. Maybe you should cast it back to (u32) when calling it.
Cheers, Longman
| |