Hello, Bart. On Tue, Mar 06, 2018 at 05:52:50PM +0000, Bart Van Assche wrote: > I think the rcu_read_lock() and rcu_read_unlock() are really necessary in this > code. In the LWN article "The RCU-barrier menagerie" it is explained that RCU > can be used to enforce write ordering globally if the code that reads the > writes
Yeah but, for it to be meaningful, just like barriers, it has to be paired with something on the other side. > 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_entre() is doing rcu_read_lock_sched(). They don't interlock with each other in any way. 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. I'll drop this patch from the series for now. Let's revisit it later. Thanks. -- tejun
