All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] blk-stat: Add ability to not bucket IO; improve IO polling.
@ 2017-04-05 17:39 ` sbates
  0 siblings, 0 replies; 12+ messages in thread
From: sbates @ 2017-04-05 17:39 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-nvme, Damien.LeMoal, osandov, sbates, sagi

From: Stephen Bates <sbates@raithlin.com>

Omar recently developed some patches for block layer stats that use
callbacks to determine which bucket an IO should be considered for. At
the same time there was discussion at LSF/MM that we might not want to
consider all IO when generating stats for certain algorithms (e.g. IO
completion polling) or to bucket them in a more optimal fashion.

This set does two things. It makes the bucket callback for stats
signed so we can now ignore IO that cause a negative to be returned
from the bucket function. It then improves the IO polling latency
estimations by bucketing stats based on IO size and direction.

This patchset applies cleanly on 6809ef67eb7b4b68d (Merge branch
'for-4.12/block' into for-next) in Jens' for-next tree.

I've lightly tested this using QEMU and a real NVMe low-latency
device. I do not have performance number yet. Feedback would be
appreciated! I am not *super* happy with how the bucketing by size is
done. Any suggestions on how to improve this would be appreciated!

Cc: Damien.LeMoal@wdc.com
Cc: osandov@osandov.com

Changes since v1:
  Dropped the cast in blk_stat_rq_ddir() as per Omar's suggestion.
  Moved to an array of buckets based on IO size rather than a filter
  as suggested by Jens and Damien.

Stephen Bates (2):
  blk-stat: convert blk-stat bucket callback to signed
  blk-mq: Add a polling specific stats function

 block/blk-mq.c   | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 block/blk-stat.c |  6 ++++--
 block/blk-stat.h |  9 +++++----
 3 files changed, 53 insertions(+), 16 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 0/2] blk-stat: Add ability to not bucket IO; improve IO polling.
@ 2017-04-05 17:39 ` sbates
  0 siblings, 0 replies; 12+ messages in thread
From: sbates @ 2017-04-05 17:39 UTC (permalink / raw)


From: Stephen Bates <sbates@raithlin.com>

Omar recently developed some patches for block layer stats that use
callbacks to determine which bucket an IO should be considered for. At
the same time there was discussion at LSF/MM that we might not want to
consider all IO when generating stats for certain algorithms (e.g. IO
completion polling) or to bucket them in a more optimal fashion.

This set does two things. It makes the bucket callback for stats
signed so we can now ignore IO that cause a negative to be returned
from the bucket function. It then improves the IO polling latency
estimations by bucketing stats based on IO size and direction.

This patchset applies cleanly on 6809ef67eb7b4b68d (Merge branch
'for-4.12/block' into for-next) in Jens' for-next tree.

I've lightly tested this using QEMU and a real NVMe low-latency
device. I do not have performance number yet. Feedback would be
appreciated! I am not *super* happy with how the bucketing by size is
done. Any suggestions on how to improve this would be appreciated!

Cc: Damien.LeMoal at wdc.com
Cc: osandov at osandov.com

Changes since v1:
  Dropped the cast in blk_stat_rq_ddir() as per Omar's suggestion.
  Moved to an array of buckets based on IO size rather than a filter
  as suggested by Jens and Damien.

Stephen Bates (2):
  blk-stat: convert blk-stat bucket callback to signed
  blk-mq: Add a polling specific stats function

 block/blk-mq.c   | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 block/blk-stat.c |  6 ++++--
 block/blk-stat.h |  9 +++++----
 3 files changed, 53 insertions(+), 16 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/2] blk-stat: convert blk-stat bucket callback to signed
  2017-04-05 17:39 ` sbates
@ 2017-04-05 17:39   ` sbates
  -1 siblings, 0 replies; 12+ messages in thread
From: sbates @ 2017-04-05 17:39 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-nvme, Damien.LeMoal, osandov, sbates, sagi

From: Stephen Bates <sbates@raithlin.com>

In order to allow for filtering of IO based on some other properties
of the request than direction we allow the bucket function to return
an int.

If the bucket callback returns a negative do no count it in the stats
accumulation.

Signed-off-by: Stephen Bates <sbates@raithlin.com>
---
 block/blk-stat.c | 6 ++++--
 block/blk-stat.h | 9 +++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/blk-stat.c b/block/blk-stat.c
index e77ec52..dde9d39 100644
--- a/block/blk-stat.c
+++ b/block/blk-stat.c
@@ -19,7 +19,7 @@ struct blk_queue_stats {
 	bool enable_accounting;
 };
 
-unsigned int blk_stat_rq_ddir(const struct request *rq)
+int blk_stat_rq_ddir(const struct request *rq)
 {
 	return rq_data_dir(rq);
 }
@@ -104,6 +104,8 @@ void blk_stat_add(struct request *rq)
 	list_for_each_entry_rcu(cb, &q->stats->callbacks, list) {
 		if (blk_stat_is_active(cb)) {
 			bucket = cb->bucket_fn(rq);
+			if (bucket < 0)
+				continue;
 			stat = &this_cpu_ptr(cb->cpu_stat)[bucket];
 			__blk_stat_add(stat, value);
 		}
@@ -135,7 +137,7 @@ static void blk_stat_timer_fn(unsigned long data)
 
 struct blk_stat_callback *
 blk_stat_alloc_callback(void (*timer_fn)(struct blk_stat_callback *),
-			unsigned int (*bucket_fn)(const struct request *),
+			int (*bucket_fn)(const struct request *),
 			unsigned int buckets, void *data)
 {
 	struct blk_stat_callback *cb;
diff --git a/block/blk-stat.h b/block/blk-stat.h
index 53f08a6..622a62c 100644
--- a/block/blk-stat.h
+++ b/block/blk-stat.h
@@ -48,9 +48,10 @@ struct blk_stat_callback {
 
 	/**
 	 * @bucket_fn: Given a request, returns which statistics bucket it
-	 * should be accounted under.
+	 * should be accounted under. Return -1 for no bucket for this
+	 * request.
 	 */
-	unsigned int (*bucket_fn)(const struct request *);
+	int (*bucket_fn)(const struct request *);
 
 	/**
 	 * @buckets: Number of statistics buckets.
@@ -120,7 +121,7 @@ void blk_stat_enable_accounting(struct request_queue *q);
  *
  * Return: Data direction of the request, either READ or WRITE.
  */
-unsigned int blk_stat_rq_ddir(const struct request *rq);
+int blk_stat_rq_ddir(const struct request *rq);
 
 /**
  * blk_stat_alloc_callback() - Allocate a block statistics callback.
@@ -135,7 +136,7 @@ unsigned int blk_stat_rq_ddir(const struct request *rq);
  */
 struct blk_stat_callback *
 blk_stat_alloc_callback(void (*timer_fn)(struct blk_stat_callback *),
-			unsigned int (*bucket_fn)(const struct request *),
+			int (*bucket_fn)(const struct request *),
 			unsigned int buckets, void *data);
 
 /**
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 1/2] blk-stat: convert blk-stat bucket callback to signed
@ 2017-04-05 17:39   ` sbates
  0 siblings, 0 replies; 12+ messages in thread
From: sbates @ 2017-04-05 17:39 UTC (permalink / raw)


From: Stephen Bates <sbates@raithlin.com>

In order to allow for filtering of IO based on some other properties
of the request than direction we allow the bucket function to return
an int.

If the bucket callback returns a negative do no count it in the stats
accumulation.

Signed-off-by: Stephen Bates <sbates at raithlin.com>
---
 block/blk-stat.c | 6 ++++--
 block/blk-stat.h | 9 +++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/blk-stat.c b/block/blk-stat.c
index e77ec52..dde9d39 100644
--- a/block/blk-stat.c
+++ b/block/blk-stat.c
@@ -19,7 +19,7 @@ struct blk_queue_stats {
 	bool enable_accounting;
 };
 
-unsigned int blk_stat_rq_ddir(const struct request *rq)
+int blk_stat_rq_ddir(const struct request *rq)
 {
 	return rq_data_dir(rq);
 }
@@ -104,6 +104,8 @@ void blk_stat_add(struct request *rq)
 	list_for_each_entry_rcu(cb, &q->stats->callbacks, list) {
 		if (blk_stat_is_active(cb)) {
 			bucket = cb->bucket_fn(rq);
+			if (bucket < 0)
+				continue;
 			stat = &this_cpu_ptr(cb->cpu_stat)[bucket];
 			__blk_stat_add(stat, value);
 		}
@@ -135,7 +137,7 @@ static void blk_stat_timer_fn(unsigned long data)
 
 struct blk_stat_callback *
 blk_stat_alloc_callback(void (*timer_fn)(struct blk_stat_callback *),
-			unsigned int (*bucket_fn)(const struct request *),
+			int (*bucket_fn)(const struct request *),
 			unsigned int buckets, void *data)
 {
 	struct blk_stat_callback *cb;
diff --git a/block/blk-stat.h b/block/blk-stat.h
index 53f08a6..622a62c 100644
--- a/block/blk-stat.h
+++ b/block/blk-stat.h
@@ -48,9 +48,10 @@ struct blk_stat_callback {
 
 	/**
 	 * @bucket_fn: Given a request, returns which statistics bucket it
-	 * should be accounted under.
+	 * should be accounted under. Return -1 for no bucket for this
+	 * request.
 	 */
-	unsigned int (*bucket_fn)(const struct request *);
+	int (*bucket_fn)(const struct request *);
 
 	/**
 	 * @buckets: Number of statistics buckets.
@@ -120,7 +121,7 @@ void blk_stat_enable_accounting(struct request_queue *q);
  *
  * Return: Data direction of the request, either READ or WRITE.
  */
-unsigned int blk_stat_rq_ddir(const struct request *rq);
+int blk_stat_rq_ddir(const struct request *rq);
 
 /**
  * blk_stat_alloc_callback() - Allocate a block statistics callback.
@@ -135,7 +136,7 @@ unsigned int blk_stat_rq_ddir(const struct request *rq);
  */
 struct blk_stat_callback *
 blk_stat_alloc_callback(void (*timer_fn)(struct blk_stat_callback *),
-			unsigned int (*bucket_fn)(const struct request *),
+			int (*bucket_fn)(const struct request *),
 			unsigned int buckets, void *data);
 
 /**
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] blk-mq: Add a polling specific stats function
  2017-04-05 17:39 ` sbates
@ 2017-04-05 17:39   ` sbates
  -1 siblings, 0 replies; 12+ messages in thread
From: sbates @ 2017-04-05 17:39 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-nvme, Damien.LeMoal, osandov, sbates, sagi

From: Stephen Bates <sbates@raithlin.com>

Rather than bucketing IO statisics based on direction only we also
bucket based on the IO size. This leads to improved polling
performance. Update the bucket callback function and use it in the
polling latency estimation.

Signed-off-by: Stephen Bates <sbates@raithlin.com>
---
 block/blk-mq.c | 53 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 061fc2c..8fb1fb0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -42,6 +42,33 @@ static LIST_HEAD(all_q_list);
 static void blk_mq_poll_stats_start(struct request_queue *q);
 static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb);
 
+static int blk_mq_poll_stats_bkt(const struct request *rq)
+{
+	int dir, bytes;
+
+	dir = blk_stat_rq_ddir(rq);
+	bytes = blk_rq_bytes(rq);
+
+	if (bytes <= 512)
+		return dir;
+	else if (bytes <= 4096)
+		return dir + 2;
+	else if (bytes <= 8192)
+		return dir + 4;
+	else if (bytes <= 16384)
+		return dir + 6;
+	else if (bytes <= 32768)
+		return dir + 8;
+	else if (bytes <= 65536)
+		return dir + 10;
+	else
+		return dir + 12;
+
+	return -1;
+}
+/* Must be consisitent with function above */
+#define BLK_MQ_POLL_STATS_BKTS 14
+
 /*
  * Check if any of the ctx's have pending work in this hardware queue
  */
@@ -2245,7 +2272,8 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 	q->mq_ops = set->ops;
 
 	q->poll_cb = blk_stat_alloc_callback(blk_mq_poll_stats_fn,
-					     blk_stat_rq_ddir, 2, q);
+					     blk_mq_poll_stats_bkt,
+					     BLK_MQ_POLL_STATS_BKTS, q);
 	if (!q->poll_cb)
 		goto err_exit;
 
@@ -2663,11 +2691,12 @@ static void blk_mq_poll_stats_start(struct request_queue *q)
 static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb)
 {
 	struct request_queue *q = cb->data;
+	int bucket;
 
-	if (cb->stat[READ].nr_samples)
-		q->poll_stat[READ] = cb->stat[READ];
-	if (cb->stat[WRITE].nr_samples)
-		q->poll_stat[WRITE] = cb->stat[WRITE];
+	for (bucket = 0; bucket < BLK_MQ_POLL_STATS_BKTS; bucket++) {
+		if (cb->stat[bucket].nr_samples)
+			q->poll_stat[bucket] = cb->stat[bucket];
+	}
 }
 
 static unsigned long blk_mq_poll_nsecs(struct request_queue *q,
@@ -2675,6 +2704,7 @@ static unsigned long blk_mq_poll_nsecs(struct request_queue *q,
 				       struct request *rq)
 {
 	unsigned long ret = 0;
+	int bucket;
 
 	/*
 	 * If stats collection isn't on, don't sleep but turn it on for
@@ -2689,12 +2719,15 @@ static unsigned long blk_mq_poll_nsecs(struct request_queue *q,
 	 * For instance, if the completion latencies are tight, we can
 	 * get closer than just half the mean. This is especially
 	 * important on devices where the completion latencies are longer
-	 * than ~10 usec.
+	 * than ~10 usec. We do use the stats for the relevant IO size
+	 * if available which does lead to better estimates.
 	 */
-	if (req_op(rq) == REQ_OP_READ && q->poll_stat[READ].nr_samples)
-		ret = (q->poll_stat[READ].mean + 1) / 2;
-	else if (req_op(rq) == REQ_OP_WRITE && q->poll_stat[WRITE].nr_samples)
-		ret = (q->poll_stat[WRITE].mean + 1) / 2;
+	bucket = blk_mq_poll_stats_bkt(rq);
+	if (bucket < 0)
+		return ret;
+
+	if (q->poll_stat[bucket].nr_samples)
+		ret = (q->poll_stat[bucket].mean + 1) / 2;
 
 	return ret;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] blk-mq: Add a polling specific stats function
@ 2017-04-05 17:39   ` sbates
  0 siblings, 0 replies; 12+ messages in thread
From: sbates @ 2017-04-05 17:39 UTC (permalink / raw)


From: Stephen Bates <sbates@raithlin.com>

Rather than bucketing IO statisics based on direction only we also
bucket based on the IO size. This leads to improved polling
performance. Update the bucket callback function and use it in the
polling latency estimation.

Signed-off-by: Stephen Bates <sbates at raithlin.com>
---
 block/blk-mq.c | 53 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 061fc2c..8fb1fb0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -42,6 +42,33 @@ static LIST_HEAD(all_q_list);
 static void blk_mq_poll_stats_start(struct request_queue *q);
 static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb);
 
+static int blk_mq_poll_stats_bkt(const struct request *rq)
+{
+	int dir, bytes;
+
+	dir = blk_stat_rq_ddir(rq);
+	bytes = blk_rq_bytes(rq);
+
+	if (bytes <= 512)
+		return dir;
+	else if (bytes <= 4096)
+		return dir + 2;
+	else if (bytes <= 8192)
+		return dir + 4;
+	else if (bytes <= 16384)
+		return dir + 6;
+	else if (bytes <= 32768)
+		return dir + 8;
+	else if (bytes <= 65536)
+		return dir + 10;
+	else
+		return dir + 12;
+
+	return -1;
+}
+/* Must be consisitent with function above */
+#define BLK_MQ_POLL_STATS_BKTS 14
+
 /*
  * Check if any of the ctx's have pending work in this hardware queue
  */
@@ -2245,7 +2272,8 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 	q->mq_ops = set->ops;
 
 	q->poll_cb = blk_stat_alloc_callback(blk_mq_poll_stats_fn,
-					     blk_stat_rq_ddir, 2, q);
+					     blk_mq_poll_stats_bkt,
+					     BLK_MQ_POLL_STATS_BKTS, q);
 	if (!q->poll_cb)
 		goto err_exit;
 
@@ -2663,11 +2691,12 @@ static void blk_mq_poll_stats_start(struct request_queue *q)
 static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb)
 {
 	struct request_queue *q = cb->data;
+	int bucket;
 
-	if (cb->stat[READ].nr_samples)
-		q->poll_stat[READ] = cb->stat[READ];
-	if (cb->stat[WRITE].nr_samples)
-		q->poll_stat[WRITE] = cb->stat[WRITE];
+	for (bucket = 0; bucket < BLK_MQ_POLL_STATS_BKTS; bucket++) {
+		if (cb->stat[bucket].nr_samples)
+			q->poll_stat[bucket] = cb->stat[bucket];
+	}
 }
 
 static unsigned long blk_mq_poll_nsecs(struct request_queue *q,
@@ -2675,6 +2704,7 @@ static unsigned long blk_mq_poll_nsecs(struct request_queue *q,
 				       struct request *rq)
 {
 	unsigned long ret = 0;
+	int bucket;
 
 	/*
 	 * If stats collection isn't on, don't sleep but turn it on for
@@ -2689,12 +2719,15 @@ static unsigned long blk_mq_poll_nsecs(struct request_queue *q,
 	 * For instance, if the completion latencies are tight, we can
 	 * get closer than just half the mean. This is especially
 	 * important on devices where the completion latencies are longer
-	 * than ~10 usec.
+	 * than ~10 usec. We do use the stats for the relevant IO size
+	 * if available which does lead to better estimates.
 	 */
-	if (req_op(rq) == REQ_OP_READ && q->poll_stat[READ].nr_samples)
-		ret = (q->poll_stat[READ].mean + 1) / 2;
-	else if (req_op(rq) == REQ_OP_WRITE && q->poll_stat[WRITE].nr_samples)
-		ret = (q->poll_stat[WRITE].mean + 1) / 2;
+	bucket = blk_mq_poll_stats_bkt(rq);
+	if (bucket < 0)
+		return ret;
+
+	if (q->poll_stat[bucket].nr_samples)
+		ret = (q->poll_stat[bucket].mean + 1) / 2;
 
 	return ret;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] blk-mq: Add a polling specific stats function
  2017-04-05 17:39   ` sbates
@ 2017-04-05 18:14     ` Jens Axboe
  -1 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2017-04-05 18:14 UTC (permalink / raw)
  To: sbates; +Cc: linux-block, linux-nvme, Damien.LeMoal, osandov, sagi

On 04/05/2017 11:39 AM, sbates@raithlin.com wrote:
> @@ -42,6 +42,33 @@ static LIST_HEAD(all_q_list);
>  static void blk_mq_poll_stats_start(struct request_queue *q);
>  static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb);
>  
> +static int blk_mq_poll_stats_bkt(const struct request *rq)
> +{
> +	int dir, bytes;
> +
> +	dir = blk_stat_rq_ddir(rq);
> +	bytes = blk_rq_bytes(rq);
> +
> +	if (bytes <= 512)
> +		return dir;
> +	else if (bytes <= 4096)
> +		return dir + 2;
> +	else if (bytes <= 8192)
> +		return dir + 4;
> +	else if (bytes <= 16384)
> +		return dir + 6;
> +	else if (bytes <= 32768)
> +		return dir + 8;
> +	else if (bytes <= 65536)
> +		return dir + 10;
> +	else
> +		return dir + 12;

Why not just have 8 buckets, and make it:

	bucket = ddir + ilog2(bytes) - 9;

and cap it at MAX_BUCKET (8) and put all those above into the top
bucket.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] blk-mq: Add a polling specific stats function
@ 2017-04-05 18:14     ` Jens Axboe
  0 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2017-04-05 18:14 UTC (permalink / raw)


On 04/05/2017 11:39 AM, sbates@raithlin.com wrote:
> @@ -42,6 +42,33 @@ static LIST_HEAD(all_q_list);
>  static void blk_mq_poll_stats_start(struct request_queue *q);
>  static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb);
>  
> +static int blk_mq_poll_stats_bkt(const struct request *rq)
> +{
> +	int dir, bytes;
> +
> +	dir = blk_stat_rq_ddir(rq);
> +	bytes = blk_rq_bytes(rq);
> +
> +	if (bytes <= 512)
> +		return dir;
> +	else if (bytes <= 4096)
> +		return dir + 2;
> +	else if (bytes <= 8192)
> +		return dir + 4;
> +	else if (bytes <= 16384)
> +		return dir + 6;
> +	else if (bytes <= 32768)
> +		return dir + 8;
> +	else if (bytes <= 65536)
> +		return dir + 10;
> +	else
> +		return dir + 12;

Why not just have 8 buckets, and make it:

	bucket = ddir + ilog2(bytes) - 9;

and cap it at MAX_BUCKET (8) and put all those above into the top
bucket.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] blk-mq: Add a polling specific stats function
  2017-04-05 18:14     ` Jens Axboe
@ 2017-04-07 12:11       ` Stephen  Bates
  -1 siblings, 0 replies; 12+ messages in thread
From: Stephen  Bates @ 2017-04-07 12:11 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, linux-nvme, Damien.LeMoal, osandov, sagi

T24gMjAxNy0wNC0wNSwgNzoxNCBQTSwgIkplbnMgQXhib2UiIDxheGJvZUBrZXJuZWwuZGs+IHdy
b3RlOg0KDQo+IFdoeSBub3QganVzdCBoYXZlIDggYnVja2V0cywgYW5kIG1ha2UgaXQ6DQo+DQo+
CWJ1Y2tldCA9IGRkaXIgKyBpbG9nMihieXRlcykgLSA5Ow0KPg0KPiBhbmQgY2FwIGl0IGF0IE1B
WF9CVUNLRVQgKDgpIGFuZCBwdXQgYWxsIHRob3NlIGFib3ZlIGludG8gdGhlIHRvcA0KPiBidWNr
ZXQuDQoNClRoYW5rcy4gSG93ZXZlciwgdGhhdCBlcXVhdGlvbiBkb2VzIG5vdCBkaWZmZXJlbnRp
YXRlIGJldHdlZW4gZGlyZWN0aW9uIGFuZCBzaXplLiBJbnN0ZWFkIHdlIGNhbiB1c2UNCg0KYnVj
a2V0ID0gZGRpciArIDIqKGlsb2cyKGJ5dGVzKSAtIDkpOw0KDQphbmQgdGhlbiBiaW4gYW55IElP
IG92ZXIgNjRLIGluIHRoZSBsYXJnZXN0IG9mIHRoZSB0d28gYnVja2V0cyBiYXNlZCBvbiBkaXJl
Y3Rpb24uIEnigJlsbCBpbXBsZW1lbnQgdGhpcyBpbiBhIHYz4oCmLg0KDQpDaGVlcnMNCg0KU3Rl
cGhlbg0KDQoNCg0KDQo=

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] blk-mq: Add a polling specific stats function
@ 2017-04-07 12:11       ` Stephen  Bates
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen  Bates @ 2017-04-07 12:11 UTC (permalink / raw)


On 2017-04-05, 7:14 PM, "Jens Axboe" <axboe@kernel.dk> wrote:

> Why not just have 8 buckets, and make it:
>
>	bucket = ddir + ilog2(bytes) - 9;
>
> and cap it at MAX_BUCKET (8) and put all those above into the top
> bucket.

Thanks. However, that equation does not differentiate between direction and size. Instead we can use

bucket = ddir + 2*(ilog2(bytes) - 9);

and then bin any IO over 64K in the largest of the two buckets based on direction. I?ll implement this in a v3?.

Cheers

Stephen

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] blk-mq: Add a polling specific stats function
  2017-04-07 12:11       ` Stephen  Bates
@ 2017-04-07 14:01         ` Jens Axboe
  -1 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2017-04-07 14:01 UTC (permalink / raw)
  To: Stephen Bates; +Cc: linux-block, linux-nvme, Damien.LeMoal, osandov, sagi

On 04/07/2017 06:11 AM, Stephen  Bates wrote:
> On 2017-04-05, 7:14 PM, "Jens Axboe" <axboe@kernel.dk> wrote:
> 
>> Why not just have 8 buckets, and make it:
>>
>> 	bucket = ddir + ilog2(bytes) - 9;
>>
>> and cap it at MAX_BUCKET (8) and put all those above into the top
>> bucket.
> 
> Thanks. However, that equation does not differentiate between
> direction and size. Instead we can use
> 
> bucket = ddir + 2*(ilog2(bytes) - 9);

It would be cleaner to just embed the fact that we have 2 sets of
identical buckets, and return 

	bucket = ilog2(bytes) - 9;

and have poll_stat be indexed by:

	->poll_stat[ddir][bucket];

instead.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] blk-mq: Add a polling specific stats function
@ 2017-04-07 14:01         ` Jens Axboe
  0 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2017-04-07 14:01 UTC (permalink / raw)


On 04/07/2017 06:11 AM, Stephen  Bates wrote:
> On 2017-04-05, 7:14 PM, "Jens Axboe" <axboe@kernel.dk> wrote:
> 
>> Why not just have 8 buckets, and make it:
>>
>> 	bucket = ddir + ilog2(bytes) - 9;
>>
>> and cap it at MAX_BUCKET (8) and put all those above into the top
>> bucket.
> 
> Thanks. However, that equation does not differentiate between
> direction and size. Instead we can use
> 
> bucket = ddir + 2*(ilog2(bytes) - 9);

It would be cleaner to just embed the fact that we have 2 sets of
identical buckets, and return 

	bucket = ilog2(bytes) - 9;

and have poll_stat be indexed by:

	->poll_stat[ddir][bucket];

instead.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-04-07 14:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-05 17:39 [PATCH v2 0/2] blk-stat: Add ability to not bucket IO; improve IO polling sbates
2017-04-05 17:39 ` sbates
2017-04-05 17:39 ` [PATCH v2 1/2] blk-stat: convert blk-stat bucket callback to signed sbates
2017-04-05 17:39   ` sbates
2017-04-05 17:39 ` [PATCH v2 2/2] blk-mq: Add a polling specific stats function sbates
2017-04-05 17:39   ` sbates
2017-04-05 18:14   ` Jens Axboe
2017-04-05 18:14     ` Jens Axboe
2017-04-07 12:11     ` Stephen  Bates
2017-04-07 12:11       ` Stephen  Bates
2017-04-07 14:01       ` Jens Axboe
2017-04-07 14:01         ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.