[PATCH 1/2 v4] block/throttle: remove redundant type transition

[PATCH 1/2 v4] block/throttle: remove redundant type transition

[Robin Dong]

From: Robin Dong <sanbai@taobao.com>

We don't need to convert tg to blkg and then convert it back in
throtl_update_dispatch_stats().

Signed-off-by: Robin Dong <sanbai@taobao.com>
---
 block/blk-throttle.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index a9664fa..46ddeff 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -674,10 +674,9 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg,
 	return 0;
 }
 
-static void throtl_update_dispatch_stats(struct blkcg_gq *blkg, u64 bytes,
+static void throtl_update_dispatch_stats(struct throtl_grp *tg, u64 bytes,
 					 int rw)
 {
-	struct throtl_grp *tg = blkg_to_tg(blkg);
 	struct tg_stats_cpu *stats_cpu;
 	unsigned long flags;
 
@@ -708,7 +707,7 @@ static void throtl_charge_bio(struct throtl_grp *tg, struct bio *bio)
 	tg->bytes_disp[rw] += bio->bi_size;
 	tg->io_disp[rw]++;
 
-	throtl_update_dispatch_stats(tg_to_blkg(tg), bio->bi_size, bio->bi_rw);
+	throtl_update_dispatch_stats(tg, bio->bi_size, bio->bi_rw);
 }
 
 static void throtl_add_bio_tg(struct throtl_data *td, struct throtl_grp *tg,
@@ -1127,7 +1126,7 @@ bool blk_throtl_bio(struct request_queue *q, struct bio *bio)
 	tg = throtl_lookup_tg(td, blkcg);
 	if (tg) {
 		if (tg_no_rule_group(tg, rw)) {
-			throtl_update_dispatch_stats(tg_to_blkg(tg),
+			throtl_update_dispatch_stats(tg,
 						     bio->bi_size, bio->bi_rw);
 			goto out_unlock_rcu;
 		}
-- 
1.7.1

[PATCH 2/2 v4] block/throttle: Add IO submitted information in blkio.throttle

[Robin Dong]

From: Robin Dong <sanbai@taobao.com>

Currently, if the IO is throttled by io-throttle, the system admin has no idea
of the situation and can't report it to the real application user about that
he/she has to do something.

So this patch adds a new interface named blkio.throttle.io_submitted which
exposes the number of bios that have been sent into blk-throttle therefore the
user could calculate the difference from throttle.io_serviced to see how many
IOs are currently throttled.

Cc: Tejun Heo <tj@kernel.org>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Tao Ma <boyu.mt@taobao.com>
Signed-off-by: Robin Dong <sanbai@taobao.com>
---
v3 <-- v2:
 - Use nr-queued[] of struct throtl_grp for stats instaed of adding new blkg_rwstat.

v4 <-- v3:
 - Add two new blkg_rwstat arguments to count total bios be sent into blk_throttle.

 block/blk-throttle.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 46ddeff..c6391b5 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -46,6 +46,10 @@ struct tg_stats_cpu {
 	struct blkg_rwstat		service_bytes;
 	/* total IOs serviced, post merge */
 	struct blkg_rwstat		serviced;
+	/* total bytes submitted into blk-throttle */
+	struct blkg_rwstat		submit_bytes;
+	/* total IOs submitted into blk-throttle */
+	struct blkg_rwstat		submitted;
 };
 
 struct throtl_grp {
@@ -266,6 +270,8 @@ static void throtl_pd_reset_stats(struct blkcg_gq *blkg)
 
 		blkg_rwstat_reset(&sc->service_bytes);
 		blkg_rwstat_reset(&sc->serviced);
+		blkg_rwstat_reset(&sc->submit_bytes);
+		blkg_rwstat_reset(&sc->submitted);
 	}
 }
 
@@ -699,6 +705,30 @@ static void throtl_update_dispatch_stats(struct throtl_grp *tg, u64 bytes,
 	local_irq_restore(flags);
 }
 
+static void throtl_update_submit_stats(struct throtl_grp *tg, u64 bytes, int rw)
+{
+	struct tg_stats_cpu *stats_cpu;
+	unsigned long flags;
+
+	/* If per cpu stats are not allocated yet, don't do any accounting. */
+	if (tg->stats_cpu == NULL)
+		return;
+
+	/*
+	 * Disabling interrupts to provide mutual exclusion between two
+	 * writes on same cpu. It probably is not needed for 64bit. Not
+	 * optimizing that case yet.
+	 */
+	local_irq_save(flags);
+
+	stats_cpu = this_cpu_ptr(tg->stats_cpu);
+
+	blkg_rwstat_add(&stats_cpu->submitted, rw, 1);
+	blkg_rwstat_add(&stats_cpu->submit_bytes, rw, bytes);
+
+	local_irq_restore(flags);
+}
+
 static void throtl_charge_bio(struct throtl_grp *tg, struct bio *bio)
 {
 	bool rw = bio_data_dir(bio);
@@ -1084,6 +1114,16 @@ static struct cftype throtl_files[] = {
 		.private = offsetof(struct tg_stats_cpu, serviced),
 		.read_seq_string = tg_print_cpu_rwstat,
 	},
+	{
+		.name = "throttle.io_submit_bytes",
+		.private = offsetof(struct tg_stats_cpu, submit_bytes),
+		.read_seq_string = tg_print_cpu_rwstat,
+	},
+	{
+		.name = "throttle.io_submitted",
+		.private = offsetof(struct tg_stats_cpu, submitted),
+		.read_seq_string = tg_print_cpu_rwstat,
+	},
 	{ }	/* terminate */
 };
 
@@ -1128,6 +1168,8 @@ bool blk_throtl_bio(struct request_queue *q, struct bio *bio)
 		if (tg_no_rule_group(tg, rw)) {
 			throtl_update_dispatch_stats(tg,
 						     bio->bi_size, bio->bi_rw);
+			throtl_update_submit_stats(tg,
+					bio->bi_size, bio->bi_rw);
 			goto out_unlock_rcu;
 		}
 	}
@@ -1141,6 +1183,7 @@ bool blk_throtl_bio(struct request_queue *q, struct bio *bio)
 	if (unlikely(!tg))
 		goto out_unlock;
 
+	throtl_update_submit_stats(tg, bio->bi_size, bio->bi_rw);
 	if (tg->nr_queued[rw]) {
 		/*
 		 * There is already another bio queued in same dir. No
-- 
1.7.1

Re: [PATCH 2/2 v4] block/throttle: Add IO submitted information in blkio.throttle

[Vivek Goyal]

On Fri, Oct 26, 2012 at 12:47:48PM +0800, Robin Dong wrote:

[..]
> @@ -1084,6 +1114,16 @@ static struct cftype throtl_files[] = {
>  		.private = offsetof(struct tg_stats_cpu, serviced),
>  		.read_seq_string = tg_print_cpu_rwstat,
>  	},
> +	{
> +		.name = "throttle.io_submit_bytes",

Do we really need io_submit_bytes stats? Your need seems to be able to
figure out if there are pending IOs in the group and if you need to
increase the bandwidth. For that, isn't number of bios enough?

Also even if we retain bytes, let us change it to "io_submitted_bytes".

Thanks
Vivek

Re: [PATCH 2/2 v4] block/throttle: Add IO submitted information in blkio.throttle

[Robin Dong]

2012/10/27 Vivek Goyal <vgoyal@redhat.com>:
> On Fri, Oct 26, 2012 at 12:47:48PM +0800, Robin Dong wrote:
>
> [..]
>> @@ -1084,6 +1114,16 @@ static struct cftype throtl_files[] = {
>>               .private = offsetof(struct tg_stats_cpu, serviced),
>>               .read_seq_string = tg_print_cpu_rwstat,
>>       },
>> +     {
>> +             .name = "throttle.io_submit_bytes",
>
> Do we really need io_submit_bytes stats? Your need seems to be able to
> figure out if there are pending IOs in the group and if you need to
> increase the bandwidth. For that, isn't number of bios enough?

 I just want to be more consistent to "io_service_bytes" and "io_serviced" pair

>
> Also even if we retain bytes, let us change it to "io_submitted_bytes".

Since "io_service_bytes" is "service", I use "io_submit_bytes"....
Never mind, "io_submitted_bytes" is much better.

>
> Thanks
> Vivek



-- 
--
Best Regard
Robin Dong