| Message ID | 20210527010134.32448-8-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve I/O priority support | expand |
On 2021/05/27 10:02, Bart Van Assche wrote: > For interactive workloads it is important that synchronous requests are > not delayed. Hence reserve 25% of tags for synchronous requests. This patch s/tags/scheduler tags to be clear that we are not talking about the device tags. Same in the patch title may be. > still allows asynchronous requests to fill the hardware queues since > blk_mq_init_sched() makes sure that the number of scheduler requests is the > double of the hardware queue depth. From blk_mq_init_sched(): > > q->nr_requests = 2 * min_t(unsigned int, q->tag_set->queue_depth, > BLKDEV_MAX_RQ); > > Cc: Damien Le Moal <damien.lemoal@wdc.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/mq-deadline.c | 46 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/block/mq-deadline.c b/block/mq-deadline.c > index 2ab844a4b6b5..81f487d77e09 100644 > --- a/block/mq-deadline.c > +++ b/block/mq-deadline.c > @@ -69,6 +69,7 @@ struct deadline_data { > int fifo_batch; > int writes_starved; > int front_merges; > + u32 async_depth; > > spinlock_t lock; > spinlock_t zone_lock; > @@ -399,6 +400,38 @@ static struct request *dd_dispatch_request(struct blk_mq_hw_ctx *hctx) > return rq; > } > > +static void dd_limit_depth(unsigned int op, struct blk_mq_alloc_data *data) Similarly as you did in patch 1, may be add a comment about this operation and when it is called ? > +{ > + struct deadline_data *dd = data->q->elevator->elevator_data; > + > + /* Do not throttle synchronous reads. */ > + if (op_is_sync(op) && !op_is_write(op)) > + return; > + > + /* > + * Throttle asynchronous requests and writes such that these requests > + * do not block the allocation of synchronous requests. > + */ > + data->shallow_depth = dd->async_depth; > +} > + > +static void dd_depth_updated(struct blk_mq_hw_ctx *hctx) > +{ > + struct request_queue *q = hctx->queue; > + struct deadline_data *dd = q->elevator->elevator_data; > + struct blk_mq_tags *tags = hctx->sched_tags; > + > + dd->async_depth = 3 * q->nr_requests / 4; I think that nr_requests is always at least 2, but it may be good to have a sanity check here that we do not end up with async_depth == 0, no ? > + > + sbitmap_queue_min_shallow_depth(tags->bitmap_tags, dd->async_depth); > +} > + > +static int dd_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int hctx_idx) > +{ > + dd_depth_updated(hctx); > + return 0; > +} > + > static void dd_exit_sched(struct elevator_queue *e) > { > struct deadline_data *dd = e->elevator_data; > @@ -744,6 +777,15 @@ static int deadline_starved_show(void *data, struct seq_file *m) > return 0; > } > > +static int dd_async_depth_show(void *data, struct seq_file *m) > +{ > + struct request_queue *q = data; > + struct deadline_data *dd = q->elevator->elevator_data; > + > + seq_printf(m, "%u\n", dd->async_depth); > + return 0; > +} > + > static void *deadline_dispatch_start(struct seq_file *m, loff_t *pos) > __acquires(&dd->lock) > { > @@ -786,6 +828,7 @@ static const struct blk_mq_debugfs_attr deadline_queue_debugfs_attrs[] = { > DEADLINE_QUEUE_DDIR_ATTRS(write), > {"batching", 0400, deadline_batching_show}, > {"starved", 0400, deadline_starved_show}, > + {"async_depth", 0400, dd_async_depth_show}, > {"dispatch", 0400, .seq_ops = &deadline_dispatch_seq_ops}, > {}, > }; > @@ -794,6 +837,8 @@ static const struct blk_mq_debugfs_attr deadline_queue_debugfs_attrs[] = { > > static struct elevator_type mq_deadline = { > .ops = { > + .depth_updated = dd_depth_updated, > + .limit_depth = dd_limit_depth, > .insert_requests = dd_insert_requests, > .dispatch_request = dd_dispatch_request, > .prepare_request = dd_prepare_request, > @@ -807,6 +852,7 @@ static struct elevator_type mq_deadline = { > .has_work = dd_has_work, > .init_sched = dd_init_sched, > .exit_sched = dd_exit_sched, > + .init_hctx = dd_init_hctx, > }, > > #ifdef CONFIG_BLK_DEBUG_FS >
On 5/27/21 3:01 AM, Bart Van Assche wrote: > For interactive workloads it is important that synchronous requests are > not delayed. Hence reserve 25% of tags for synchronous requests. This patch > still allows asynchronous requests to fill the hardware queues since > blk_mq_init_sched() makes sure that the number of scheduler requests is the > double of the hardware queue depth. From blk_mq_init_sched(): > > q->nr_requests = 2 * min_t(unsigned int, q->tag_set->queue_depth, > BLKDEV_MAX_RQ); > > Cc: Damien Le Moal <damien.lemoal@wdc.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/mq-deadline.c | 46 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > I wonder if that's a good idea in general ... I'm thinking of poor SATA drives which only have 31 tags; setting aside 8 of them for specific use-cases does make a difference one would think. Do you have testcases for this? Cheers, Hannes
On 5/26/21 11:54 PM, Hannes Reinecke wrote: > On 5/27/21 3:01 AM, Bart Van Assche wrote: >> For interactive workloads it is important that synchronous requests are >> not delayed. Hence reserve 25% of tags for synchronous requests. This patch >> still allows asynchronous requests to fill the hardware queues since >> blk_mq_init_sched() makes sure that the number of scheduler requests is the >> double of the hardware queue depth. From blk_mq_init_sched(): >> >> q->nr_requests = 2 * min_t(unsigned int, q->tag_set->queue_depth, >> BLKDEV_MAX_RQ); > > I wonder if that's a good idea in general ... I'm thinking of poor SATA > drives which only have 31 tags; setting aside 8 of them for specific > use-cases does make a difference one would think. > > Do you have testcases for this? Hi Hannes, The mq-deadline scheduler is the only scheduler that does not yet set aside tags for synchronous requests. BFQ and Kyber both implement the .limit_depth I/O scheduler callback function. Yes, I have a test case for this, namely SCSI UFS devices. The UFS device in my test setup supports a single I/O queue and limits the number of outstanding SCSI commands to 32 (UFSHCD_CAN_QUEUE). Please note that the limit mentioned above is still above the number of controller tags. For e.g. UFS q->tag_set->queue_depth == 32 and attaching an I/O scheduler increases nr_requests from 32 to 64. Reserving 25% of tags for synchronous requests leaves 3 * 64 / 4 = 48 scheduler tags for asynchronous requests. Thanks, Bart.
On 5/26/21 8:33 PM, Damien Le Moal wrote: > On 2021/05/27 10:02, Bart Van Assche wrote: >> For interactive workloads it is important that synchronous requests are >> not delayed. Hence reserve 25% of tags for synchronous requests. This patch > > s/tags/scheduler tags > > to be clear that we are not talking about the device tags. Same in the patch > title may be. OK. >> +static void dd_limit_depth(unsigned int op, struct blk_mq_alloc_data *data) > > Similarly as you did in patch 1, may be add a comment about this operation and > when it is called ? Will do. >> +static void dd_depth_updated(struct blk_mq_hw_ctx *hctx) >> +{ >> + struct request_queue *q = hctx->queue; >> + struct deadline_data *dd = q->elevator->elevator_data; >> + struct blk_mq_tags *tags = hctx->sched_tags; >> + >> + dd->async_depth = 3 * q->nr_requests / 4; > > I think that nr_requests is always at least 2, but it may be good to have a > sanity check here that we do not end up with async_depth == 0, no ? OK, I will add a check. Thanks, Bart.
diff --git a/block/mq-deadline.c b/block/mq-deadline.c index 2ab844a4b6b5..81f487d77e09 100644 --- a/block/mq-deadline.c +++ b/block/mq-deadline.c @@ -69,6 +69,7 @@ struct deadline_data { int fifo_batch; int writes_starved; int front_merges; + u32 async_depth; spinlock_t lock; spinlock_t zone_lock; @@ -399,6 +400,38 @@ static struct request *dd_dispatch_request(struct blk_mq_hw_ctx *hctx) return rq; } +static void dd_limit_depth(unsigned int op, struct blk_mq_alloc_data *data) +{ + struct deadline_data *dd = data->q->elevator->elevator_data; + + /* Do not throttle synchronous reads. */ + if (op_is_sync(op) && !op_is_write(op)) + return; + + /* + * Throttle asynchronous requests and writes such that these requests + * do not block the allocation of synchronous requests. + */ + data->shallow_depth = dd->async_depth; +} + +static void dd_depth_updated(struct blk_mq_hw_ctx *hctx) +{ + struct request_queue *q = hctx->queue; + struct deadline_data *dd = q->elevator->elevator_data; + struct blk_mq_tags *tags = hctx->sched_tags; + + dd->async_depth = 3 * q->nr_requests / 4; + + sbitmap_queue_min_shallow_depth(tags->bitmap_tags, dd->async_depth); +} + +static int dd_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int hctx_idx) +{ + dd_depth_updated(hctx); + return 0; +} + static void dd_exit_sched(struct elevator_queue *e) { struct deadline_data *dd = e->elevator_data; @@ -744,6 +777,15 @@ static int deadline_starved_show(void *data, struct seq_file *m) return 0; } +static int dd_async_depth_show(void *data, struct seq_file *m) +{ + struct request_queue *q = data; + struct deadline_data *dd = q->elevator->elevator_data; + + seq_printf(m, "%u\n", dd->async_depth); + return 0; +} + static void *deadline_dispatch_start(struct seq_file *m, loff_t *pos) __acquires(&dd->lock) { @@ -786,6 +828,7 @@ static const struct blk_mq_debugfs_attr deadline_queue_debugfs_attrs[] = { DEADLINE_QUEUE_DDIR_ATTRS(write), {"batching", 0400, deadline_batching_show}, {"starved", 0400, deadline_starved_show}, + {"async_depth", 0400, dd_async_depth_show}, {"dispatch", 0400, .seq_ops = &deadline_dispatch_seq_ops}, {}, }; @@ -794,6 +837,8 @@ static const struct blk_mq_debugfs_attr deadline_queue_debugfs_attrs[] = { static struct elevator_type mq_deadline = { .ops = { + .depth_updated = dd_depth_updated, + .limit_depth = dd_limit_depth, .insert_requests = dd_insert_requests, .dispatch_request = dd_dispatch_request, .prepare_request = dd_prepare_request, @@ -807,6 +852,7 @@ static struct elevator_type mq_deadline = { .has_work = dd_has_work, .init_sched = dd_init_sched, .exit_sched = dd_exit_sched, + .init_hctx = dd_init_hctx, }, #ifdef CONFIG_BLK_DEBUG_FS
For interactive workloads it is important that synchronous requests are not delayed. Hence reserve 25% of tags for synchronous requests. This patch still allows asynchronous requests to fill the hardware queues since blk_mq_init_sched() makes sure that the number of scheduler requests is the double of the hardware queue depth. From blk_mq_init_sched(): q->nr_requests = 2 * min_t(unsigned int, q->tag_set->queue_depth, BLKDEV_MAX_RQ); Cc: Damien Le Moal <damien.lemoal@wdc.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Ming Lei <ming.lei@redhat.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/mq-deadline.c | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)