Messages in this thread | | | From | Bart Van Assche <> | Subject | Re: [PATCH 5/7] block: Remove superflous rcu_read_[un]lock_sched() in blk_queue_enter() | Date | Wed, 14 Mar 2018 20:05:30 +0000 |
| |
On Wed, 2018-03-14 at 11:46 -0700, tj@kernel.org wrote: > > that are ordered with an RCU read lock (https://lwn.net/Articles/573497/). See > > also the following comment in scsi_device_quiesce(): > > > > /* > > * Ensure that the effect of blk_set_preempt_only() will be visible > > * for percpu_ref_tryget() callers that occur after the queue > > * unfreeze even if the queue was already frozen before this function > > * was called. See also https://lwn.net/Articles/573497/. > > */ > > > > Since this patch introduces a subtle and hard to debug race condition, please > > drop this patch. > > Hah, the pairing is between scsi_device_quiesce() and > blk_queue_enter()? But that doesn't make sense either because > scsi_device_quiesce() is doing regular synchronize_rcu() and > blk_queue_enter() is doing rcu_read_lock_sched(). They don't > interlock with each other in any way.
Can you clarify this further? From <linux/rcupdate.h>:
static inline void synchronize_rcu(void) { synchronize_sched(); }
> So, the right thing to do here would be somehow moving the RCU > synchronization into blk_set_preempt_only() and switching to regular > RCU protection in blk_queue_enter(). The code as-is isn't really > doing anything.
Since the RCU protection in blk_queue_enter() surrounds a percpu_ref_tryget_live() call and since percpu_ref_tryget_live() calls rcu_read_lock/unlock_sched() itself I don't think that it is allowed to switch to regular RCU protection in blk_queue_enter() without modifying the RCU implementation.
Bart.
| |