* [PATCH] locking/qrwlock: queued_write_lock_slowpath() cleanup
@ 2021-04-26 18:50 Waiman Long
2021-04-26 18:51 ` Waiman Long
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Waiman Long @ 2021-04-26 18:50 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng
Cc: linux-kernel, Borislav Petkov, Ali Saidi, Steve Capper, Waiman Long
Make the code more readable by replacing the atomic_cmpxchg_acquire()
by an equivalent atomic_try_cmpxchg_acquire() and change atomic_add()
to atomic_or().
For architectures that use qrwlock, I do not find one that has an
atomic_add() defined but not an atomic_or(). I guess it should be fine
by changing atomic_add() to atomic_or().
Note that the previous use of atomic_add() isn't wrong as only one
writer that is the wait_lock owner can set the waiting flag and the
flag will be cleared later on when acquiring the write lock.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/locking/qrwlock.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c
index b94f3831e963..ec36b73f4733 100644
--- a/kernel/locking/qrwlock.c
+++ b/kernel/locking/qrwlock.c
@@ -66,12 +66,12 @@ void queued_write_lock_slowpath(struct qrwlock *lock)
arch_spin_lock(&lock->wait_lock);
/* Try to acquire the lock directly if no reader is present */
- if (!atomic_read(&lock->cnts) &&
- (atomic_cmpxchg_acquire(&lock->cnts, 0, _QW_LOCKED) == 0))
+ if (!(cnts = atomic_read(&lock->cnts)) &&
+ atomic_try_cmpxchg_acquire(&lock->cnts, &cnts, _QW_LOCKED))
goto unlock;
/* Set the waiting flag to notify readers that a writer is pending */
- atomic_add(_QW_WAITING, &lock->cnts);
+ atomic_or(_QW_WAITING, &lock->cnts);
/* When no more readers or writers, set the locked flag */
do {
--
2.18.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] locking/qrwlock: queued_write_lock_slowpath() cleanup
2021-04-26 18:50 [PATCH] locking/qrwlock: queued_write_lock_slowpath() cleanup Waiman Long
@ 2021-04-26 18:51 ` Waiman Long
2021-04-27 7:56 ` Will Deacon
2021-05-06 13:48 ` [tip: locking/urgent] locking/qrwlock: Cleanup queued_write_lock_slowpath() tip-bot2 for Waiman Long
2 siblings, 0 replies; 5+ messages in thread
From: Waiman Long @ 2021-04-26 18:51 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng
Cc: linux-kernel, Borislav Petkov, Ali Saidi, Steve Capper
On 4/26/21 2:50 PM, Waiman Long wrote:
> Make the code more readable by replacing the atomic_cmpxchg_acquire()
> by an equivalent atomic_try_cmpxchg_acquire() and change atomic_add()
> to atomic_or().
>
> For architectures that use qrwlock, I do not find one that has an
> atomic_add() defined but not an atomic_or(). I guess it should be fine
> by changing atomic_add() to atomic_or().
>
> Note that the previous use of atomic_add() isn't wrong as only one
> writer that is the wait_lock owner can set the waiting flag and the
> flag will be cleared later on when acquiring the write lock.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> kernel/locking/qrwlock.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c
> index b94f3831e963..ec36b73f4733 100644
> --- a/kernel/locking/qrwlock.c
> +++ b/kernel/locking/qrwlock.c
> @@ -66,12 +66,12 @@ void queued_write_lock_slowpath(struct qrwlock *lock)
> arch_spin_lock(&lock->wait_lock);
>
> /* Try to acquire the lock directly if no reader is present */
> - if (!atomic_read(&lock->cnts) &&
> - (atomic_cmpxchg_acquire(&lock->cnts, 0, _QW_LOCKED) == 0))
> + if (!(cnts = atomic_read(&lock->cnts)) &&
> + atomic_try_cmpxchg_acquire(&lock->cnts, &cnts, _QW_LOCKED))
> goto unlock;
>
> /* Set the waiting flag to notify readers that a writer is pending */
> - atomic_add(_QW_WAITING, &lock->cnts);
> + atomic_or(_QW_WAITING, &lock->cnts);
>
> /* When no more readers or writers, set the locked flag */
> do {
Sorry, I missed the "v2" in the title.
Cheers,
Longman
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] locking/qrwlock: queued_write_lock_slowpath() cleanup
2021-04-26 18:50 [PATCH] locking/qrwlock: queued_write_lock_slowpath() cleanup Waiman Long
2021-04-26 18:51 ` Waiman Long
@ 2021-04-27 7:56 ` Will Deacon
2021-05-04 9:19 ` Peter Zijlstra
2021-05-06 13:48 ` [tip: locking/urgent] locking/qrwlock: Cleanup queued_write_lock_slowpath() tip-bot2 for Waiman Long
2 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2021-04-27 7:56 UTC (permalink / raw)
To: Waiman Long
Cc: Peter Zijlstra, Ingo Molnar, Boqun Feng, linux-kernel,
Borislav Petkov, Ali Saidi, Steve Capper
On Mon, Apr 26, 2021 at 02:50:17PM -0400, Waiman Long wrote:
> Make the code more readable by replacing the atomic_cmpxchg_acquire()
> by an equivalent atomic_try_cmpxchg_acquire() and change atomic_add()
> to atomic_or().
>
> For architectures that use qrwlock, I do not find one that has an
> atomic_add() defined but not an atomic_or(). I guess it should be fine
> by changing atomic_add() to atomic_or().
>
> Note that the previous use of atomic_add() isn't wrong as only one
> writer that is the wait_lock owner can set the waiting flag and the
> flag will be cleared later on when acquiring the write lock.
Right, there's no functional change here.
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> kernel/locking/qrwlock.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c
> index b94f3831e963..ec36b73f4733 100644
> --- a/kernel/locking/qrwlock.c
> +++ b/kernel/locking/qrwlock.c
> @@ -66,12 +66,12 @@ void queued_write_lock_slowpath(struct qrwlock *lock)
> arch_spin_lock(&lock->wait_lock);
>
> /* Try to acquire the lock directly if no reader is present */
> - if (!atomic_read(&lock->cnts) &&
> - (atomic_cmpxchg_acquire(&lock->cnts, 0, _QW_LOCKED) == 0))
> + if (!(cnts = atomic_read(&lock->cnts)) &&
> + atomic_try_cmpxchg_acquire(&lock->cnts, &cnts, _QW_LOCKED))
> goto unlock;
>
> /* Set the waiting flag to notify readers that a writer is pending */
> - atomic_add(_QW_WAITING, &lock->cnts);
> + atomic_or(_QW_WAITING, &lock->cnts);
Acked-by: Will Deacon <will@kernel.org>
Will
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] locking/qrwlock: queued_write_lock_slowpath() cleanup
2021-04-27 7:56 ` Will Deacon
@ 2021-05-04 9:19 ` Peter Zijlstra
0 siblings, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2021-05-04 9:19 UTC (permalink / raw)
To: Will Deacon
Cc: Waiman Long, Ingo Molnar, Boqun Feng, linux-kernel,
Borislav Petkov, Ali Saidi, Steve Capper
On Tue, Apr 27, 2021 at 08:56:59AM +0100, Will Deacon wrote:
> On Mon, Apr 26, 2021 at 02:50:17PM -0400, Waiman Long wrote:
> > Make the code more readable by replacing the atomic_cmpxchg_acquire()
> > by an equivalent atomic_try_cmpxchg_acquire() and change atomic_add()
> > to atomic_or().
> >
> > For architectures that use qrwlock, I do not find one that has an
> > atomic_add() defined but not an atomic_or(). I guess it should be fine
> > by changing atomic_add() to atomic_or().
> >
> > Note that the previous use of atomic_add() isn't wrong as only one
> > writer that is the wait_lock owner can set the waiting flag and the
> > flag will be cleared later on when acquiring the write lock.
>
> Right, there's no functional change here.
>
> > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Waiman Long <longman@redhat.com>
> Acked-by: Will Deacon <will@kernel.org>
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: locking/urgent] locking/qrwlock: Cleanup queued_write_lock_slowpath()
2021-04-26 18:50 [PATCH] locking/qrwlock: queued_write_lock_slowpath() cleanup Waiman Long
2021-04-26 18:51 ` Waiman Long
2021-04-27 7:56 ` Will Deacon
@ 2021-05-06 13:48 ` tip-bot2 for Waiman Long
2 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for Waiman Long @ 2021-05-06 13:48 UTC (permalink / raw)
To: linux-tip-commits
Cc: Linus Torvalds, Waiman Long, Peter Zijlstra (Intel),
Will Deacon, x86, linux-kernel
The following commit has been merged into the locking/urgent branch of tip:
Commit-ID: 28ce0e70ecc30cc7d558a0304e6b816d70848f9a
Gitweb: https://git.kernel.org/tip/28ce0e70ecc30cc7d558a0304e6b816d70848f9a
Author: Waiman Long <longman@redhat.com>
AuthorDate: Mon, 26 Apr 2021 14:50:17 -04:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Thu, 06 May 2021 15:33:49 +02:00
locking/qrwlock: Cleanup queued_write_lock_slowpath()
Make the code more readable by replacing the atomic_cmpxchg_acquire()
by an equivalent atomic_try_cmpxchg_acquire() and change atomic_add()
to atomic_or().
For architectures that use qrwlock, I do not find one that has an
atomic_add() defined but not an atomic_or(). I guess it should be fine
by changing atomic_add() to atomic_or().
Note that the previous use of atomic_add() isn't wrong as only one
writer that is the wait_lock owner can set the waiting flag and the
flag will be cleared later on when acquiring the write lock.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Waiman Long <longman@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Will Deacon <will@kernel.org>
Link: https://lkml.kernel.org/r/20210426185017.19815-1-longman@redhat.com
---
kernel/locking/qrwlock.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c
index b94f383..ec36b73 100644
--- a/kernel/locking/qrwlock.c
+++ b/kernel/locking/qrwlock.c
@@ -66,12 +66,12 @@ void queued_write_lock_slowpath(struct qrwlock *lock)
arch_spin_lock(&lock->wait_lock);
/* Try to acquire the lock directly if no reader is present */
- if (!atomic_read(&lock->cnts) &&
- (atomic_cmpxchg_acquire(&lock->cnts, 0, _QW_LOCKED) == 0))
+ if (!(cnts = atomic_read(&lock->cnts)) &&
+ atomic_try_cmpxchg_acquire(&lock->cnts, &cnts, _QW_LOCKED))
goto unlock;
/* Set the waiting flag to notify readers that a writer is pending */
- atomic_add(_QW_WAITING, &lock->cnts);
+ atomic_or(_QW_WAITING, &lock->cnts);
/* When no more readers or writers, set the locked flag */
do {
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-06 13:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26 18:50 [PATCH] locking/qrwlock: queued_write_lock_slowpath() cleanup Waiman Long
2021-04-26 18:51 ` Waiman Long
2021-04-27 7:56 ` Will Deacon
2021-05-04 9:19 ` Peter Zijlstra
2021-05-06 13:48 ` [tip: locking/urgent] locking/qrwlock: Cleanup queued_write_lock_slowpath() tip-bot2 for Waiman Long
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.