[RFC PATCH v2 2/3] bio: track timestamp of submitting bio the disk driver

[RFC PATCH v2 2/3] bio: track timestamp of submitting bio the disk driver

[Weiping Zhang]

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(+)

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
-- 
2.18.1

Re: [RFC PATCH v2 2/3] bio: track timestamp of submitting bio the disk driver

[Christoph Hellwig]

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.

Re: [RFC PATCH v2 2/3] bio: track timestamp of submitting bio the disk driver

[Weiping Zhang]

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

Re: [RFC PATCH v2 2/3] bio: track timestamp of submitting bio the disk driver

[Christoph Hellwig]

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.

Re: [RFC PATCH v2 2/3] bio: track timestamp of submitting bio the disk driver

[Weiping Zhang]

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