On Thu, May 25, 2017 at 05:24:54AM +0000, Bart Van Assche wrote: > On Thu, 2017-05-25 at 12:21 +0800, Ming Lei wrote: > > One big problem of blk_mq_quiesce_queue() is that it > > can't prevent .queue_rq() in direct issue path from > > being run even though hw queues are stopped by > > blk_mq_quiesce_queue(). > > That's wrong. All what's needed to prevent that > __blk_mq_try_issue_directly() calls .queue_rq() for a stopped > queue is to check in __blk_mq_try_issue_directly() whether the That should work, and I just didn't want to introduce a check on global variable in fast path, but just figured out that it is inevitable for percpu_ref too. > relevant hardware queue has been stopped. That approach has the I don't think it is a good idea to abuse stopped state for checking queue quiesce, actually stopping queue has made lots of trouble, we should avoid it in blk_mq_quiesce_queue() in the future, even most of them in driver side. So I will introduce a queue quiesce flag like in patch 2 for this purpose. > following two advantages over the approach of your patch series: > - Lower code complexity and hence easier to review and to maintain. > - Faster for queues for which BLK_MQ_F_BLOCKING has not been set. > For non-preemptible kernels rcu_read_lock() and rcu_read_lock() > are optimized out. That is not possible for > percpu_ref_tryget_live() / percpu_ref_put(). That won't be issue for perpcu access at all. Another big issue is that 'srcu_struct' is very big, which shouldn't be embedded into hctx, since we only have one real user of BLK_MQ_F_BLOCKING. So I will fix that too. Thanks, Ming
