Re: [PATCH 1/2] block: move queue quiesce into elevator_change()

[Ming Lei <ming.lei@redhat.com>]

On Thu, May 08, 2025 at 12:35:21AM +0530, Nilay Shroff wrote:
> 
> 
> On 5/7/25 7:58 PM, Ming Lei wrote:
> > On Wed, May 07, 2025 at 03:53:49PM +0200, Christoph Hellwig wrote:
> >> On Wed, May 07, 2025 at 08:04:02PM +0800, Ming Lei wrote:
> >>> blk_mq_freeze_queue() can't be called on quiesced queue, otherwise it may
> >>> never return if there is any queued requests.
> >>>
> >>> Fix it by moving queue quiesce int elevator_change() by adding one flag to
> >>> 'struct elv_change_ctx' for controlling this behavior.
> >>
> >> Why do we even need to quiesce the queue here, and not anywhere else?
> > 
> > Quiesce is for draining the in-progress critical area, which can't be
> > covered by queue freeze. Typically, all requests are freed, the run queue
> > activity isn't finished yet, so schedule data can be touched by the un-finished
> > code path.
> > 
> > We did fix this kind of bugs by queue quiesce several times.
> > 
> Technically after freezing the queue, we don't have any in-flight request when 
> blk_mq_freeze_queue returns. Yes we may still have some dispatch operations
> running and so we want to wait for it to finish. In that case, we may just call
> synchronize_rcu/synchronize_srcu (instead of blk_mq_quiesce_queue) to ensure that
> all in-progress dispatch operations are finished. That way we can also avoid
> calling blk_mq_unquiesce_queue later when we finish switching elevator. 

synchronize_rcu/synchronize_srcu can't be better than blk_mq_quiesce_queue,
because it can't prevent from entering new rcu/srcu read lock critical area.


Thanks,
Ming