Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Steve Capper
Hiya, On 15/04/2021 17:45, Will Deacon wrote: On Thu, Apr 15, 2021 at 04:26:46PM +, Ali Saidi wrote: On Thu, 15 Apr 2021 16:02:29 +0100, Will Deacon wrote: On Thu, Apr 15, 2021 at 02:25:52PM +, Ali Saidi wrote: While this code is executed with the wait_lock held, a reader can acqui

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Waiman Long
On 4/15/21 12:45 PM, Will Deacon wrote: With that in mind, it would probably be a good idea to eyeball the qspinlock slowpath as well, as that uses both atomic_cond_read_acquire() and atomic_try_cmpxchg_relaxed(). It seems plausible that the same thing could occur here in qspinlock:

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Will Deacon
On Thu, Apr 15, 2021 at 04:26:46PM +, Ali Saidi wrote: > > On Thu, 15 Apr 2021 16:02:29 +0100, Will Deacon wrote: > > On Thu, Apr 15, 2021 at 02:25:52PM +, Ali Saidi wrote: > > > While this code is executed with the wait_lock held, a reader can > > > acquire the lock without holding wait_l

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Ali Saidi
On Thu, 15 Apr 2021 16:02:29 +0100, Will Deacon wrote: > On Thu, Apr 15, 2021 at 02:25:52PM +, Ali Saidi wrote: > > While this code is executed with the wait_lock held, a reader can > > acquire the lock without holding wait_lock. The writer side loops > > checking the value with the atomic_c

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Peter Zijlstra
On Thu, Apr 15, 2021 at 04:28:21PM +0100, Will Deacon wrote: > On Thu, Apr 15, 2021 at 05:03:58PM +0200, Peter Zijlstra wrote: > > @@ -73,9 +75,8 @@ void queued_write_lock_slowpath(struct qrwlock *lock) > > > > /* When no more readers or writers, set the locked flag */ > > do { > > -

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Will Deacon
On Thu, Apr 15, 2021 at 04:37:58PM +0100, Catalin Marinas wrote: > On Thu, Apr 15, 2021 at 04:28:21PM +0100, Will Deacon wrote: > > On Thu, Apr 15, 2021 at 05:03:58PM +0200, Peter Zijlstra wrote: > > > diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c > > > index 4786dd271b45..22aecc

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Catalin Marinas
On Thu, Apr 15, 2021 at 04:28:21PM +0100, Will Deacon wrote: > On Thu, Apr 15, 2021 at 05:03:58PM +0200, Peter Zijlstra wrote: > > On Thu, Apr 15, 2021 at 02:25:52PM +, Ali Saidi wrote: > > > While this code is executed with the wait_lock held, a reader can > > > acquire the lock without holdin

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Will Deacon
On Thu, Apr 15, 2021 at 05:03:58PM +0200, Peter Zijlstra wrote: > On Thu, Apr 15, 2021 at 02:25:52PM +, Ali Saidi wrote: > > While this code is executed with the wait_lock held, a reader can > > acquire the lock without holding wait_lock. The writer side loops > > checking the value with the a

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Waiman Long
On 4/15/21 10:25 AM, Ali Saidi wrote: While this code is executed with the wait_lock held, a reader can acquire the lock without holding wait_lock. The writer side loops checking the value with the atomic_cond_read_acquire(), but only truly acquires the lock when the compare-and-exchange is comp

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Peter Zijlstra
On Thu, Apr 15, 2021 at 02:25:52PM +, Ali Saidi wrote: > While this code is executed with the wait_lock held, a reader can > acquire the lock without holding wait_lock. The writer side loops > checking the value with the atomic_cond_read_acquire(), but only truly > acquires the lock when the c

Re: [PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Will Deacon
On Thu, Apr 15, 2021 at 02:25:52PM +, Ali Saidi wrote: > While this code is executed with the wait_lock held, a reader can > acquire the lock without holding wait_lock. The writer side loops > checking the value with the atomic_cond_read_acquire(), but only truly > acquires the lock when the c

[PATCH] locking/qrwlock: Fix ordering in queued_write_lock_slowpath

2021-04-15 Thread Ali Saidi
While this code is executed with the wait_lock held, a reader can acquire the lock without holding wait_lock. The writer side loops checking the value with the atomic_cond_read_acquire(), but only truly acquires the lock when the compare-and-exchange is completed successfully which isn’t ordered.