Re: [PATCH 5/7] block: Remove superflous rcu_read_[un]lock_sched() in blk_queue_enter()

[Bart Van Assche <Bart.VanAssche@wdc.com>]

On Tue, 2018-03-06 at 09:33 -0800, Tejun Heo wrote:
> 3a0a529971ec ("block, scsi: Make SCSI quiesce and resume work
> reliably") added rcu_read_[un]lock_sched() to blk_queue_enter() along
> with other changes but it doesn't seem to be doing anything.
> 
> blk_queue_enter() is called with @q - the pointer to the target
> request_queue, so the caller obviously has to guarantee that @q can be
> dereferenced, and inside the RCU-sched protected area, there's nothing
> which needs further protection.
> 
> Let's remove the superflous rcu_read_[un]lock_sched().
> 
> [ ... ]
>
> This came up while auditing percpu_ref users for problematic RCU
> usages.  I couldn't understand what the RCU protection was doing.
> It'd be great if you can take a look and tell me whether I missed
> something.

Hello Tejun,

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
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.

Thanks,

Bart.