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
