Messages in this thread | | | Date | Sat, 01 Sep 2012 21:58:43 +0800 | From | Tao Ma <> | Subject | Re: [PATCH V2] block/throttle: Add IO throttled information in blkio.throttle. |
| |
Hi Tejun, On 09/01/2012 09:05 AM, Tejun Heo wrote: > On Fri, Aug 31, 2012 at 01:15:09PM +0800, Tao Ma wrote: >> From: Tao Ma <boyu.mt@taobao.com> >> >> Currently, if the IO is throttled by io-throttle, the SA has no idea of > > What's SA? system admin. > >> 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 > > Why does the application user "has to" do something? There's nothing > the upper layer "must" do. I'm not necessarily objecting to adding > the stat but the description seems a bit misleading. > >> named blkio.throttle.io_queued which indicates how many IOs are >> currently throttled. > > Also, the suggested stat is rather lacking for such purposes. There's > no way other than keeping polling to find out the condition, which is > rather sad. What's the actual use case here? Vivek and I have talked about its usage in my first try. See the thread here. https://lkml.org/lkml/2012/5/22/81 And I am OK to say it again here. In our case, we use flashcache as a block device and the bad thing is that flashcache is a bio-based dm target and we can't use block io controller here to control the weight of different cgroups. So io throttle is chosen. But as io throttle can only set a hard upper limit for different instances, it makes the control not flexible enough. Say with io controller, if there is no requests form the cgroup with weight 1000, a cgroup with 500 can use the whole bandwidth of the underlying device. But if we set 1000 iops for cgroup A and 500 iops for cgroup B in io throttle, cgroup B can't exceed its limit even if cgroup A has no request pending. So if we can export the io_queued information out to the system admin, they can write some daemon and in the above case, increase the upper limit of cgroup B to some number say 1000. It helps us to utilize the device more efficiently. Does it make sense to you?
> >> Also another function blkg_rwstat_dec is added since the number of throttled >> IOs can be either added or decreased. > > Maybe just make blkg_rwstat_add() to take int64_t instead of uint64_t? sure, will change it in the later version. > >> +static void throtl_update_queued_stats(struct throtl_grp *tg, int rw, int add) >> +{ >> + 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); >> + if (add) >> + blkg_rwstat_add(&stats_cpu->io_queued, rw, 1); >> + else >> + blkg_rwstat_dec(&stats_cpu->io_queued, rw, 1); >> + >> + local_irq_restore(flags); > > Adding throttle.io_queued could be a bit more consistent? sorry, I don't know what is your meaning here. You mean some codes like blkg_rwstat_add(&stats_cpu->throttle.io_queude, rw, 1)?
Thanks Tao
| |