OpenVZ Forum - RDF feed
type: channel
items: [PATCH 2/3] i/o bandwidth controller infrastructure
items: Re: [PATCH 2/3] i/o bandwidth controller infrastructure

[PATCH 2/3] i/o bandwidth controller infrastructure
type: item
description: This is the core io-throttle kernel infrastructure. It creates the basic interfaces to cgroups and implements the I/O measurement and throttling functions. Signed-off-by: Andrea Righi <righi.andrea@gmail.com> --- block/Makefile | 2 + block/blk-io-throttle.c | 393 +++++++++++++++++++++++++++++++++++++++ include/linux/blk-io-throttle.h | 12 ++ include/linux/cgroup_subsys.h | 6 + init/Kconfig | 10 + 5 files changed, 423 insertions(+), 0 deletions(-) create mode 100644 block/blk-io-throttle.c create mode 100644 include/linux/blk-io-throttle.h diff --git a/block/Makefile b/block/Makefile index 5a43c7d..8dec69b 100644 --- a/block/Makefile +++ b/block/Makefile @@ -14,3 +14,5 @@ obj-$(CONFIG_IOSCHED_CFQ) += cfq-iosched.o obj-$(CONFIG_BLK_DEV_IO_TRACE) += blktrace.o obj-$(CONFIG_BLOCK_COMPAT) += compat_ioctl.o + +obj-$(CONFIG_CGROUP_IO_THROTTLE) += blk-io-throttle.o diff --git a/block/blk-io-throttle.c b/block/blk-io-throttle.c new file mode 100644 index 0000000..4ec02bb --- /dev/null +++ b/block/blk-io-throttle.c @@ -0,0 +1,393 @@ +/* + * blk-io-throttle.c + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + * + * Copyright (C) 2008 Andrea Righi <righi.andrea@gmail.com> + */ + +#include <linux/init.h> +#include <linux/module.h> +#include <linux/cgroup.h> +#include <linux/slab.h> +#include <linux/gfp.h> …
creator: Andrea Righi
date: 2008-06-20T10:05:34-00:00

Re: [PATCH 2/3] i/o bandwidth controller infrastructure
type: item
description: Carl Henrik Lunde wrote: > Did you consider using token bucket instead of this (leaky bucket?)? > > I've attached a patch which implements token bucket. Although not as > precise as the leaky bucket the performance is better at high bandwidth > streaming loads. > > The leaky bucket stops at around 53 MB/s while token bucket works for > up to 64 MB/s. The baseline (no cgroups) is 66 MB/s. > > benchmark: > two streaming readers (fio) with block size 128k, bucket size 4 MB > 90% of the bandwidth was allocated to one process, the other gets 10% > > bw-limit: actual bw algorithm bw1 bw2 > 5 MiB/s: 5.0 MiB/s leaky_bucket 0.5 4.5 > 5 MiB/s: 5.2 MiB/s token_bucket 0.6 4.6 > 10 MiB/s: 10.0 MiB/s leaky_bucket 1.0 9.0 > 10 MiB/s: 10.3 MiB/s token_bucket 1.0 9.2 > 15 MiB/s: 15.0 MiB/s leaky_bucket 1.5 13.5 > 15 MiB/s: 15.4 MiB/s token_bucket 1.5 13.8 > 20 MiB/s: 19.9 MiB/s leaky_bucket 2.0 17.9 > 20 MiB/s: 20.5 MiB/s token_bucket 2.1 18.4 > 25 MiB/s: 24.4 MiB/s leaky_bucket 2.5 21.9 > 25 MiB/s: 25.6 MiB/s token_bucket 2.6 23.0 > 30 MiB/s: 29.2 MiB/s leaky_bucket 3.0 26.2 > 30 MiB/s: 30.7 MiB/s token_bucket 3.1 27.7 > 35 MiB/s: 34.3 MiB/s leaky_bucket 3.4 30.9 > 35 MiB/s: 35.9 MiB/s token_bucket 3.6 32.3 > 40 MiB/s: 39.7 MiB/s leaky_bucket 3.9 35.8 > 40 MiB/s: 41.0 MiB/s token_bucket 4.1 36.9 > 45 MiB/s: 44.0 MiB/s leaky_bucket 4.3 39.7 > 45 MiB/s: 46.1 MiB/s token_bucket 4.6 41.5 > 50 MiB/s: 47.9 MiB/s leaky_bucket 4.7 43.2 > 50 MiB/s: 51.0 MiB/s token_bucket 5.1 45.9 > 55 MiB/s: 50.5 MiB/s leaky_bucket 5.0 45.5 > 55 MiB/s: 56.2 MiB/s token_bucket 5.6 50.5 > 60 MiB/s: 52.9 MiB/s leaky_bucket 5.2 47.7 > 60 MiB/s: 61.0 MiB/s token_bucket 6.1 54.9 > 65 MiB/s: 53.0 MiB/s leaky_bucket 5.4 47.6 > 65 MiB/s: 63.7 MiB/s token_bucket 6.6 57.1 > 70 MiB/s: 53.8 MiB/s leaky_bucket 5.5 48.4 > 70 MiB/s: 64.1 MiB/s token_bucket 7.1 57.0 Carl, based on your token bucket solution I've implemented a run-time leaky bucket / token bucket switcher: # leaky bucket # echo 0 > /cgroups/foo/blockio.thro…
creator: Andrea Righi
date: 2008-06-22T13:11:13-00:00

Re: [PATCH 2/3] i/o bandwidth controller infrastructure
type: item
description: On Fri, 20 Jun 2008 12:05:34 +0200 Andrea Righi <righi.andrea@gmail.com> wrote: > This is the core io-throttle kernel infrastructure. It creates the basic > interfaces to cgroups and implements the I/O measurement and throttling > functions. > > Signed-off-by: Andrea Righi <righi.andrea@gmail.com> > --- > block/Makefile | 2 + > block/blk-io-throttle.c | 393 +++++++++++++++++++++++++++++++++++++++ > include/linux/blk-io-throttle.h | 12 ++ > include/linux/cgroup_subsys.h | 6 + > init/Kconfig | 10 + > 5 files changed, 423 insertions(+), 0 deletions(-) > create mode 100644 block/blk-io-throttle.c > create mode 100644 include/linux/blk-io-throttle.h > > diff --git a/block/Makefile b/block/Makefile > index 5a43c7d..8dec69b 100644 > --- a/block/Makefile > +++ b/block/Makefile > @@ -14,3 +14,5 @@ obj-$(CONFIG_IOSCHED_CFQ) += cfq-iosched.o > > obj-$(CONFIG_BLK_DEV_IO_TRACE) += blktrace.o > obj-$(CONFIG_BLOCK_COMPAT) += compat_ioctl.o > + > +obj-$(CONFIG_CGROUP_IO_THROTTLE) += blk-io-throttle.o > diff --git a/block/blk-io-throttle.c b/block/blk-io-throttle.c > new file mode 100644 > index 0000000..4ec02bb > --- /dev/null > +++ b/block/blk-io-throttle.c > @@ -0,0 +1,393 @@ > +/* > + * blk-io-throttle.c > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public > + * License along with this program; if not, write to the > + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, > + * Boston, MA 021110-1307, USA. > + * > + * Copyright (C…
creator: akpm
date: 2008-06-26T00:29:00-00:00

Re: [PATCH 2/3] i/o bandwidth controller infrastructure
type: item
description: Andrew, thanks for your time and the detailed revision. I'll try to fix everything you reported and better document the code according to your suggestions. I'll re-submit a new patchset version ASAP. A few comments below. Andrew Morton wrote: [snip] >> +static ssize_t iothrottle_read(struct cgroup *cont, >> + struct cftype *cft, >> + struct file *file, >> + char __user *userbuf, >> + size_t nbytes, >> + loff_t *ppos) >> +{ >> + struct iothrottle *iot; >> + char *buffer; >> + int s = 0; >> + struct iothrottle_node *n; >> + ssize_t ret; >> + >> + buffer = kmalloc(nbytes + 1, GFP_KERNEL); >> + if (!buffer) >> + return -ENOMEM; >> + >> + cgroup_lock(); >> + if (cgroup_is_removed(cont)) { >> + ret = -ENODEV; >> + goto out; >> + } >> + >> + iot = cgroup_to_iothrottle(cont); >> + rcu_read_lock(); >> + list_for_each_entry_rcu(n, &iot->list, node) { >> + unsigned long delta, rate; >> + >> + BUG_ON(!n->dev); >> + delta = jiffies_to_usecs((long)jiffies - (long)n->timestamp); >> + rate = delta ? KBS(atomic_long_read(&n->stat) / delta) : 0; >> + s += scnprintf(buffer + s, nbytes - s, >> + "=== device (%u,%u) ===\n" >> + " bandwidth limit: %lu KiB/sec\n" >> + "current i/o usage: %lu KiB/sec\n", >> + MAJOR(n->dev), MINOR(n->dev), >> + n->iorate, rate); >> + } >> + rcu_read_unlock(); >> + ret = simple_read_from_buffer(userbuf, nbytes, ppos, buffer, s); >> +out: >> + cgroup_unlock(); >> + kfree(buffer); >> + return ret; >> +} > > This is a kernel->userspace interface. It is part of the kernel ABI. > We will need to support in a back-compatible fashion for ever. Hence > it is important. The entire proposed kernel<->userspace interface > should be completely described in the changelog or the documentation so > that we can understand and review what you are proposing. and BTW I was actually wondering if the output could be changed with something less human readable and better parseable, I means just print only raw numbers. And describe the semantic in the documentation. >> +static inli…
creator: Andrea Righi
date: 2008-06-26T22:36:46-00:00

Re: [PATCH 2/3] i/o bandwidth controller infrastructure
type: item
description: On Fri, 27 Jun 2008 00:36:46 +0200 Andrea Righi <righi.andrea@gmail.com> wrote: > > Does all this code treat /dev/sda1 as a separate device from /dev/sda2? > > If so, that would be broken. > > Yes, all the partitions are treated as separate devices with > (potentially) different limiting rules, but I don't understand why it > would be broken... dev_t has both minor and major numbers, so it would > be possible to select single partitions as well. Well it's functionally broken, isn't it? A physical disk has a fixed IO bandwidth and when the administrator wants to partition that bandwidth amongst control groups he will need to consider the entire device when doing so? I mean, the whole point of this feature and of control groups as a whole is isolation. But /dev/sda1 and /dev/sda2 are very much _not_ isolated. Whereas /dev/sda and /dev/sdb are (to a large degree) isolated. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: akpm
date: 2008-06-26T22:59:48-00:00

Re: [PATCH 2/3] i/o bandwidth controller infrastructure
type: item
description: Andrew Morton wrote: > On Fri, 27 Jun 2008 00:36:46 +0200 > Andrea Righi <righi.andrea@gmail.com> wrote: > >>> Does all this code treat /dev/sda1 as a separate device from /dev/sda2? >>> If so, that would be broken. >> Yes, all the partitions are treated as separate devices with >> (potentially) different limiting rules, but I don't understand why it >> would be broken... dev_t has both minor and major numbers, so it would >> be possible to select single partitions as well. > > Well it's functionally broken, isn't it? A physical disk has a fixed > IO bandwidth and when the administrator wants to partition that > bandwidth amongst control groups he will need to consider the entire > device when doing so? > > I mean, the whole point of this feature and of control groups as a > whole is isolation. But /dev/sda1 and /dev/sda2 are very much _not_ > isolated. Whereas /dev/sda and /dev/sdb are (to a large degree) > isolated. well... yes, sounds reasonable. In this case we could just ignore the minor number and consider only major number as the key to identify a specific block device (both for userspace<->kernel interface and when accounting/throttling i/o requests). -Andrea _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Andrea Righi
date: 2008-06-27T10:53:28-00:00

Re: [PATCH 2/3] i/o bandwidth controller infrastructure
type: item
description: Andrea Righi wrote: > Andrew Morton wrote: >> On Fri, 27 Jun 2008 00:36:46 +0200 >> Andrea Righi <righi.andrea@gmail.com> wrote: >> >>>> Does all this code treat /dev/sda1 as a separate device from /dev/sda2? >>>> If so, that would be broken. >>> Yes, all the partitions are treated as separate devices with >>> (potentially) different limiting rules, but I don't understand why it >>> would be broken... dev_t has both minor and major numbers, so it would >>> be possible to select single partitions as well. >> Well it's functionally broken, isn't it? A physical disk has a fixed >> IO bandwidth and when the administrator wants to partition that >> bandwidth amongst control groups he will need to consider the entire >> device when doing so? >> >> I mean, the whole point of this feature and of control groups as a >> whole is isolation. But /dev/sda1 and /dev/sda2 are very much _not_ >> isolated. Whereas /dev/sda and /dev/sdb are (to a large degree) >> isolated. > > well... yes, sounds reasonable. In this case we could just ignore the > minor number and consider only major number as the key to identify a > specific block device (both for userspace<->kernel interface and when > accounting/throttling i/o requests). oops.. no, this is obviously wrong. So, I dunno if it would be better to add complexity in cgroup_io_throttle() to identify the disk a partition belongs or to just use the struct block_device as key, instead of dev_t, as you intially suggested. I'll investigate. -Andrea _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Andrea Righi
date: 2008-06-30T16:10:54-00:00