| Message ID | dc018874d79575db91e3ec44c09511720e2b0ab0.1533846185.git.osandov@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kyber: better heuristics | expand |
Hi Omar On 08/10/2018 04:26 AM, Omar Sandoval wrote: > @@ -524,6 +524,9 @@ inline void __blk_mq_end_request(struct request *rq, blk_status_t error) > blk_stat_add(rq, now); > } > > + if (rq->internal_tag != -1) > + blk_mq_sched_completed_request(rq, now); > + Is it OK to move the io scheduler completed callback into __blk_mq_end_request ? There is a relatively long distance between the __blk_mq_complete_request and __blk_mq_end_request, especially the driver's mq_ops.complete and the bio_endio. Thanks Jianchao
On Fri, Aug 10, 2018 at 10:59:37AM +0800, jianchao.wang wrote: > Hi Omar > > On 08/10/2018 04:26 AM, Omar Sandoval wrote: > > @@ -524,6 +524,9 @@ inline void __blk_mq_end_request(struct request *rq, blk_status_t error) > > blk_stat_add(rq, now); > > } > > > > + if (rq->internal_tag != -1) > > + blk_mq_sched_completed_request(rq, now); > > + > > Is it OK to move the io scheduler completed callback into __blk_mq_end_request ? It's ok in the sense that only Kyber uses it. > There is a relatively long distance between the __blk_mq_complete_request and __blk_mq_end_request, > especially the driver's mq_ops.complete and the bio_endio. That's a fair point. We could maybe consolidate all of the I/O time measurements to __blk_mq_complete_request() and have a separate callback for the total time measurement in __blk_mq_end_request(), but I'm not sure it's worth adding another hook for that.
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h index 0cb8f938dff9..74fb6ff9a30d 100644 --- a/block/blk-mq-sched.h +++ b/block/blk-mq-sched.h @@ -54,12 +54,12 @@ blk_mq_sched_allow_merge(struct request_queue *q, struct request *rq, return true; } -static inline void blk_mq_sched_completed_request(struct request *rq) +static inline void blk_mq_sched_completed_request(struct request *rq, u64 now) { struct elevator_queue *e = rq->q->elevator; if (e && e->type->ops.mq.completed_request) - e->type->ops.mq.completed_request(rq); + e->type->ops.mq.completed_request(rq, now); } static inline void blk_mq_sched_started_request(struct request *rq) diff --git a/block/blk-mq.c b/block/blk-mq.c index 654b0dc7e001..0c5be0001d0f 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -524,6 +524,9 @@ inline void __blk_mq_end_request(struct request *rq, blk_status_t error) blk_stat_add(rq, now); } + if (rq->internal_tag != -1) + blk_mq_sched_completed_request(rq, now); + blk_account_io_done(rq, now); if (rq->end_io) { @@ -560,8 +563,6 @@ static void __blk_mq_complete_request(struct request *rq) if (!blk_mq_mark_complete(rq)) return; - if (rq->internal_tag != -1) - blk_mq_sched_completed_request(rq); if (!test_bit(QUEUE_FLAG_SAME_COMP, &rq->q->queue_flags)) { rq->q->softirq_done_fn(rq); diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c index a1660bafc912..95d062c07c61 100644 --- a/block/kyber-iosched.c +++ b/block/kyber-iosched.c @@ -558,12 +558,12 @@ static void kyber_finish_request(struct request *rq) rq_clear_domain_token(kqd, rq); } -static void kyber_completed_request(struct request *rq) +static void kyber_completed_request(struct request *rq, u64 now) { struct request_queue *q = rq->q; struct kyber_queue_data *kqd = q->elevator->elevator_data; unsigned int sched_domain; - u64 now, latency, target; + u64 latency, target; /* * Check if this request met our latency goal. If not, quickly gather @@ -585,7 +585,6 @@ static void kyber_completed_request(struct request *rq) if (blk_stat_is_active(kqd->cb)) return; - now = ktime_get_ns(); if (now < rq->io_start_time_ns) return; diff --git a/include/linux/elevator.h b/include/linux/elevator.h index a02deea30185..015bb59c0331 100644 --- a/include/linux/elevator.h +++ b/include/linux/elevator.h @@ -111,7 +111,7 @@ struct elevator_mq_ops { void (*insert_requests)(struct blk_mq_hw_ctx *, struct list_head *, bool); struct request *(*dispatch_request)(struct blk_mq_hw_ctx *); bool (*has_work)(struct blk_mq_hw_ctx *); - void (*completed_request)(struct request *); + void (*completed_request)(struct request *, u64); void (*started_request)(struct request *); void (*requeue_request)(struct request *); struct request *(*former_request)(struct request_queue *, struct request *);