Re: [PATCH v3] blk-mq-sched: separate mark hctx and queue restart operations
From: Jens Axboe <axboe@fb.com>
Date: 2017-02-15 16:54:19
On 02/15/2017 09:45 AM, Omar Sandoval wrote:
From: Omar Sandoval <redacted> In blk_mq_sched_dispatch_requests(), we call blk_mq_sched_mark_restart() after we dispatch requests left over on our hardware queue dispatch list. This is so we'll go back and dispatch requests from the scheduler. In this case, it's only necessary to restart the hardware queue that we are running; there's no reason to run other hardware queues just because we are using shared tags. So, split out blk_mq_sched_mark_restart() into two operations, one for just the hardware queue and one for the whole request queue. The core code uses both, and I/O schedulers may also want to use them. This also requires adjusting blk_mq_sched_restart_queues() to always check the queue restart flag, not just when using shared tags.
Looks good to me - just one comment:
quoted hunk ↗ jump to hunk
@@ -936,7 +936,10 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list) * in case the needed IO completed right before we * marked the queue as needing a restart. */ - blk_mq_sched_mark_restart(hctx); + if (hctx->flags & BLK_MQ_F_TAG_SHARED) + blk_mq_sched_mark_restart_queue(hctx); + else + blk_mq_sched_mark_restart_hctx(hctx); if (!blk_mq_get_driver_tag(rq, &hctx, false)) break; }
Since we now pushed the SHARED tag into the caller, I think this warrants a comment as to why the two cases are different (getting a driver tag can fail with 0 pending IOs for SHARED). Just update the existing comment. -- Jens Axboe