Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
On Fri, Oct 19, 2012 at 12:36:00PM -0700, Tejun Heo wrote: > Hello, Vivek. > > On Fri, Oct 19, 2012 at 11:00:11AM -0400, Vivek Goyal wrote: > > > That way we can stick to the usual stats facility. > > > > So how does this help? Because it is a monotonically increasing value > > we can use per cpu stats without extra locking? Or somthing else? > > It's generally much simpler to expose dumb increasing counters. You > don't have to worry about mismatching decrements (for whatever reason, > percpu or segmented counters), so unless there's a pretty good reason > to deviate, why deviate? I really can't think of a reason to deviate. I was just curious of advantages. I think it does make sense to not get into decrement business and let user space subtract two values and figure out how much IO from the group is throttled. Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
Hello, Vivek. On Fri, Oct 19, 2012 at 11:00:11AM -0400, Vivek Goyal wrote: > > That way we can stick to the usual stats facility. > > So how does this help? Because it is a monotonically increasing value > we can use per cpu stats without extra locking? Or somthing else? It's generally much simpler to expose dumb increasing counters. You don't have to worry about mismatching decrements (for whatever reason, percpu or segmented counters), so unless there's a pretty good reason to deviate, why deviate? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
On Thu, Oct 18, 2012 at 04:24:04PM -0700, Tejun Heo wrote: > Hello, Vivek. > > On Wed, Oct 17, 2012 at 09:49:45AM -0400, Vivek Goyal wrote: > > Can you explain a bit more. Whe do you mean by "total number queued". I > > think throttle.io_queued will total number of bios queued in the cgroup > > at the time of query. > > Instead of exposing the number that blk-throttle currently has > deferred, we can expose the number of bios that have been sent to > blk-throttle and the number of bios which left blk-throttle, both > monotically increasing and the difference indicating the number being > deferred. Ok, I see it now. So we currently already maintain the number of IOs dispatched from blk-throttle in throttle.io_serviced. Now you are suggesting that maintain another counter which keeps track of total number IOs submitted to blk-throttle, say throttle.io_submitted? I think using throttle.io_queued will be little confusing because in CFQ we already use blkio.io_queued to represent number of IOs currently queued and it is not monotonically increasing value. > That way we can stick to the usual stats facility. So how does this help? Because it is a monotonically increasing value we can use per cpu stats without extra locking? Or somthing else? Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
On Thu, Oct 18, 2012 at 04:24:04PM -0700, Tejun Heo wrote: Hello, Vivek. On Wed, Oct 17, 2012 at 09:49:45AM -0400, Vivek Goyal wrote: Can you explain a bit more. Whe do you mean by total number queued. I think throttle.io_queued will total number of bios queued in the cgroup at the time of query. Instead of exposing the number that blk-throttle currently has deferred, we can expose the number of bios that have been sent to blk-throttle and the number of bios which left blk-throttle, both monotically increasing and the difference indicating the number being deferred. Ok, I see it now. So we currently already maintain the number of IOs dispatched from blk-throttle in throttle.io_serviced. Now you are suggesting that maintain another counter which keeps track of total number IOs submitted to blk-throttle, say throttle.io_submitted? I think using throttle.io_queued will be little confusing because in CFQ we already use blkio.io_queued to represent number of IOs currently queued and it is not monotonically increasing value. That way we can stick to the usual stats facility. So how does this help? Because it is a monotonically increasing value we can use per cpu stats without extra locking? Or somthing else? Thanks Vivek -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
Hello, Vivek. On Fri, Oct 19, 2012 at 11:00:11AM -0400, Vivek Goyal wrote: That way we can stick to the usual stats facility. So how does this help? Because it is a monotonically increasing value we can use per cpu stats without extra locking? Or somthing else? It's generally much simpler to expose dumb increasing counters. You don't have to worry about mismatching decrements (for whatever reason, percpu or segmented counters), so unless there's a pretty good reason to deviate, why deviate? Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
On Fri, Oct 19, 2012 at 12:36:00PM -0700, Tejun Heo wrote: Hello, Vivek. On Fri, Oct 19, 2012 at 11:00:11AM -0400, Vivek Goyal wrote: That way we can stick to the usual stats facility. So how does this help? Because it is a monotonically increasing value we can use per cpu stats without extra locking? Or somthing else? It's generally much simpler to expose dumb increasing counters. You don't have to worry about mismatching decrements (for whatever reason, percpu or segmented counters), so unless there's a pretty good reason to deviate, why deviate? I really can't think of a reason to deviate. I was just curious of advantages. I think it does make sense to not get into decrement business and let user space subtract two values and figure out how much IO from the group is throttled. Thanks Vivek -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
Hello, Vivek. On Wed, Oct 17, 2012 at 09:49:45AM -0400, Vivek Goyal wrote: > Can you explain a bit more. Whe do you mean by "total number queued". I > think throttle.io_queued will total number of bios queued in the cgroup > at the time of query. Instead of exposing the number that blk-throttle currently has deferred, we can expose the number of bios that have been sent to blk-throttle and the number of bios which left blk-throttle, both monotically increasing and the difference indicating the number being deferred. That way we can stick to the usual stats facility. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
Hello, Vivek. On Wed, Oct 17, 2012 at 09:49:45AM -0400, Vivek Goyal wrote: Can you explain a bit more. Whe do you mean by total number queued. I think throttle.io_queued will total number of bios queued in the cgroup at the time of query. Instead of exposing the number that blk-throttle currently has deferred, we can expose the number of bios that have been sent to blk-throttle and the number of bios which left blk-throttle, both monotically increasing and the difference indicating the number being deferred. That way we can stick to the usual stats facility. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
On Tue, Oct 16, 2012 at 04:27:06PM -0700, Tejun Heo wrote: [..] > > Changelog from v2: > > Use nr-queued[] of struct throtl_grp for stats instaed of adding new > > blkg_rwstat. > > As I wrote last time, I would prefer exposing the total number queued > to blk-throttle rather than exposing the number of bios being > currently held and let userland calculate from the difference from > throttle.io_serviced. That is simpler and more inline with all other > stats. Hi Tejun, Can you explain a bit more. Whe do you mean by "total number queued". I think throttle.io_queued will total number of bios queued in the cgroup at the time of query. Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
On Tue, Oct 16, 2012 at 04:27:06PM -0700, Tejun Heo wrote: [..] Changelog from v2: Use nr-queued[] of struct throtl_grp for stats instaed of adding new blkg_rwstat. As I wrote last time, I would prefer exposing the total number queued to blk-throttle rather than exposing the number of bios being currently held and let userland calculate from the difference from throttle.io_serviced. That is simpler and more inline with all other stats. Hi Tejun, Can you explain a bit more. Whe do you mean by total number queued. I think throttle.io_queued will total number of bios queued in the cgroup at the time of query. Thanks Vivek -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
Hello, Robin. On Tue, Oct 09, 2012 at 02:53:45PM +0800, Robin Dong wrote: > Currently, if the IO is throttled by io-throttle, the SA has no idea of > the situation and can't report it to the real application user about Please don't use "SA" in the commit message. Just write it out as "system admin" or "userspace" or whatever. It's only gonna confuse people trying to understand the commit later on. > that he/she has to do something. So this patch adds a new interface > named blkio.throttle.io_queued which indicates how many IOs are > currently throttled. > > The nr_queued[] of struct throtl_grp is of type "unsigned int" and updates > to it are atomic both at 32bit and 64bit platforms, so we could just > read tg->nr_queued only under blkcg->lock. > > Changelog from v2: > Use nr-queued[] of struct throtl_grp for stats instaed of adding new > blkg_rwstat. As I wrote last time, I would prefer exposing the total number queued to blk-throttle rather than exposing the number of bios being currently held and let userland calculate from the difference from throttle.io_serviced. That is simpler and more inline with all other stats. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
Hello, Robin. On Tue, Oct 09, 2012 at 02:53:45PM +0800, Robin Dong wrote: Currently, if the IO is throttled by io-throttle, the SA has no idea of the situation and can't report it to the real application user about Please don't use SA in the commit message. Just write it out as system admin or userspace or whatever. It's only gonna confuse people trying to understand the commit later on. that he/she has to do something. So this patch adds a new interface named blkio.throttle.io_queued which indicates how many IOs are currently throttled. The nr_queued[] of struct throtl_grp is of type unsigned int and updates to it are atomic both at 32bit and 64bit platforms, so we could just read tg-nr_queued only under blkcg-lock. Changelog from v2: Use nr-queued[] of struct throtl_grp for stats instaed of adding new blkg_rwstat. As I wrote last time, I would prefer exposing the total number queued to blk-throttle rather than exposing the number of bios being currently held and let userland calculate from the difference from throttle.io_serviced. That is simpler and more inline with all other stats. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
On Tue, Oct 09, 2012 at 02:53:45PM +0800, Robin Dong wrote: > From: Robin Dong > > Currently, if the IO is throttled by io-throttle, the SA 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_queued which indicates how many IOs are > currently throttled. > > The nr_queued[] of struct throtl_grp is of type "unsigned int" and updates > to it are atomic both at 32bit and 64bit platforms, so we could just > read tg->nr_queued only under blkcg->lock. > > Changelog from v2: > Use nr-queued[] of struct throtl_grp for stats instaed of adding new > blkg_rwstat. > > Cc: Tejun Heo > Cc: Vivek Goyal > Cc: Jens Axboe > Signed-off-by: Tao Ma > Signed-off-by: Robin Dong > --- > block/blk-throttle.c | 40 > 1 files changed, 40 insertions(+), 0 deletions(-) > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > index a9664fa..e410448 100644 > --- a/block/blk-throttle.c > +++ b/block/blk-throttle.c > @@ -953,6 +953,32 @@ static u64 tg_prfill_cpu_rwstat(struct seq_file *sf, > return __blkg_prfill_rwstat(sf, pd, ); > } > > +static u64 tg_prfill_io_queued(struct seq_file *sf, > + struct blkg_policy_data *pd, int off) > +{ > + static const char *rwstr[] = { > + [READ] = "Read", > + [WRITE] = "Write", > + }; > + struct throtl_grp *tg = pd_to_tg(pd); > + const char *dname = NULL; > + unsigned int v; > + int i; > + > + if (pd->blkg->q->backing_dev_info.dev) > + dname = dev_name(pd->blkg->q->backing_dev_info.dev); > + > + if (!dname) > + return 0; > + > + for (i = 0; i <= WRITE; i++) > + seq_printf(sf, "%s %s %u\n", dname, rwstr[i], tg->nr_queued[i]); You are printing only READ/WRITE stats and not the SYNC/ASYNC stats. This is not inline with rest of the stats like throttle.io_serviced and throttle.io_service_bytes. I guess it is better to maintain rwstat for io_queued and display it according to io_serviced. Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
On Tue, Oct 09, 2012 at 02:53:45PM +0800, Robin Dong wrote: From: Robin Dong san...@taobao.com Currently, if the IO is throttled by io-throttle, the SA 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_queued which indicates how many IOs are currently throttled. The nr_queued[] of struct throtl_grp is of type unsigned int and updates to it are atomic both at 32bit and 64bit platforms, so we could just read tg-nr_queued only under blkcg-lock. Changelog from v2: Use nr-queued[] of struct throtl_grp for stats instaed of adding new blkg_rwstat. Cc: Tejun Heo t...@kernel.org Cc: Vivek Goyal vgo...@redhat.com Cc: Jens Axboe ax...@kernel.dk Signed-off-by: Tao Ma boyu...@taobao.com Signed-off-by: Robin Dong san...@taobao.com --- block/blk-throttle.c | 40 1 files changed, 40 insertions(+), 0 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index a9664fa..e410448 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -953,6 +953,32 @@ static u64 tg_prfill_cpu_rwstat(struct seq_file *sf, return __blkg_prfill_rwstat(sf, pd, rwstat); } +static u64 tg_prfill_io_queued(struct seq_file *sf, + struct blkg_policy_data *pd, int off) +{ + static const char *rwstr[] = { + [READ] = Read, + [WRITE] = Write, + }; + struct throtl_grp *tg = pd_to_tg(pd); + const char *dname = NULL; + unsigned int v; + int i; + + if (pd-blkg-q-backing_dev_info.dev) + dname = dev_name(pd-blkg-q-backing_dev_info.dev); + + if (!dname) + return 0; + + for (i = 0; i = WRITE; i++) + seq_printf(sf, %s %s %u\n, dname, rwstr[i], tg-nr_queued[i]); You are printing only READ/WRITE stats and not the SYNC/ASYNC stats. This is not inline with rest of the stats like throttle.io_serviced and throttle.io_service_bytes. I guess it is better to maintain rwstat for io_queued and display it according to io_serviced. Thanks Vivek -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
From: Robin Dong Currently, if the IO is throttled by io-throttle, the SA 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_queued which indicates how many IOs are currently throttled. The nr_queued[] of struct throtl_grp is of type "unsigned int" and updates to it are atomic both at 32bit and 64bit platforms, so we could just read tg->nr_queued only under blkcg->lock. Changelog from v2: Use nr-queued[] of struct throtl_grp for stats instaed of adding new blkg_rwstat. Cc: Tejun Heo Cc: Vivek Goyal Cc: Jens Axboe Signed-off-by: Tao Ma Signed-off-by: Robin Dong --- block/blk-throttle.c | 40 1 files changed, 40 insertions(+), 0 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index a9664fa..e410448 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -953,6 +953,32 @@ static u64 tg_prfill_cpu_rwstat(struct seq_file *sf, return __blkg_prfill_rwstat(sf, pd, ); } +static u64 tg_prfill_io_queued(struct seq_file *sf, + struct blkg_policy_data *pd, int off) +{ + static const char *rwstr[] = { + [READ] = "Read", + [WRITE] = "Write", + }; + struct throtl_grp *tg = pd_to_tg(pd); + const char *dname = NULL; + unsigned int v; + int i; + + if (pd->blkg->q->backing_dev_info.dev) + dname = dev_name(pd->blkg->q->backing_dev_info.dev); + + if (!dname) + return 0; + + for (i = 0; i <= WRITE; i++) + seq_printf(sf, "%s %s %u\n", dname, rwstr[i], tg->nr_queued[i]); + + v = tg->nr_queued[READ] + tg->nr_queued[WRITE]; + seq_printf(sf, "%s Total %u\n", dname, v); + return v; +} + static int tg_print_cpu_rwstat(struct cgroup *cgrp, struct cftype *cft, struct seq_file *sf) { @@ -963,6 +989,16 @@ static int tg_print_cpu_rwstat(struct cgroup *cgrp, struct cftype *cft, return 0; } +static int tg_print_io_queued(struct cgroup *cgrp, struct cftype *cft, + struct seq_file *sf) +{ + struct blkcg *blkcg = cgroup_to_blkcg(cgrp); + + blkcg_print_blkgs(sf, blkcg, tg_prfill_io_queued, _policy_throtl, + cft->private, true); + return 0; +} + static u64 tg_prfill_conf_u64(struct seq_file *sf, struct blkg_policy_data *pd, int off) { @@ -1085,6 +1121,10 @@ static struct cftype throtl_files[] = { .private = offsetof(struct tg_stats_cpu, serviced), .read_seq_string = tg_print_cpu_rwstat, }, + { + .name = "throttle.io_queued", + .read_seq_string = tg_print_io_queued, + }, { } /* terminate */ }; -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V3] block/throttle: Add IO throttled information in blkio.throttle
From: Robin Dong san...@taobao.com Currently, if the IO is throttled by io-throttle, the SA 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_queued which indicates how many IOs are currently throttled. The nr_queued[] of struct throtl_grp is of type unsigned int and updates to it are atomic both at 32bit and 64bit platforms, so we could just read tg-nr_queued only under blkcg-lock. Changelog from v2: Use nr-queued[] of struct throtl_grp for stats instaed of adding new blkg_rwstat. Cc: Tejun Heo t...@kernel.org Cc: Vivek Goyal vgo...@redhat.com Cc: Jens Axboe ax...@kernel.dk Signed-off-by: Tao Ma boyu...@taobao.com Signed-off-by: Robin Dong san...@taobao.com --- block/blk-throttle.c | 40 1 files changed, 40 insertions(+), 0 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index a9664fa..e410448 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -953,6 +953,32 @@ static u64 tg_prfill_cpu_rwstat(struct seq_file *sf, return __blkg_prfill_rwstat(sf, pd, rwstat); } +static u64 tg_prfill_io_queued(struct seq_file *sf, + struct blkg_policy_data *pd, int off) +{ + static const char *rwstr[] = { + [READ] = Read, + [WRITE] = Write, + }; + struct throtl_grp *tg = pd_to_tg(pd); + const char *dname = NULL; + unsigned int v; + int i; + + if (pd-blkg-q-backing_dev_info.dev) + dname = dev_name(pd-blkg-q-backing_dev_info.dev); + + if (!dname) + return 0; + + for (i = 0; i = WRITE; i++) + seq_printf(sf, %s %s %u\n, dname, rwstr[i], tg-nr_queued[i]); + + v = tg-nr_queued[READ] + tg-nr_queued[WRITE]; + seq_printf(sf, %s Total %u\n, dname, v); + return v; +} + static int tg_print_cpu_rwstat(struct cgroup *cgrp, struct cftype *cft, struct seq_file *sf) { @@ -963,6 +989,16 @@ static int tg_print_cpu_rwstat(struct cgroup *cgrp, struct cftype *cft, return 0; } +static int tg_print_io_queued(struct cgroup *cgrp, struct cftype *cft, + struct seq_file *sf) +{ + struct blkcg *blkcg = cgroup_to_blkcg(cgrp); + + blkcg_print_blkgs(sf, blkcg, tg_prfill_io_queued, blkcg_policy_throtl, + cft-private, true); + return 0; +} + static u64 tg_prfill_conf_u64(struct seq_file *sf, struct blkg_policy_data *pd, int off) { @@ -1085,6 +1121,10 @@ static struct cftype throtl_files[] = { .private = offsetof(struct tg_stats_cpu, serviced), .read_seq_string = tg_print_cpu_rwstat, }, + { + .name = throttle.io_queued, + .read_seq_string = tg_print_io_queued, + }, { } /* terminate */ }; -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/