| Message ID | 20211203214544.343460-2-axboe@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for list issue | expand |
On 12/3/21 10:45 PM, Jens Axboe wrote: > If we have a list of requests in our plug list, send it to the driver in > one go, if possible. The driver must set mq_ops->queue_rqs() to support > this, if not the usual one-by-one path is used. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > --- > block/blk-mq.c | 24 +++++++++++++++++++++--- > include/linux/blk-mq.h | 8 ++++++++ > 2 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 22ec21aa0c22..9ac9174a2ba4 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -2513,6 +2513,7 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) > { > struct blk_mq_hw_ctx *this_hctx; > struct blk_mq_ctx *this_ctx; > + struct request *rq; > unsigned int depth; > LIST_HEAD(list); > > @@ -2521,7 +2522,26 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) > plug->rq_count = 0; > > if (!plug->multiple_queues && !plug->has_elevator && !from_schedule) { > - blk_mq_run_dispatch_ops(plug->mq_list->q, > + struct request_queue *q; > + > + rq = plug->mq_list; > + q = rq->q; > + > + /* > + * Peek first request and see if we have a ->queue_rqs() hook. > + * If we do, we can dispatch the whole plug list in one go. We > + * already know at this point that all requests belong to the > + * same queue, caller must ensure that's the case. > + */ > + if (q->mq_ops->queue_rqs && > + !(rq->mq_hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { What is the dependency on shared tags here? From what I've seen it's just about submitting requests; the only difference to shared tags is the way the tags are allocated. Care to explain? Cheers, Hannes
On 12/4/21 3:43 AM, Hannes Reinecke wrote: > On 12/3/21 10:45 PM, Jens Axboe wrote: >> If we have a list of requests in our plug list, send it to the driver in >> one go, if possible. The driver must set mq_ops->queue_rqs() to support >> this, if not the usual one-by-one path is used. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> --- >> block/blk-mq.c | 24 +++++++++++++++++++++--- >> include/linux/blk-mq.h | 8 ++++++++ >> 2 files changed, 29 insertions(+), 3 deletions(-) >> >> diff --git a/block/blk-mq.c b/block/blk-mq.c >> index 22ec21aa0c22..9ac9174a2ba4 100644 >> --- a/block/blk-mq.c >> +++ b/block/blk-mq.c >> @@ -2513,6 +2513,7 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) >> { >> struct blk_mq_hw_ctx *this_hctx; >> struct blk_mq_ctx *this_ctx; >> + struct request *rq; >> unsigned int depth; >> LIST_HEAD(list); >> >> @@ -2521,7 +2522,26 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) >> plug->rq_count = 0; >> >> if (!plug->multiple_queues && !plug->has_elevator && !from_schedule) { >> - blk_mq_run_dispatch_ops(plug->mq_list->q, >> + struct request_queue *q; >> + >> + rq = plug->mq_list; >> + q = rq->q; >> + >> + /* >> + * Peek first request and see if we have a ->queue_rqs() hook. >> + * If we do, we can dispatch the whole plug list in one go. We >> + * already know at this point that all requests belong to the >> + * same queue, caller must ensure that's the case. >> + */ >> + if (q->mq_ops->queue_rqs && >> + !(rq->mq_hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { > > What is the dependency on shared tags here? > From what I've seen it's just about submitting requests; the only > difference to shared tags is the way the tags are allocated. > Care to explain? For shared tags, we need to actively increment the use count per request. This path doesn't do that, so it's disabled for now. It could be done, but then it'd have to be in the caller, so I'd rather leave it for a future optimization if anyone cares enough about this for shared tags. I can add a comment about it if that helps.
On 12/4/21 9:13 PM, Jens Axboe wrote: > On 12/4/21 3:43 AM, Hannes Reinecke wrote: >> On 12/3/21 10:45 PM, Jens Axboe wrote: >>> If we have a list of requests in our plug list, send it to the driver in >>> one go, if possible. The driver must set mq_ops->queue_rqs() to support >>> this, if not the usual one-by-one path is used. >>> >>> Signed-off-by: Jens Axboe <axboe@kernel.dk> >>> --- >>> block/blk-mq.c | 24 +++++++++++++++++++++--- >>> include/linux/blk-mq.h | 8 ++++++++ >>> 2 files changed, 29 insertions(+), 3 deletions(-) >>> >>> diff --git a/block/blk-mq.c b/block/blk-mq.c >>> index 22ec21aa0c22..9ac9174a2ba4 100644 >>> --- a/block/blk-mq.c >>> +++ b/block/blk-mq.c >>> @@ -2513,6 +2513,7 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) >>> { >>> struct blk_mq_hw_ctx *this_hctx; >>> struct blk_mq_ctx *this_ctx; >>> + struct request *rq; >>> unsigned int depth; >>> LIST_HEAD(list); >>> >>> @@ -2521,7 +2522,26 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) >>> plug->rq_count = 0; >>> >>> if (!plug->multiple_queues && !plug->has_elevator && !from_schedule) { >>> - blk_mq_run_dispatch_ops(plug->mq_list->q, >>> + struct request_queue *q; >>> + >>> + rq = plug->mq_list; >>> + q = rq->q; >>> + >>> + /* >>> + * Peek first request and see if we have a ->queue_rqs() hook. >>> + * If we do, we can dispatch the whole plug list in one go. We >>> + * already know at this point that all requests belong to the >>> + * same queue, caller must ensure that's the case. >>> + */ >>> + if (q->mq_ops->queue_rqs && >>> + !(rq->mq_hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { >> >> What is the dependency on shared tags here? >> From what I've seen it's just about submitting requests; the only >> difference to shared tags is the way the tags are allocated. >> Care to explain? > > For shared tags, we need to actively increment the use count per > request. This path doesn't do that, so it's disabled for now. It could > be done, but then it'd have to be in the caller, so I'd rather leave it > for a future optimization if anyone cares enough about this for shared > tags. > > I can add a comment about it if that helps. > Please do. It'll act as a reminder what needs to be done if and when one of the drivers requiring shared tags is looking at implementing queue_rqs. Cheers, Hannes
On 12/5/21 2:07 AM, Hannes Reinecke wrote: > On 12/4/21 9:13 PM, Jens Axboe wrote: >> On 12/4/21 3:43 AM, Hannes Reinecke wrote: >>> On 12/3/21 10:45 PM, Jens Axboe wrote: >>>> If we have a list of requests in our plug list, send it to the driver in >>>> one go, if possible. The driver must set mq_ops->queue_rqs() to support >>>> this, if not the usual one-by-one path is used. >>>> >>>> Signed-off-by: Jens Axboe <axboe@kernel.dk> >>>> --- >>>> block/blk-mq.c | 24 +++++++++++++++++++++--- >>>> include/linux/blk-mq.h | 8 ++++++++ >>>> 2 files changed, 29 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/block/blk-mq.c b/block/blk-mq.c >>>> index 22ec21aa0c22..9ac9174a2ba4 100644 >>>> --- a/block/blk-mq.c >>>> +++ b/block/blk-mq.c >>>> @@ -2513,6 +2513,7 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) >>>> { >>>> struct blk_mq_hw_ctx *this_hctx; >>>> struct blk_mq_ctx *this_ctx; >>>> + struct request *rq; >>>> unsigned int depth; >>>> LIST_HEAD(list); >>>> >>>> @@ -2521,7 +2522,26 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) >>>> plug->rq_count = 0; >>>> >>>> if (!plug->multiple_queues && !plug->has_elevator && !from_schedule) { >>>> - blk_mq_run_dispatch_ops(plug->mq_list->q, >>>> + struct request_queue *q; >>>> + >>>> + rq = plug->mq_list; >>>> + q = rq->q; >>>> + >>>> + /* >>>> + * Peek first request and see if we have a ->queue_rqs() hook. >>>> + * If we do, we can dispatch the whole plug list in one go. We >>>> + * already know at this point that all requests belong to the >>>> + * same queue, caller must ensure that's the case. >>>> + */ >>>> + if (q->mq_ops->queue_rqs && >>>> + !(rq->mq_hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { >>> >>> What is the dependency on shared tags here? >>> From what I've seen it's just about submitting requests; the only >>> difference to shared tags is the way the tags are allocated. >>> Care to explain? >> >> For shared tags, we need to actively increment the use count per >> request. This path doesn't do that, so it's disabled for now. It could >> be done, but then it'd have to be in the caller, so I'd rather leave it >> for a future optimization if anyone cares enough about this for shared >> tags. >> >> I can add a comment about it if that helps. >> > Please do. > It'll act as a reminder what needs to be done if and when one of the > drivers requiring shared tags is looking at implementing queue_rqs. I added to the comment yesterday: https://git.kernel.dk/cgit/linux-block/commit/?h=perf-wip&id=f9f526700607bf804fa8541c824ea54253f4241a
diff --git a/block/blk-mq.c b/block/blk-mq.c index 22ec21aa0c22..9ac9174a2ba4 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2513,6 +2513,7 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) { struct blk_mq_hw_ctx *this_hctx; struct blk_mq_ctx *this_ctx; + struct request *rq; unsigned int depth; LIST_HEAD(list); @@ -2521,7 +2522,26 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) plug->rq_count = 0; if (!plug->multiple_queues && !plug->has_elevator && !from_schedule) { - blk_mq_run_dispatch_ops(plug->mq_list->q, + struct request_queue *q; + + rq = plug->mq_list; + q = rq->q; + + /* + * Peek first request and see if we have a ->queue_rqs() hook. + * If we do, we can dispatch the whole plug list in one go. We + * already know at this point that all requests belong to the + * same queue, caller must ensure that's the case. + */ + if (q->mq_ops->queue_rqs && + !(rq->mq_hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { + blk_mq_run_dispatch_ops(q, + q->mq_ops->queue_rqs(&plug->mq_list)); + if (rq_list_empty(plug->mq_list)) + return; + } + + blk_mq_run_dispatch_ops(q, blk_mq_plug_issue_direct(plug, false)); if (rq_list_empty(plug->mq_list)) return; @@ -2531,8 +2551,6 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) this_ctx = NULL; depth = 0; do { - struct request *rq; - rq = rq_list_pop(&plug->mq_list); if (!this_hctx) { diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index ecdc049b52fa..cdd183757ea0 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -494,6 +494,14 @@ struct blk_mq_ops { */ void (*commit_rqs)(struct blk_mq_hw_ctx *); + /** + * @queue_rqs: Queue a list of new requests. Driver is guaranteed + * that each request belongs to the same queue. If the driver doesn't + * empty the @rqlist completely, then the rest will be queued + * individually by the block layer upon return. + */ + void (*queue_rqs)(struct request **rqlist); + /** * @get_budget: Reserve budget before queue request, once .queue_rq is * run, it is driver's responsibility to release the
If we have a list of requests in our plug list, send it to the driver in one go, if possible. The driver must set mq_ops->queue_rqs() to support this, if not the usual one-by-one path is used. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- block/blk-mq.c | 24 +++++++++++++++++++++--- include/linux/blk-mq.h | 8 ++++++++ 2 files changed, 29 insertions(+), 3 deletions(-)