Skip to contentSkip to conversation

block: callback-based statistics

PATCH In review Omar Sandoval 2 participants 6 messages Mar 21, 2017 Block layer

lore
Share & tools
lore
#0 lore raw

v2 cover block: callback-based statistics

From: Omar Sandoval <[email protected]>

This patchset generalizes the blk-stats infrastructure to allow users to register a callback to be called at a given time with the statistics of requests completed during that window. Writeback throttling and hybrid polling are converted to the new infrastructure. The new Kyber I/O scheduler uses this, as well (but it needs to be rebased on this v2).

The details are in patch 4, which is the actual conversion. Patches 1-3 are preparation cleanups.

Changes since v1:

- Now the user can subdivide stats into arbitrary buckets. Both in-tree users just do reads vs. writes, but we can extend poll based on request size in the future
- blk_stat_arm_callback() became blk_stat_activate_msecs() and blk_stat_activate_nsecs()
- The poll statistics are exposed in debugfs

Omar Sandoval (4):
block: remove extra calls to wbt_exit()
blk-stat: use READ and WRITE instead of BLK_STAT_{READ,WRITE}
blk-stat: move BLK_RQ_STAT_BATCH definition to blk-stat.c
blk-stat: convert to callback-based statistics reporting

 block/blk-core.c          |   7 +-
 block/blk-mq-debugfs.c    |  99 +++++++--------
 block/blk-mq.c            |  78 ++++++++----
 block/blk-mq.h            |   1 -
 block/blk-stat.c          | 315 ++++++++++++++++++++++------------------------
 block/blk-stat.h          | 182 ++++++++++++++++++++++++---
 block/blk-sysfs.c         |  31 +----
 block/blk-wbt.c           |  61 ++++-----
 block/blk-wbt.h           |   2 +-
 include/linux/blk_types.h |   3 -
 include/linux/blkdev.h    |  10 +-
 11 files changed, 454 insertions(+), 335 deletions(-)
Signature · 1 line
2.12.0
#1 lore raw

PATCH v2 1/4 block: remove extra calls to wbt_exit()

From: Omar Sandoval <[email protected]>

We always call wbt_exit() from blk_release_queue(), so these are unnecessary.

---
 block/blk-core.c | 1 -
 block/blk-mq.c   | 2 --
 2 files changed, 3 deletions(-)
block/blk-core.c +0 -1
index d772c221cc17..e8a9bc0d4bbb 100644
@@ -889,7 +889,6 @@ int blk_init_allocated_queue(struct request_queue *q)
 		q->exit_rq_fn(q, q->fq->flush_rq);
 out_free_flush_queue:
 	blk_free_flush_queue(q->fq);
-	wbt_exit(q);
 	return -ENOMEM;
 }
 EXPORT_SYMBOL(blk_init_allocated_queue);
block/blk-mq.c +0 -2
index a4546f060e80..534f49a90e3a 100644
@@ -2431,8 +2431,6 @@ void blk_mq_free_queue(struct request_queue *q)
 	list_del_init(&q->all_q_node);
 	mutex_unlock(&all_q_mutex);
 
-	wbt_exit(q);
-
 	blk_mq_del_queue_tag_set(q);
 
 	blk_mq_exit_hw_queues(q, set, set->nr_hw_queues);
Signature · 1 line
2.12.0
#2 lore raw

PATCH v2 2/4 blk-stat: use READ and WRITE instead of BLK_STAT_{READ,WRITE}

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

The stats buckets will become generic soon, so make the existing users use the common READ and WRITE definitions instead of one internal to blk-stat.

---
 block/blk-mq-debugfs.c | 12 ++++----
 block/blk-mq.c         | 12 ++++----
 block/blk-stat.c       | 80 ++++++++++++++++++++++++--------------------------
 block/blk-stat.h       |  5 ----
 block/blk-sysfs.c      |  4 +--
 block/blk-wbt.c        | 12 ++++----
 6 files changed, 59 insertions(+), 66 deletions(-)
block/blk-mq-debugfs.c +6 -6
index f6d917977b33..48c88723944a 100644
@@ -333,17 +333,17 @@ static int hctx_stats_show(struct seq_file *m, void *v)
 	struct blk_mq_hw_ctx *hctx = m->private;
 	struct blk_rq_stat stat[2];
 
-	blk_stat_init(&stat[BLK_STAT_READ]);
-	blk_stat_init(&stat[BLK_STAT_WRITE]);
+	blk_stat_init(&stat[READ]);
+	blk_stat_init(&stat[WRITE]);
 
 	blk_hctx_stat_get(hctx, stat);
 
 	seq_puts(m, "read: ");
-	print_stat(m, &stat[BLK_STAT_READ]);
+	print_stat(m, &stat[READ]);
 	seq_puts(m, "\n");
 
 	seq_puts(m, "write: ");
-	print_stat(m, &stat[BLK_STAT_WRITE]);
+	print_stat(m, &stat[WRITE]);
 	seq_puts(m, "\n");
 	return 0;
 }
@@ -362,8 +362,8 @@ static ssize_t hctx_stats_write(struct file *file, const char __user *buf,
 	int i;
 
 	hctx_for_each_ctx(hctx, ctx, i) {
-		blk_stat_init(&ctx->stat[BLK_STAT_READ]);
-		blk_stat_init(&ctx->stat[BLK_STAT_WRITE]);
+		blk_stat_init(&ctx->stat[READ]);
+		blk_stat_init(&ctx->stat[WRITE]);
 	}
 	return count;
 }
block/blk-mq.c +6 -6
index 534f49a90e3a..559e5363bb2c 100644
@@ -2040,8 +2040,8 @@ static void blk_mq_init_cpu_queues(struct request_queue *q,
 		spin_lock_init(&__ctx->lock);
 		INIT_LIST_HEAD(&__ctx->rq_list);
 		__ctx->queue = q;
-		blk_stat_init(&__ctx->stat[BLK_STAT_READ]);
-		blk_stat_init(&__ctx->stat[BLK_STAT_WRITE]);
+		blk_stat_init(&__ctx->stat[READ]);
+		blk_stat_init(&__ctx->stat[WRITE]);
 
 		/* If the cpu isn't online, the cpu is mapped to first hctx */
 		if (!cpu_online(i))
@@ -2769,10 +2769,10 @@ static unsigned long blk_mq_poll_nsecs(struct request_queue *q,
 	 * important on devices where the completion latencies are longer
 	 * than ~10 usec.
 	 */
-	if (req_op(rq) == REQ_OP_READ && stat[BLK_STAT_READ].nr_samples)
-		ret = (stat[BLK_STAT_READ].mean + 1) / 2;
-	else if (req_op(rq) == REQ_OP_WRITE && stat[BLK_STAT_WRITE].nr_samples)
-		ret = (stat[BLK_STAT_WRITE].mean + 1) / 2;
+	if (req_op(rq) == REQ_OP_READ && stat[READ].nr_samples)
+		ret = (stat[READ].mean + 1) / 2;
+	else if (req_op(rq) == REQ_OP_WRITE && stat[WRITE].nr_samples)
+		ret = (stat[WRITE].mean + 1) / 2;
 
 	return ret;
 }
block/blk-stat.c +39 -41

140 lines not on this page · raw message

block/blk-stat.h +0 -5
index a2050a0a5314..34384328b46b 100644
@@ -15,11 +15,6 @@
 #define BLK_STAT_TIME_MASK	((1ULL << BLK_STAT_SHIFT) - 1)
 #define BLK_STAT_MASK		~BLK_STAT_TIME_MASK
 
-enum {
-	BLK_STAT_READ	= 0,
-	BLK_STAT_WRITE,
-};
-
 void blk_stat_add(struct blk_rq_stat *, struct request *);
 void blk_hctx_stat_get(struct blk_mq_hw_ctx *, struct blk_rq_stat *);
 void blk_queue_stat_get(struct request_queue *, struct blk_rq_stat *);
block/blk-sysfs.c +2 -2
index c44b321335f3..fdb45fd0db0b 100644
@@ -518,8 +518,8 @@ static ssize_t queue_stats_show(struct request_queue *q, char *page)
 
 	blk_queue_stat_get(q, stat);
 
-	ret = print_stat(page, &stat[BLK_STAT_READ], "read :");
-	ret += print_stat(page + ret, &stat[BLK_STAT_WRITE], "write:");
+	ret = print_stat(page, &stat[READ], "read :");
+	ret += print_stat(page + ret, &stat[WRITE], "write:");
 	return ret;
 }
 
block/blk-wbt.c +6 -6
index 1aedb1f7ee0c..aafe5b551224 100644
@@ -255,8 +255,8 @@ static inline bool stat_sample_valid(struct blk_rq_stat *stat)
 	 * that it's writes impacting us, and not just some sole read on
 	 * a device that is in a lower power state.
 	 */
-	return stat[BLK_STAT_READ].nr_samples >= 1 &&
-		stat[BLK_STAT_WRITE].nr_samples >= RWB_MIN_WRITE_SAMPLES;
+	return (stat[READ].nr_samples >= 1 &&
+		stat[WRITE].nr_samples >= RWB_MIN_WRITE_SAMPLES);
 }
 
 static u64 rwb_sync_issue_lat(struct rq_wb *rwb)
@@ -293,7 +293,7 @@ static int __latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
 	 */
 	thislat = rwb_sync_issue_lat(rwb);
 	if (thislat > rwb->cur_win_nsec ||
-	    (thislat > rwb->min_lat_nsec && !stat[BLK_STAT_READ].nr_samples)) {
+	    (thislat > rwb->min_lat_nsec && !stat[READ].nr_samples)) {
 		trace_wbt_lat(bdi, thislat);
 		return LAT_EXCEEDED;
 	}
@@ -308,7 +308,7 @@ static int __latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
 		 * waited or still has writes in flights, consider us doing
 		 * just writes as well.
 		 */
-		if ((stat[BLK_STAT_WRITE].nr_samples && blk_stat_is_current(stat)) ||
+		if ((stat[WRITE].nr_samples && blk_stat_is_current(stat)) ||
 		    wb_recent_wait(rwb) || wbt_inflight(rwb))
 			return LAT_UNKNOWN_WRITES;
 		return LAT_UNKNOWN;
@@ -317,8 +317,8 @@ static int __latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
 	/*
 	 * If the 'min' latency exceeds our target, step down.
 	 */
-	if (stat[BLK_STAT_READ].min > rwb->min_lat_nsec) {
-		trace_wbt_lat(bdi, stat[BLK_STAT_READ].min);
+	if (stat[READ].min > rwb->min_lat_nsec) {
+		trace_wbt_lat(bdi, stat[READ].min);
 		trace_wbt_stat(bdi, stat);
 		return LAT_EXCEEDED;
 	}
Signature · 1 line
2.12.0
#3 lore raw

PATCH v2 3/4 blk-stat: move BLK_RQ_STAT_BATCH definition to blk-stat.c

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

This is an implementation detail that no-one outside of blk-stat.c uses.

---
 block/blk-stat.c          | 2 ++
 include/linux/blk_types.h | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
block/blk-stat.c +2 -0
index f80582be5344..4681c488c262 100644
@@ -9,6 +9,8 @@
 #include "blk-stat.h"
 #include "blk-mq.h"
 
+#define BLK_RQ_STAT_BATCH	64
+
 static void blk_stat_flush_batch(struct blk_rq_stat *stat)
 {
 	const s32 nr_batch = READ_ONCE(stat->nr_batch);
include/linux/blk_types.h +0 -2
index d703acb55d0f..e213c5e7500b 100644
@@ -287,8 +287,6 @@ struct blk_issue_stat {
 	u64 time;
 };
 
-#define BLK_RQ_STAT_BATCH	64
-
 struct blk_rq_stat {
 	s64 mean;
 	u64 min;
Signature · 1 line
2.12.0
#4 lore raw

PATCH v2 4/4 blk-stat: convert to callback-based statistics reporting

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Currently, statistics are gathered in ~0.13s windows, and users grab the statistics whenever they need them. This is not ideal for both in-tree users:

1. Writeback throttling wants its own dynamically sized window of statistics. Since the blk-stats statistics are reset after every window and the wbt windows don't line up with the blk-stats windows, wbt doesn't see every I/O.
2. Polling currently grabs the statistics on every I/O. Again, depending on how the window lines up, we may miss some I/Os. It's also unnecessary overhead to get the statistics on every I/O; the hybrid polling heuristic would be just as happy with the statistics from the previous full window.

This reworks the blk-stats infrastructure to be callback-based: users register a callback that they want called at a given time with all of the statistics from the window during which the callback was active. Users can dynamically bucketize the statistics. wbt and polling both currently use read vs. write, but polling can be extended to further subdivide based on request size.

The callbacks are kept on an RCU list, and each callback has percpu stats buffers. There will only be a few users, so the overhead on the I/O completion side is low. The stats flushing is also simplified
considerably: since the timer function is responsible for clearing the statistics, we don't have to worry about stale statistics.

wbt is a trivial conversion. After the conversion, the windowing problem mentioned above is fixed.

For polling, we register an extra callback that caches the previous window's statistics in the struct request_queue for the hybrid polling heuristic to use.

Since we no longer have a single stats buffer for the request queue, this also removes the sysfs and debugfs stats entries. To replace those, we add a debugfs entry for the poll statistics.

---
 block/blk-core.c          |   6 +-
 block/blk-mq-debugfs.c    |  99 +++++++--------
 block/blk-mq.c            |  76 +++++++----
 block/blk-mq.h            |   1 -
 block/blk-stat.c          | 311 ++++++++++++++++++++++------------------------
 block/blk-stat.h          | 181 +++++++++++++++++++++++++--
 block/blk-sysfs.c         |  31 +----
 block/blk-wbt.c           |  51 +++-----
 block/blk-wbt.h           |   2 +-
 include/linux/blk_types.h |   1 -
 include/linux/blkdev.h    |  10 +-
 11 files changed, 449 insertions(+), 320 deletions(-)
block/blk-core.c +5 -1
index e8a9bc0d4bbb..78d04ddededc 100644
@@ -852,6 +852,10 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio);
 
 int blk_init_allocated_queue(struct request_queue *q)
 {
+	q->stats = blk_alloc_queue_stats();
+	if (!q->stats)
+		return -ENOMEM;
+
 	q->fq = blk_alloc_flush_queue(q, NUMA_NO_NODE, q->cmd_size);
 	if (!q->fq)
 		return -ENOMEM;
@@ -2698,7 +2702,7 @@ void blk_finish_request(struct request *req, int error)
 	struct request_queue *q = req->q;
 
 	if (req->rq_flags & RQF_STATS)
-		blk_stat_add(&q->rq_stats[rq_data_dir(req)], req);
+		blk_stat_add(req);
 
 	if (req->rq_flags & RQF_QUEUED)
 		blk_queue_end_tag(q, req);
block/blk-mq-debugfs.c +44 -55

129 lines not on this page · raw message

block/blk-mq.c +52 -24

125 lines not on this page · raw message

block/blk-mq.h +0 -1
index b79f9a7d8cf6..8d49c06fc520 100644
@@ -20,7 +20,6 @@ struct blk_mq_ctx {
 
 	/* incremented at completion time */
 	unsigned long		____cacheline_aligned_in_smp rq_completed[2];
-	struct blk_rq_stat	stat[2];
 
 	struct request_queue	*queue;
 	struct kobject		kobj;
block/blk-stat.c +147 -164

366 lines not on this page · raw message

block/blk-stat.h +168 -13

197 lines not on this page · raw message

block/blk-sysfs.c +5 -26
index fdb45fd0db0b..fa831cb2fc30 100644
@@ -503,26 +503,6 @@ static ssize_t queue_dax_show(struct request_queue *q, char *page)
 	return queue_var_show(blk_queue_dax(q), page);
 }
 
-static ssize_t print_stat(char *page, struct blk_rq_stat *stat, const char *pre)
-{
-	return sprintf(page, "%s samples=%llu, mean=%lld, min=%lld, max=%lld\n",
-			pre, (long long) stat->nr_samples,
-			(long long) stat->mean, (long long) stat->min,
-			(long long) stat->max);
-}
-
-static ssize_t queue_stats_show(struct request_queue *q, char *page)
-{
-	struct blk_rq_stat stat[2];
-	ssize_t ret;
-
-	blk_queue_stat_get(q, stat);
-
-	ret = print_stat(page, &stat[READ], "read :");
-	ret += print_stat(page + ret, &stat[WRITE], "write:");
-	return ret;
-}
-
 static struct queue_sysfs_entry queue_requests_entry = {
 	.attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
 	.show = queue_requests_show,
@@ -691,11 +671,6 @@ static struct queue_sysfs_entry queue_dax_entry = {
 	.show = queue_dax_show,
 };
 
-static struct queue_sysfs_entry queue_stats_entry = {
-	.attr = {.name = "stats", .mode = S_IRUGO },
-	.show = queue_stats_show,
-};
-
 static struct queue_sysfs_entry queue_wb_lat_entry = {
 	.attr = {.name = "wbt_lat_usec", .mode = S_IRUGO | S_IWUSR },
 	.show = queue_wb_lat_show,
@@ -733,7 +708,6 @@ static struct attribute *default_attrs[] = {
 	&queue_poll_entry.attr,
 	&queue_wc_entry.attr,
 	&queue_dax_entry.attr,
-	&queue_stats_entry.attr,
 	&queue_wb_lat_entry.attr,
 	&queue_poll_delay_entry.attr,
 	NULL,
@@ -811,6 +785,9 @@ static void blk_release_queue(struct kobject *kobj)
 		container_of(kobj, struct request_queue, kobj);
 
 	wbt_exit(q);
+	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
+		blk_stat_remove_callback(q, q->poll_cb);
+	blk_stat_free_callback(q->poll_cb);
 	bdi_put(q->backing_dev_info);
 	blkcg_exit_queue(q);
 
@@ -819,6 +796,8 @@ static void blk_release_queue(struct kobject *kobj)
 		elevator_exit(q->elevator);
 	}
 
+	blk_free_queue_stats(q->stats);
+
 	blk_exit_rl(&q->root_rl);
 
 	if (q->queue_tags)
block/blk-wbt.c +19 -32

142 lines not on this page · raw message

block/blk-wbt.h +1 -1
index 65f1de519f67..591ff2f4b2ee 100644
@@ -81,7 +81,7 @@ struct rq_wb {
 	u64 win_nsec;				/* default window size */
 	u64 cur_win_nsec;			/* current window size */
 
-	struct timer_list window_timer;
+	struct blk_stat_callback *cb;
 
 	s64 sync_issue;
 	void *sync_cookie;
include/linux/blk_types.h +0 -1
index e213c5e7500b..270119a501fb 100644
@@ -294,7 +294,6 @@ struct blk_rq_stat {
 	s32 nr_samples;
 	s32 nr_batch;
 	u64 batch;
-	s64 time;
 };
 
 #endif /* __LINUX_BLK_TYPES_H */
include/linux/blkdev.h +8 -2
index 5a7da607ca04..1a7dc42a8918 100644
@@ -40,6 +40,8 @@ struct blkcg_gq;
 struct blk_flush_queue;
 struct pr_ops;
 struct rq_wb;
+struct blk_queue_stats;
+struct blk_stat_callback;
 
 #define BLKDEV_MIN_RQ	4
 #define BLKDEV_MAX_RQ	128	/* Default maximum */
@@ -388,6 +390,7 @@ struct request_queue {
 	int			nr_rqs[2];	/* # allocated [a]sync rqs */
 	int			nr_rqs_elvpriv;	/* # allocated rqs w/ elvpriv */
 
+	struct blk_queue_stats	*stats;
 	struct rq_wb		*rq_wb;
 
 	/*
@@ -505,8 +508,6 @@ struct request_queue {
 	unsigned int		nr_sorted;
 	unsigned int		in_flight[2];
 
-	struct blk_rq_stat	rq_stats[2];
-
 	/*
 	 * Number of active block driver functions for which blk_drain_queue()
 	 * must wait. Must be incremented around functions that unlock the
@@ -516,6 +517,10 @@ struct request_queue {
 
 	unsigned int		rq_timeout;
 	int			poll_nsec;
+
+	struct blk_stat_callback	*poll_cb;
+	struct blk_rq_stat	poll_stat[2];
+
 	struct timer_list	timeout;
 	struct work_struct	timeout_work;
 	struct list_head	timeout_list;
@@ -611,6 +616,7 @@ struct request_queue {
 #define QUEUE_FLAG_DAX         26	/* device supports DAX */
 #define QUEUE_FLAG_STATS       27	/* track rq completion times */
 #define QUEUE_FLAG_RESTART     28	/* queue needs restart at completion */
+#define QUEUE_FLAG_POLL_STATS  29	/* collecting stats for hybrid polling */
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_STACKABLE)	|	\
Signature · 1 line
2.12.0
#5 lore raw
In reply to Omar Sandoval
Quoted 20 lines · Omar Sandoval

From: Omar Sandoval <[email protected]>

This patchset generalizes the blk-stats infrastructure to allow users to register a callback to be called at a given time with the statistics of requests completed during that window. Writeback throttling and hybrid polling are converted to the new infrastructure. The new Kyber I/O scheduler uses this, as well (but it needs to be rebased on this v2).

The details are in patch 4, which is the actual conversion. Patches 1-3 are preparation cleanups.

Changes since v1:

- Now the user can subdivide stats into arbitrary buckets. Both in-tree users just do reads vs. writes, but we can extend poll based on request size in the future
- blk_stat_arm_callback() became blk_stat_activate_msecs() and blk_stat_activate_nsecs()
- The poll statistics are exposed in debugfs

Quoted 5 lines

Omar Sandoval (4):
block: remove extra calls to wbt_exit()
blk-stat: use READ and WRITE instead of BLK_STAT_{READ,WRITE}
blk-stat: move BLK_RQ_STAT_BATCH definition to blk-stat.c
blk-stat: convert to callback-based statistics reporting

Added for 4.12, thanks Omar. This is a nice improvement for the writeback throttling as well, since solves the issues with the stats and throttling having separate and unsynced windows.

Signature · 1 line
Jens Axboe