| Message ID | 51a1cb7b-6136-a220-6cac-9bccd2262ca3@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk-mq: don't call ktime_get_ns() if we don't need it | expand |
On Fri, Nov 30, 2018 at 08:56:25AM -0700, Jens Axboe wrote: > We only need the request fields and the end_io time if we have stats > enabled, or if we have a scheduler attached as those may use it for > completion time stats. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > --- > block/blk-mq.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 7dcef565dc0f..2b458e9da946 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -316,7 +316,8 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data, > RB_CLEAR_NODE(&rq->rb_node); > rq->rq_disk = NULL; > rq->part = NULL; > - rq->start_time_ns = ktime_get_ns(); > + if (data->q->elevator) > + rq->start_time_ns = ktime_get_ns(); We also seem to use start_time_ns it in blk_account_io_done. Also update it in attempt_merge, which might have some odd results for completly uninitialized fields.
On 11/30/18 10:27 AM, Christoph Hellwig wrote: > On Fri, Nov 30, 2018 at 08:56:25AM -0700, Jens Axboe wrote: >> We only need the request fields and the end_io time if we have stats >> enabled, or if we have a scheduler attached as those may use it for >> completion time stats. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> --- >> block/blk-mq.c | 13 ++++++++++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/block/blk-mq.c b/block/blk-mq.c >> index 7dcef565dc0f..2b458e9da946 100644 >> --- a/block/blk-mq.c >> +++ b/block/blk-mq.c >> @@ -316,7 +316,8 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data, >> RB_CLEAR_NODE(&rq->rb_node); >> rq->rq_disk = NULL; >> rq->part = NULL; >> - rq->start_time_ns = ktime_get_ns(); >> + if (data->q->elevator) >> + rq->start_time_ns = ktime_get_ns(); > > We also seem to use start_time_ns it in blk_account_io_done. But only if RQF_STATS is set, in which case we are passing it in correctly. > Also update it in attempt_merge, which might have some odd results > for completly uninitialized fields. I did notice these, but it's pointless to change those, as you'd then have to check validity. Nobody cares at that point...
On 11/30/18 10:29 AM, Jens Axboe wrote: > On 11/30/18 10:27 AM, Christoph Hellwig wrote: >> On Fri, Nov 30, 2018 at 08:56:25AM -0700, Jens Axboe wrote: >>> We only need the request fields and the end_io time if we have stats >>> enabled, or if we have a scheduler attached as those may use it for >>> completion time stats. >>> >>> Signed-off-by: Jens Axboe <axboe@kernel.dk> >>> --- >>> block/blk-mq.c | 13 ++++++++++--- >>> 1 file changed, 10 insertions(+), 3 deletions(-) >>> >>> diff --git a/block/blk-mq.c b/block/blk-mq.c >>> index 7dcef565dc0f..2b458e9da946 100644 >>> --- a/block/blk-mq.c >>> +++ b/block/blk-mq.c >>> @@ -316,7 +316,8 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data, >>> RB_CLEAR_NODE(&rq->rb_node); >>> rq->rq_disk = NULL; >>> rq->part = NULL; >>> - rq->start_time_ns = ktime_get_ns(); >>> + if (data->q->elevator) >>> + rq->start_time_ns = ktime_get_ns(); >> >> We also seem to use start_time_ns it in blk_account_io_done. > > But only if RQF_STATS is set, in which case we are passing it in correctly. Oh, needs a stats check for this one, for some reason I was thinking of the end_io path. Will update.
diff --git a/block/blk-mq.c b/block/blk-mq.c index 7dcef565dc0f..2b458e9da946 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -316,7 +316,8 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data, RB_CLEAR_NODE(&rq->rb_node); rq->rq_disk = NULL; rq->part = NULL; - rq->start_time_ns = ktime_get_ns(); + if (data->q->elevator) + rq->start_time_ns = ktime_get_ns(); rq->io_start_time_ns = 0; rq->nr_phys_segments = 0; #if defined(CONFIG_BLK_DEV_INTEGRITY) @@ -522,12 +523,18 @@ EXPORT_SYMBOL_GPL(blk_mq_free_request); inline void __blk_mq_end_request(struct request *rq, blk_status_t error) { - u64 now = ktime_get_ns(); + u64 now = 0; + /* + * Only read ktime if we need it, which is if stats are enabled + * or we have a scheduler attached that may use it. + */ if (rq->rq_flags & RQF_STATS) { blk_mq_poll_stats_start(rq->q); + now = ktime_get_ns(); blk_stat_add(rq, now); - } + } else if (rq->q->elevator) + now = ktime_get_ns(); if (rq->internal_tag != -1) blk_mq_sched_completed_request(rq, now);
We only need the request fields and the end_io time if we have stats enabled, or if we have a scheduler attached as those may use it for completion time stats. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- block/blk-mq.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)