Message ID | 20200327062859.GA12588@192.168.3.9 (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [RFC,v2,1/3] bio: update the real issue size when bio_split | expand |
On Fri, Mar 27, 2020 at 02:28:59PM +0800, Weiping Zhang wrote: > Change-Id: Ibb9caf20616f83e111113ab5c824c05930c0e523 > Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com> This needs a commit description and loose the weird change id. I also think oyu need to fins a way to not bloat the bio even more, cgroup is a really bad offender for bio size.
Christoph Hellwig <hch@infradead.org> 于2020年3月31日周二 下午4:25写道: > > On Fri, Mar 27, 2020 at 02:28:59PM +0800, Weiping Zhang wrote: > > Change-Id: Ibb9caf20616f83e111113ab5c824c05930c0e523 > > Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com> > > This needs a commit description and loose the weird change id. > OK, I rewirte commit description, it record the timestamp of issue bio to the disk driver, then we can get the delta time in rq_qos_done_bio. It's same as the D2C time of blktrace. > I also think oyu need to fins a way to not bloat the bio even more, > cgroup is a really bad offender for bio size. struct request { u64 io_start_time_ns; also record this timestamp, I'll check if we can use it. Thanks a lot
On Tue, Mar 31, 2020 at 04:45:33PM +0800, Weiping Zhang wrote: > Christoph Hellwig <hch@infradead.org> 于2020年3月31日周二 下午4:25写道: > > > > On Fri, Mar 27, 2020 at 02:28:59PM +0800, Weiping Zhang wrote: > > > Change-Id: Ibb9caf20616f83e111113ab5c824c05930c0e523 > > > Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com> > > > > This needs a commit description and loose the weird change id. > > > OK, I rewirte commit description, it record the timestamp of issue bio > to the disk driver, > then we can get the delta time in rq_qos_done_bio. It's same as the D2C time > of blktrace. > > I also think oyu need to fins a way to not bloat the bio even more, > > cgroup is a really bad offender for bio size. > struct request { > u64 io_start_time_ns; > also record this timestamp, I'll check if we can use it. But except for a few exceptions bios are never issued directly to the driver, requests are. And the few exception (rsxx, umem) probably should be rewritten to use requests. And with generic_{start,end}_io_acct we already have helpers to track bio based stats, which we should not duplicate just for cgroups.
Christoph Hellwig <hch@infradead.org> 于2020年3月31日周二 下午5:16写道: > > On Tue, Mar 31, 2020 at 04:45:33PM +0800, Weiping Zhang wrote: > > Christoph Hellwig <hch@infradead.org> 于2020年3月31日周二 下午4:25写道: > > > > > > On Fri, Mar 27, 2020 at 02:28:59PM +0800, Weiping Zhang wrote: > > > > Change-Id: Ibb9caf20616f83e111113ab5c824c05930c0e523 > > > > Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com> > > > > > > This needs a commit description and loose the weird change id. > > > > > OK, I rewirte commit description, it record the timestamp of issue bio > > to the disk driver, > > then we can get the delta time in rq_qos_done_bio. It's same as the D2C time > > of blktrace. > > > I also think oyu need to fins a way to not bloat the bio even more, > > > cgroup is a really bad offender for bio size. > > struct request { > > u64 io_start_time_ns; > > also record this timestamp, I'll check if we can use it. > > But except for a few exceptions bios are never issued directly to the > driver, requests are. And the few exception (rsxx, umem) probably should > be rewritten to use requests. And with generic_{start,end}_io_acct we > already have helpers to track bio based stats, which we should not > duplicate just for cgroups. generic_{start,end}_io_acct and blk_account_io_done, these two method use the a timeline (part->stamp), but cgroup doesn't have, so cgroup cann't use these general helper to counting the total io ticks for read,write and others. Block cgroup use delta = now - bio->bi_issue[issue_time] to counting total io ticks. How about move it into the blk-iotrack code, rq_qos_issue will call the rq_qos_ops.issue, then if user doesn't enable blk-iotrack, these code will not be executed. Thanks
diff --git a/block/blk-mq.c b/block/blk-mq.c index 5b2e6550e0b6..53db008ac8d0 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -652,6 +652,7 @@ EXPORT_SYMBOL(blk_mq_complete_request); void blk_mq_start_request(struct request *rq) { struct request_queue *q = rq->q; + struct bio *bio; trace_block_rq_issue(q, rq); @@ -660,6 +661,8 @@ void blk_mq_start_request(struct request *rq) rq->stats_sectors = blk_rq_sectors(rq); rq->rq_flags |= RQF_STATS; rq_qos_issue(q, rq); + __rq_for_each_bio(bio, rq) + blkcg_bio_start_init(bio); } WARN_ON_ONCE(blk_mq_rq_state(rq) != MQ_RQ_IDLE); diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h index e4a6949fd171..9720f04a9523 100644 --- a/include/linux/blk-cgroup.h +++ b/include/linux/blk-cgroup.h @@ -579,6 +579,11 @@ static inline void blkcg_bio_issue_init(struct bio *bio) bio_issue_init(&bio->bi_issue, bio_sectors(bio)); } +static inline void blkcg_bio_start_init(struct bio *bio) +{ + bio_start_init(&bio->bi_start); +} + static inline bool blkcg_bio_issue_check(struct request_queue *q, struct bio *bio) { @@ -738,6 +743,7 @@ static inline void blkg_get(struct blkcg_gq *blkg) { } static inline void blkg_put(struct blkcg_gq *blkg) { } static inline bool blkcg_punt_bio_submit(struct bio *bio) { return false; } ++static inline void blkcg_bio_start_init(struct bio *bio) { } static inline void blkcg_bio_issue_init(struct bio *bio) { } static inline bool blkcg_bio_issue_check(struct request_queue *q, struct bio *bio) { return true; } diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index fea81e3775c4..b19d7f44c6e7 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -109,10 +109,38 @@ static inline bool blk_path_error(blk_status_t error) /* Reserved bit for blk-throtl */ #define BIO_ISSUE_THROTL_SKIP_LATENCY (1ULL << 63) +/* submit bio to block layer */ struct bio_issue { u64 value; }; +/* + * submit bio to the disk driver layer + * + * 63:51 reserved + * 50:0 bits: start time of bio + * + * same bitmask as bi_issue + */ +struct bio_start { + u64 value; +}; + +static inline u64 __bio_start_time(u64 time) +{ + return time & BIO_ISSUE_TIME_MASK; +} + +static inline u64 bio_start_time(struct bio_start *start) +{ + return __bio_start_time(start->value); +} + +static inline void bio_start_init(struct bio_start *start) +{ + start->value = ktime_get_ns() & BIO_ISSUE_TIME_MASK; +} + static inline u64 __bio_issue_time(u64 time) { return time & BIO_ISSUE_TIME_MASK; @@ -178,6 +206,7 @@ struct bio { */ struct blkcg_gq *bi_blkg; struct bio_issue bi_issue; + struct bio_start bi_start; #ifdef CONFIG_BLK_CGROUP_IOCOST u64 bi_iocost_cost; #endif
Change-Id: Ibb9caf20616f83e111113ab5c824c05930c0e523 Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com> --- block/blk-mq.c | 3 +++ include/linux/blk-cgroup.h | 6 ++++++ include/linux/blk_types.h | 29 +++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+)