OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/5] blk-throttle: writeback and swap IO control
items: [PATCH 1/5] blk-cgroup: move blk-cgroup.h in include/linux/blk-cgroup.h
items: [PATCH 2/5] blk-cgroup: introduce task_to_blkio_cgroup()
items: [PATCH 3/5] page_cgroup: make page tracking available for blkio
items: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
items: [PATCH 5/5] blk-throttle: buffered and anonymous page tracking instrumentation
items: Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
items: Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
items: Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio

[PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: Currently the blkio.throttle controller only support synchronous IO requests. This means that we always look at the current task to identify the "owner" of each IO request. However dirty pages in the page cache can be wrote to disk asynchronously by the per-bdi flusher kernel threads or by any other thread in the system, according to the writeback policy. For this reason the real writes to the underlying block devices may occur in a different IO context respect to the task that originally generated the dirty pages involved in the IO operation. This makes the tracking and throttling of writeback IO more complicate respect to the synchronous IO from the blkio controller's perspective. The same concept is also valid for anonymous pages involed in IO operations (swap). This patch allow to track the cgroup that originally dirtied each page in page cache and each anonymous page and pass these informations to the blk-throttle controller. These informations can be used to provide a better service level differentiation of buffered writes swap IO between different cgroups. Testcase ======== - create a cgroup with 1MiB/s write limit: # mount -t cgroup -o blkio none /mnt/cgroup # mkdir /mnt/cgroup/foo # echo 8:0 $((1024 * 1024)) > /mnt/cgroup/foo/blkio.throttle.write_bps_device - move a task into the cgroup and run a dd to generate some writeback IO Results: - 2.6.38-rc6 vanilla: $ cat /proc/$$/cgroup 1:blkio:/foo $ dd if=/dev/zero of=zero bs=1M count=1024 & $ dstat -df --dsk/sda-- read writ 0 19M 0 19M 0 0 0 0 0 19M ... - 2.6.38-rc6 + blk-throttle writeback IO control: $ cat /proc/$$/cgroup 1:blkio:/foo $ dd if=/dev/zero of=zero bs=1M count=1024 & $ dstat -df --dsk/sda-- read writ 0 1024 0 1024 0 1024 0 1024 0 1024 ... TODO ==== - lots of testing Any feedback is welcome. -Andrea [PATCH 1/5] blk-cgroup: move blk-cgroup.h in include/linux/blk-cgroup.h [PATCH 2/5] blk-cgroup: introduce task_to_blkio_cgroup() [PATCH 3/5] page_cgroup: make page tracking available for blkio [PATCH 4…
creator: Andrea Righi
date: 2011-02-22T17:12:51-00:00

[PATCH 1/5] blk-cgroup: move blk-cgroup.h in include/linux/blk-cgroup.h
type: item
description: Move blk-cgroup.h in include/linux for generic usage. Signed-off-by: Andrea Righi <arighi@develer.com> --- block/blk-cgroup.c | 2 +- block/blk-cgroup.h | 335 ------------------------------------------- block/blk-throttle.c | 2 +- block/cfq.h | 2 +- include/linux/blk-cgroup.h | 337 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 340 insertions(+), 338 deletions(-) delete mode 100644 block/blk-cgroup.h create mode 100644 include/linux/blk-cgroup.h diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 455768a..bf9d354 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -17,7 +17,7 @@ #include <linux/err.h> #include <linux/blkdev.h> #include <linux/slab.h> -#include "blk-cgroup.h" +#include <linux/blk-cgroup.h> #include <linux/genhd.h> #define MAX_KEY_LEN 100 diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h deleted file mode 100644 index ea4861b..0000000 --- a/block/blk-cgroup.h +++ /dev/null @@ -1,335 +0,0 @@ -#ifndef _BLK_CGROUP_H -#define _BLK_CGROUP_H -/* - * Common Block IO controller cgroup interface - * - * Based on ideas and code from CFQ, CFS and BFQ: - * Copyright (C) 2003 Jens Axboe <axboe@kernel.dk> - * - * Copyright (C) 2008 Fabio Checconi <fabio@gandalf.sssup.it> - * Paolo Valente <paolo.valente@unimore.it> - * - * Copyright (C) 2009 Vivek Goyal <vgoyal@redhat.com> - * Nauman Rafique <nauman@google.com> - */ - -#include <linux/cgroup.h> - -enum blkio_policy_id { - BLKIO_POLICY_PROP = 0, /* Proportional Bandwidth division */ - BLKIO_POLICY_THROTL, /* Throttling */ -}; - -/* Max limits for throttle policy */ -#define THROTL_IOPS_MAX UINT_MAX - -#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE) - -#ifndef CONFIG_BLK_CGROUP -/* When blk-cgroup is a module, its subsys_id isn't a compile-time constant */ -extern struct cgroup_subsys blkio_subsys; -#define blkio_subsys_id blkio_subsys.subsys_id -#endif - -enum stat_type { - /* Total time spent (in ns) between request dispatch to the driver and - * request completio…
creator: Andrea Righi
date: 2011-02-22T17:12:52-00:00

[PATCH 2/5] blk-cgroup: introduce task_to_blkio_cgroup()
type: item
description: Introduce a helper function to retrieve a blkio cgroup from a task. Signed-off-by: Andrea Righi <arighi@develer.com> --- block/blk-cgroup.c | 7 +++++++ include/linux/blk-cgroup.h | 4 ++++ 2 files changed, 11 insertions(+), 0 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index bf9d354..f283ae1 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -107,6 +107,13 @@ blkio_policy_search_node(const struct blkio_cgroup *blkcg, dev_t dev, return NULL; } +struct blkio_cgroup *task_to_blkio_cgroup(struct task_struct *task) +{ + return container_of(task_subsys_state(task, blkio_subsys_id), + struct blkio_cgroup, css); +} +EXPORT_SYMBOL_GPL(task_to_blkio_cgroup); + struct blkio_cgroup *cgroup_to_blkio_cgroup(struct cgroup *cgroup) { return container_of(cgroup_subsys_state(cgroup, blkio_subsys_id), diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h index 5e48204..41b59db 100644 --- a/include/linux/blk-cgroup.h +++ b/include/linux/blk-cgroup.h @@ -287,6 +287,7 @@ static inline void blkiocg_set_start_empty_time(struct blkio_group *blkg) {} extern struct blkio_cgroup blkio_root_cgroup; extern bool blkio_cgroup_disabled(void); extern struct blkio_cgroup *cgroup_to_blkio_cgroup(struct cgroup *cgroup); +extern struct blkio_cgroup *task_to_blkio_cgroup(struct task_struct *task); extern void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, struct blkio_group *blkg, void *key, dev_t dev, enum blkio_policy_id plid); @@ -311,6 +312,9 @@ static inline bool blkio_cgroup_disabled(void) { return true; } static inline struct blkio_cgroup * cgroup_to_blkio_cgroup(struct cgroup *cgroup) { return NULL; } +static inline struct blkio_cgroup * +task_to_blkio_cgroup(struct task_struct *task) { return NULL; } + static inline void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, struct blkio_group *blkg, void *key, dev_t dev, enum blkio_policy_id plid) {} -- 1.7.1 _______________________________________________ Containers mailing list Containers@lists.l…
creator: Andrea Righi
date: 2011-02-22T17:12:53-00:00

[PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: The page_cgroup infrastructure, currently available only for the memory cgroup controller, can be used to store the owner of each page and opportunely track the writeback IO. This information is encoded in the upper 16-bits of the page_cgroup->flags. A owner can be identified using a generic ID number and the following interfaces are provided to store a retrieve this information: unsigned long page_cgroup_get_owner(struct page *page); int page_cgroup_set_owner(struct page *page, unsigned long id); int page_cgroup_copy_owner(struct page *npage, struct page *opage); The blkio.throttle controller can use the cgroup css_id() as the owner's ID number. Signed-off-by: Andrea Righi <arighi@develer.com> --- block/Kconfig | 2 + block/blk-cgroup.c | 6 ++ include/linux/memcontrol.h | 6 ++ include/linux/mmzone.h | 4 +- include/linux/page_cgroup.h | 33 ++++++++++- init/Kconfig | 4 + mm/Makefile | 3 +- mm/memcontrol.c | 6 ++ mm/page_cgroup.c | 129 +++++++++++++++++++++++++++++++++++++++---- 9 files changed, 176 insertions(+), 17 deletions(-) diff --git a/block/Kconfig b/block/Kconfig index 60be1e0..1351ea8 100644 --- a/block/Kconfig +++ b/block/Kconfig @@ -80,6 +80,8 @@ config BLK_DEV_INTEGRITY config BLK_DEV_THROTTLING bool "Block layer bio throttling support" depends on BLK_CGROUP=y && EXPERIMENTAL + select MM_OWNER + select PAGE_TRACKING default n ---help--- Block layer bio throttling support. It can be used to limit diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index f283ae1..5c57f0a 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -107,6 +107,12 @@ blkio_policy_search_node(const struct blkio_cgroup *blkcg, dev_t dev, return NULL; } +bool blkio_cgroup_disabled(void) +{ + return blkio_subsys.disabled ? true : false; +} +EXPORT_SYMBOL_GPL(blkio_cgroup_disabled); + struct blkio_cgroup *task_to_blkio_cgroup(struct task_struct *task) { return container_of(task_subsys_state(task, blkio_subsys_id), diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h…
creator: Andrea Righi
date: 2011-02-22T17:12:54-00:00

[PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: Add the tracking of buffered (writeback) and anonymous pages. Dirty pages in the page cache can be processed asynchronously by the per-bdi flusher kernel threads or by any other thread in the system, according to the writeback policy. For this reason the real writes to the underlying block devices may occur in a different IO context respect to the task that originally generated the dirty pages involved in the IO operation. This makes the tracking and throttling of writeback IO more complicate respect to the synchronous IO from the blkio controller's point of view. The idea is to save the cgroup owner of each anonymous page and dirty page in page cache. A page is associated to a cgroup the first time it is dirtied in memory (for file cache pages) or when it is set as swap-backed (for anonymous pages). This information is stored using the page_cgroup functionality. Then, at the block layer, it is possible to retrieve the throttle group looking at the bio_page(bio). If the page was not explicitly associated to any cgroup the IO operation is charged to the current task/cgroup, as it was done by the previous implementation. Signed-off-by: Andrea Righi <arighi@develer.com> --- block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/blkdev.h | 26 ++++++++++++++- 2 files changed, 111 insertions(+), 2 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 9ad3d1e..a50ee04 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -8,6 +8,10 @@ #include <linux/slab.h> #include <linux/blkdev.h> #include <linux/bio.h> +#include <linux/memcontrol.h> +#include <linux/mm_inline.h> +#include <linux/pagemap.h> +#include <linux/page_cgroup.h> #include <linux/blktrace_api.h> #include <linux/blk-cgroup.h> @@ -221,6 +225,85 @@ done: return tg; } +static inline bool is_kernel_io(void) +{ + return !!(current->flags & (PF_KTHREAD | PF_KSWAPD | PF_MEMALLOC)); +} + +static int throtl_set_page_owner(struct page *page, struct mm_struct…
creator: Andrea Righi
date: 2011-02-22T17:12:55-00:00

[PATCH 5/5] blk-throttle: buffered and anonymous page tracking instrumentation
type: item
description: Apply the buffered and anonymous page tracking hooks to the opportune kernel functions. Signed-off-by: Ryo Tsuruta <ryov@valinux.co.jp> Signed-off-by: Hirokazu Takahashi <taka@valinux.co.jp> Signed-off-by: Andrea Righi <arighi@develer.com> --- fs/buffer.c | 1 + mm/bounce.c | 1 + mm/filemap.c | 1 + mm/memory.c | 5 +++++ mm/page-writeback.c | 1 + mm/swap_state.c | 2 ++ 6 files changed, 11 insertions(+), 0 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index 2219a76..6e473e6 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -667,6 +667,7 @@ static void __set_page_dirty(struct page *page, if (page->mapping) { /* Race with truncate? */ WARN_ON_ONCE(warn && !PageUptodate(page)); account_page_dirtied(page, mapping); + blk_throtl_set_filepage_owner(page, current->mm); radix_tree_tag_set(&mapping->page_tree, page_index(page), PAGECACHE_TAG_DIRTY); } diff --git a/mm/bounce.c b/mm/bounce.c index 1481de6..f85cafa 100644 --- a/mm/bounce.c +++ b/mm/bounce.c @@ -211,6 +211,7 @@ static void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig, to->bv_len = from->bv_len; to->bv_offset = from->bv_offset; inc_zone_page_state(to->bv_page, NR_BOUNCE); + blk_throtl_copy_page_owner(to->bv_page, page); if (rw == WRITE) { char *vto, *vfrom; diff --git a/mm/filemap.c b/mm/filemap.c index 83a45d3..7fca2b8 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -407,6 +407,7 @@ int add_to_page_cache_locked(struct page *page, struct address_space *mapping, gfp_mask & GFP_RECLAIM_MASK); if (error) goto out; + blk_throtl_set_filepage_owner(page, current->mm); error = radix_tree_preload(gfp_mask & ~__GFP_HIGHMEM); if (error == 0) { diff --git a/mm/memory.c b/mm/memory.c index 8e8c183..ad5906b 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -52,6 +52,7 @@ #include <linux/init.h> #include <linux/writeback.h> #include <linux/memcontrol.h> +#include <linux/blkdev.h> #include <linux/mmu_notifier.h> #include <linux/kallsyms.h> #include <linux/swapops.h> @@ -2391,6 +2392,7 @@ gotten: */ ptep_clear_fl…
creator: Andrea Righi
date: 2011-02-22T17:12:56-00:00

Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: On Tue, Feb 22, 2011 at 9:12 AM, Andrea Righi <arighi@develer.com> wrote: > Add the tracking of buffered (writeback) and anonymous pages. ... > --- > block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- > include/linux/blkdev.h | 26 ++++++++++++++- > 2 files changed, 111 insertions(+), 2 deletions(-) > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > index 9ad3d1e..a50ee04 100644 > --- a/block/blk-throttle.c > +++ b/block/blk-throttle.c ... > +int blk_throtl_set_anonpage_owner(struct page *page, struct mm_struct *mm) > +int blk_throtl_set_filepage_owner(struct page *page, struct mm_struct *mm) > +int blk_throtl_copy_page_owner(struct page *npage, struct page *opage) It would be nice if these were named blk_cgroup_*. This is arguably more correct as the id comes from the blkio subsystem, and isn't specific to blk-throttle. This will be more important very shortly, as CFQ will be using this same cgroup id for async IO tracking soon. is_kernel_io() is a good idea, it avoids a bug that we've run into with CFQ async IO tracking. Why isn't PF_KTHREAD sufficient to cover all kernel threads, including kswapd and those marked PF_MEMALLOC? Thanks, Chad _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Chad Talbott
date: 2011-02-22T18:42:41-00:00

Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: On Tue, Feb 22, 2011 at 10:42:41AM -0800, Chad Talbott wrote: > On Tue, Feb 22, 2011 at 9:12 AM, Andrea Righi <arighi@develer.com> wrote: > > Add the tracking of buffered (writeback) and anonymous pages. > ... > > --- > > block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- > > include/linux/blkdev.h | 26 ++++++++++++++- > > 2 files changed, 111 insertions(+), 2 deletions(-) > > > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > > index 9ad3d1e..a50ee04 100644 > > --- a/block/blk-throttle.c > > +++ b/block/blk-throttle.c > ... > > +int blk_throtl_set_anonpage_owner(struct page *page, struct mm_struct *mm) > > +int blk_throtl_set_filepage_owner(struct page *page, struct mm_struct *mm) > > +int blk_throtl_copy_page_owner(struct page *npage, struct page *opage) > > It would be nice if these were named blk_cgroup_*. This is arguably > more correct as the id comes from the blkio subsystem, and isn't > specific to blk-throttle. This will be more important very shortly, > as CFQ will be using this same cgroup id for async IO tracking soon. Sounds reasonable. Will do in the next version. > > is_kernel_io() is a good idea, it avoids a bug that we've run into > with CFQ async IO tracking. Why isn't PF_KTHREAD sufficient to cover > all kernel threads, including kswapd and those marked PF_MEMALLOC? With PF_MEMALLOC we're sure we don't add the page tracking overhead also to non-kernel threads when memory gets low. PF_KSWAPD is not probably needed, AFAICS it is only used by kswapd, that is created by kthread_create() and so it has the PF_KTHREAD flag set. Let's see if someone can give more deatils about that. In the while I'll investigate and try to do some tests only with PF_KTHREAD. Thanks, -Andrea _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Andrea Righi
date: 2011-02-22T19:12:47-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: On Tue, Feb 22, 2011 at 06:12:51PM +0100, Andrea Righi wrote: > Currently the blkio.throttle controller only support synchronous IO requests. > This means that we always look at the current task to identify the "owner" of > each IO request. > > However dirty pages in the page cache can be wrote to disk asynchronously by > the per-bdi flusher kernel threads or by any other thread in the system, > according to the writeback policy. > > For this reason the real writes to the underlying block devices may > occur in a different IO context respect to the task that originally > generated the dirty pages involved in the IO operation. This makes the > tracking and throttling of writeback IO more complicate respect to the > synchronous IO from the blkio controller's perspective. > > The same concept is also valid for anonymous pages involed in IO operations > (swap). > > This patch allow to track the cgroup that originally dirtied each page in page > cache and each anonymous page and pass these informations to the blk-throttle > controller. These informations can be used to provide a better service level > differentiation of buffered writes swap IO between different cgroups. > Hi Andrea, Thanks for the patches. Before I look deeper into patches, had few general queries/thoughts. - So this requires memory controller to be enabled. Does it also require these to be co-mounted? - Currently in throttling there is no limit on number of bios queued per group. I think this is not necessarily a very good idea because if throttling limits are low, we will build very long bio queues. So some AIO process can queue up lots of bios, consume lots of memory without getting blocked. I am sure there will be other side affects too. One of the side affects I noticed is that if an AIO process queues up too much of IO, and if I want to kill it now, it just hangs there for a really-2 long time (waiting for all the throttled IO to complete). So I was thinking of implementing either per group limit o…
creator: Vivek Goyal
date: 2011-02-22T19:34:03-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Tue, 22 Feb 2011 18:12:54 +0100 Andrea Righi <arighi@develer.com> wrote: > The page_cgroup infrastructure, currently available only for the memory > cgroup controller, can be used to store the owner of each page and > opportunely track the writeback IO. This information is encoded in > the upper 16-bits of the page_cgroup->flags. > > A owner can be identified using a generic ID number and the following > interfaces are provided to store a retrieve this information: > > unsigned long page_cgroup_get_owner(struct page *page); > int page_cgroup_set_owner(struct page *page, unsigned long id); > int page_cgroup_copy_owner(struct page *npage, struct page *opage); My immediate observation is that you're not really tracking the "owner" here - you're tracking an opaque 16-bit token known only to the block controller in a field which - if changed by anybody other than the block controller - will lead to mayhem in the block controller. I think it might be clearer - and safer - to say "blkcg" or some such instead of "owner" here. I'm tempted to say it might be better to just add a pointer to your throtl_grp structure into struct page_cgroup. Or maybe replace the mem_cgroup pointer with a single pointer to struct css_set. Both of those ideas, though, probably just add unwanted extra overhead now to gain generality which may or may not be wanted in the future. jon _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Jonathan Corbet
date: 2011-02-22T20:01:45-00:00

Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: On Tue, Feb 22, 2011 at 10:42:41AM -0800, Chad Talbott wrote: > On Tue, Feb 22, 2011 at 9:12 AM, Andrea Righi <arighi@develer.com> wrote: > > Add the tracking of buffered (writeback) and anonymous pages. > ... > > --- > > block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- > > include/linux/blkdev.h | 26 ++++++++++++++- > > 2 files changed, 111 insertions(+), 2 deletions(-) > > > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > > index 9ad3d1e..a50ee04 100644 > > --- a/block/blk-throttle.c > > +++ b/block/blk-throttle.c > ... > > +int blk_throtl_set_anonpage_owner(struct page *page, struct mm_struct *mm) > > +int blk_throtl_set_filepage_owner(struct page *page, struct mm_struct *mm) > > +int blk_throtl_copy_page_owner(struct page *npage, struct page *opage) > > It would be nice if these were named blk_cgroup_*. This is arguably > more correct as the id comes from the blkio subsystem, and isn't > specific to blk-throttle. This will be more important very shortly, > as CFQ will be using this same cgroup id for async IO tracking soon. Should this really be all part of blk-cgroup.c and not blk-throttle.c so that it can be used by CFQ code also down the line? Anyway all this is not throttle specific as such but blkio controller specific. Though function naming convetion is not great in blk-cgroup.c But functions either have blkio_ prefix or blkiocg_ prefix. Functions which are not directly dealing with cgroups or in general are called by blk-throttle.c and/or cfq-iosched.c I have marked as prefixed with "blkio_". Functions which directly deal with cgroup stuff and register with cgroup subsystem for this controller are generally having "blkiocg_" prefix. In this case probably we can use probably blkio_ prefix. Thanks Vivek _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Vivek Goyal
date: 2011-02-22T20:49:28-00:00

Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: On Tue, Feb 22, 2011 at 06:12:55PM +0100, Andrea Righi wrote: > Add the tracking of buffered (writeback) and anonymous pages. > > Dirty pages in the page cache can be processed asynchronously by the > per-bdi flusher kernel threads or by any other thread in the system, > according to the writeback policy. > > For this reason the real writes to the underlying block devices may > occur in a different IO context respect to the task that originally > generated the dirty pages involved in the IO operation. This makes > the tracking and throttling of writeback IO more complicate respect to > the synchronous IO from the blkio controller's point of view. > > The idea is to save the cgroup owner of each anonymous page and dirty > page in page cache. A page is associated to a cgroup the first time it > is dirtied in memory (for file cache pages) or when it is set as > swap-backed (for anonymous pages). This information is stored using the > page_cgroup functionality. > > Then, at the block layer, it is possible to retrieve the throttle group > looking at the bio_page(bio). If the page was not explicitly associated > to any cgroup the IO operation is charged to the current task/cgroup, as > it was done by the previous implementation. > > Signed-off-by: Andrea Righi <arighi@develer.com> > --- > block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- > include/linux/blkdev.h | 26 ++++++++++++++- > 2 files changed, 111 insertions(+), 2 deletions(-) > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > index 9ad3d1e..a50ee04 100644 > --- a/block/blk-throttle.c > +++ b/block/blk-throttle.c > @@ -8,6 +8,10 @@ > #include <linux/slab.h> > #include <linux/blkdev.h> > #include <linux/bio.h> > +#include <linux/memcontrol.h> > +#include <linux/mm_inline.h> > +#include <linux/pagemap.h> > +#include <linux/page_cgroup.h> > #include <linux/blktrace_api.h> > #include <linux/blk-cgroup.h> > > @@ -221,6 +225,85 @@ done: > return tg; > } > > +static inline bool is_ker…
creator: Vivek Goyal
date: 2011-02-22T21:00:30-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Tue, Feb 22, 2011 at 06:12:54PM +0100, Andrea Righi wrote: > The page_cgroup infrastructure, currently available only for the memory > cgroup controller, can be used to store the owner of each page and > opportunely track the writeback IO. This information is encoded in > the upper 16-bits of the page_cgroup->flags. > > A owner can be identified using a generic ID number and the following > interfaces are provided to store a retrieve this information: > > unsigned long page_cgroup_get_owner(struct page *page); > int page_cgroup_set_owner(struct page *page, unsigned long id); > int page_cgroup_copy_owner(struct page *npage, struct page *opage); > > The blkio.throttle controller can use the cgroup css_id() as the owner's > ID number. > > Signed-off-by: Andrea Righi <arighi@develer.com> > --- > block/Kconfig | 2 + > block/blk-cgroup.c | 6 ++ > include/linux/memcontrol.h | 6 ++ > include/linux/mmzone.h | 4 +- > include/linux/page_cgroup.h | 33 ++++++++++- > init/Kconfig | 4 + > mm/Makefile | 3 +- > mm/memcontrol.c | 6 ++ > mm/page_cgroup.c | 129 +++++++++++++++++++++++++++++++++++++++---- > 9 files changed, 176 insertions(+), 17 deletions(-) > > diff --git a/block/Kconfig b/block/Kconfig > index 60be1e0..1351ea8 100644 > --- a/block/Kconfig > +++ b/block/Kconfig > @@ -80,6 +80,8 @@ config BLK_DEV_INTEGRITY > config BLK_DEV_THROTTLING > bool "Block layer bio throttling support" > depends on BLK_CGROUP=y && EXPERIMENTAL > + select MM_OWNER > + select PAGE_TRACKING > default n > ---help--- > Block layer bio throttling support. It can be used to limit > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > index f283ae1..5c57f0a 100644 > --- a/block/blk-cgroup.c > +++ b/block/blk-cgroup.c > @@ -107,6 +107,12 @@ blkio_policy_search_node(const struct blkio_cgroup *blkcg, dev_t dev, > return NULL; > } > > +bool blkio_cgroup_disabled(void) > +{ > + return blkio_subsys.disabled ? true : false; > +} > +EXPORT_SYMBOL_GPL(blkio_cgroup_disabled); > + I think there should be opti…
creator: Vivek Goyal
date: 2011-02-22T21:22:53-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Tue, Feb 22, 2011 at 01:01:45PM -0700, Jonathan Corbet wrote: > On Tue, 22 Feb 2011 18:12:54 +0100 > Andrea Righi <arighi@develer.com> wrote: > > > The page_cgroup infrastructure, currently available only for the memory > > cgroup controller, can be used to store the owner of each page and > > opportunely track the writeback IO. This information is encoded in > > the upper 16-bits of the page_cgroup->flags. > > > > A owner can be identified using a generic ID number and the following > > interfaces are provided to store a retrieve this information: > > > > unsigned long page_cgroup_get_owner(struct page *page); > > int page_cgroup_set_owner(struct page *page, unsigned long id); > > int page_cgroup_copy_owner(struct page *npage, struct page *opage); > > My immediate observation is that you're not really tracking the "owner" > here - you're tracking an opaque 16-bit token known only to the block > controller in a field which - if changed by anybody other than the block > controller - will lead to mayhem in the block controller. I think it > might be clearer - and safer - to say "blkcg" or some such instead of > "owner" here. > > I'm tempted to say it might be better to just add a pointer to your > throtl_grp structure into struct page_cgroup. throtl_grp might not even be present when page is being dirtied. When this IO is actually submitted to device, we migth end up creating new throtl_grp. I guess other concern here would be increasing the size of page_cgroup structure. I guess you meant storing a pointer to blkio_cgroup, along the lines of storing a pointer to mem_cgroup. That also means extra 8 bytes and only one subsystem can use it at a time. So using upper bits of pc->flags is probably better. > Or maybe replace the > mem_cgroup pointer with a single pointer to struct css_set. Both of > those ideas, though, probably just add unwanted extra overhead now to gain > generality which may or may not be wanted in the future. This sounds interesting. IIUC, then this…
creator: Vivek Goyal
date: 2011-02-22T21:57:20-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: On Tue, Feb 22, 2011 at 02:34:03PM -0500, Vivek Goyal wrote: > On Tue, Feb 22, 2011 at 06:12:51PM +0100, Andrea Righi wrote: > > Currently the blkio.throttle controller only support synchronous IO requests. > > This means that we always look at the current task to identify the "owner" of > > each IO request. > > > > However dirty pages in the page cache can be wrote to disk asynchronously by > > the per-bdi flusher kernel threads or by any other thread in the system, > > according to the writeback policy. > > > > For this reason the real writes to the underlying block devices may > > occur in a different IO context respect to the task that originally > > generated the dirty pages involved in the IO operation. This makes the > > tracking and throttling of writeback IO more complicate respect to the > > synchronous IO from the blkio controller's perspective. > > > > The same concept is also valid for anonymous pages involed in IO operations > > (swap). > > > > This patch allow to track the cgroup that originally dirtied each page in page > > cache and each anonymous page and pass these informations to the blk-throttle > > controller. These informations can be used to provide a better service level > > differentiation of buffered writes swap IO between different cgroups. > > > > Hi Andrea, > > Thanks for the patches. Before I look deeper into patches, had few > general queries/thoughts. > > - So this requires memory controller to be enabled. Does it also require > these to be co-mounted? No and no. The blkio controller enables and uses the page_cgroup functionality, but it doesn't depend on the memory controller. It automatically selects CONFIG_MM_OWNER and CONFIG_PAGE_TRACKING (last one added in PATCH 3/5) and this is sufficient to make page_cgroup usable from any generic controller. > > - Currently in throttling there is no limit on number of bios queued > per group. I think this is not necessarily a very good idea because > if throttling limits are low, we will buil…
creator: Andrea Righi
date: 2011-02-22T22:41:41-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Tue, Feb 22, 2011 at 01:01:45PM -0700, Jonathan Corbet wrote: > On Tue, 22 Feb 2011 18:12:54 +0100 > Andrea Righi <arighi@develer.com> wrote: > > > The page_cgroup infrastructure, currently available only for the memory > > cgroup controller, can be used to store the owner of each page and > > opportunely track the writeback IO. This information is encoded in > > the upper 16-bits of the page_cgroup->flags. > > > > A owner can be identified using a generic ID number and the following > > interfaces are provided to store a retrieve this information: > > > > unsigned long page_cgroup_get_owner(struct page *page); > > int page_cgroup_set_owner(struct page *page, unsigned long id); > > int page_cgroup_copy_owner(struct page *npage, struct page *opage); > > My immediate observation is that you're not really tracking the "owner" > here - you're tracking an opaque 16-bit token known only to the block > controller in a field which - if changed by anybody other than the block > controller - will lead to mayhem in the block controller. I think it > might be clearer - and safer - to say "blkcg" or some such instead of > "owner" here. > Basically the idea here was to be as generic as possible and make this feature potentially available also to other subsystems, so that cgroup subsystems may represent whatever they want with the 16-bit token. However, no more than a single subsystem may be able to use this feature at the same time. > I'm tempted to say it might be better to just add a pointer to your > throtl_grp structure into struct page_cgroup. Or maybe replace the > mem_cgroup pointer with a single pointer to struct css_set. Both of > those ideas, though, probably just add unwanted extra overhead now to gain > generality which may or may not be wanted in the future. The pointer to css_set sounds good, but it would add additional space to the page_cgroup struct. Now, page_cgroup is 40 bytes (in 64-bit arch) and all of them are allocated at boot time. Using unused bits in p…
creator: Andrea Righi
date: 2011-02-22T23:01:47-00:00

Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: On Tue, Feb 22, 2011 at 03:49:28PM -0500, Vivek Goyal wrote: > On Tue, Feb 22, 2011 at 10:42:41AM -0800, Chad Talbott wrote: > > On Tue, Feb 22, 2011 at 9:12 AM, Andrea Righi <arighi@develer.com> wrote: > > > Add the tracking of buffered (writeback) and anonymous pages. > > ... > > > --- > > > block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- > > > include/linux/blkdev.h | 26 ++++++++++++++- > > > 2 files changed, 111 insertions(+), 2 deletions(-) > > > > > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > > > index 9ad3d1e..a50ee04 100644 > > > --- a/block/blk-throttle.c > > > +++ b/block/blk-throttle.c > > ... > > > +int blk_throtl_set_anonpage_owner(struct page *page, struct mm_struct *mm) > > > +int blk_throtl_set_filepage_owner(struct page *page, struct mm_struct *mm) > > > +int blk_throtl_copy_page_owner(struct page *npage, struct page *opage) > > > > It would be nice if these were named blk_cgroup_*. This is arguably > > more correct as the id comes from the blkio subsystem, and isn't > > specific to blk-throttle. This will be more important very shortly, > > as CFQ will be using this same cgroup id for async IO tracking soon. > > Should this really be all part of blk-cgroup.c and not blk-throttle.c > so that it can be used by CFQ code also down the line? Anyway all this > is not throttle specific as such but blkio controller specific. Agreed. > > Though function naming convetion is not great in blk-cgroup.c But > functions either have blkio_ prefix or blkiocg_ prefix. ok. > > Functions which are not directly dealing with cgroups or in general > are called by blk-throttle.c and/or cfq-iosched.c I have marked as > prefixed with "blkio_". Functions which directly deal with cgroup stuff > and register with cgroup subsystem for this controller are generally > having "blkiocg_" prefix. > > In this case probably we can use probably blkio_ prefix. ok. Thanks, -Andrea _______________________________________________ Containers maili…
creator: Andrea Righi
date: 2011-02-22T23:03:54-00:00

Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: On Tue, Feb 22, 2011 at 04:00:30PM -0500, Vivek Goyal wrote: > On Tue, Feb 22, 2011 at 06:12:55PM +0100, Andrea Righi wrote: > > Add the tracking of buffered (writeback) and anonymous pages. > > > > Dirty pages in the page cache can be processed asynchronously by the > > per-bdi flusher kernel threads or by any other thread in the system, > > according to the writeback policy. > > > > For this reason the real writes to the underlying block devices may > > occur in a different IO context respect to the task that originally > > generated the dirty pages involved in the IO operation. This makes > > the tracking and throttling of writeback IO more complicate respect to > > the synchronous IO from the blkio controller's point of view. > > > > The idea is to save the cgroup owner of each anonymous page and dirty > > page in page cache. A page is associated to a cgroup the first time it > > is dirtied in memory (for file cache pages) or when it is set as > > swap-backed (for anonymous pages). This information is stored using the > > page_cgroup functionality. > > > > Then, at the block layer, it is possible to retrieve the throttle group > > looking at the bio_page(bio). If the page was not explicitly associated > > to any cgroup the IO operation is charged to the current task/cgroup, as > > it was done by the previous implementation. > > > > Signed-off-by: Andrea Righi <arighi@develer.com> > > --- > > block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- > > include/linux/blkdev.h | 26 ++++++++++++++- > > 2 files changed, 111 insertions(+), 2 deletions(-) > > > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > > index 9ad3d1e..a50ee04 100644 > > --- a/block/blk-throttle.c > > +++ b/block/blk-throttle.c > > @@ -8,6 +8,10 @@ > > #include <linux/slab.h> > > #include <linux/blkdev.h> > > #include <linux/bio.h> > > +#include <linux/memcontrol.h> > > +#include <linux/mm_inline.h> > > +#include <linux/pagemap.h> > > +#include <linux/page_cgroup.h>…
creator: Andrea Righi
date: 2011-02-22T23:05:34-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Wed, Feb 23, 2011 at 12:01:47AM +0100, Andrea Righi wrote: > On Tue, Feb 22, 2011 at 01:01:45PM -0700, Jonathan Corbet wrote: > > On Tue, 22 Feb 2011 18:12:54 +0100 > > Andrea Righi <arighi@develer.com> wrote: > > > > > The page_cgroup infrastructure, currently available only for the memory > > > cgroup controller, can be used to store the owner of each page and > > > opportunely track the writeback IO. This information is encoded in > > > the upper 16-bits of the page_cgroup->flags. > > > > > > A owner can be identified using a generic ID number and the following > > > interfaces are provided to store a retrieve this information: > > > > > > unsigned long page_cgroup_get_owner(struct page *page); > > > int page_cgroup_set_owner(struct page *page, unsigned long id); > > > int page_cgroup_copy_owner(struct page *npage, struct page *opage); > > > > My immediate observation is that you're not really tracking the "owner" > > here - you're tracking an opaque 16-bit token known only to the block > > controller in a field which - if changed by anybody other than the block > > controller - will lead to mayhem in the block controller. I think it > > might be clearer - and safer - to say "blkcg" or some such instead of > > "owner" here. > > > > Basically the idea here was to be as generic as possible and make this > feature potentially available also to other subsystems, so that cgroup > subsystems may represent whatever they want with the 16-bit token. > However, no more than a single subsystem may be able to use this feature > at the same time. > > > I'm tempted to say it might be better to just add a pointer to your > > throtl_grp structure into struct page_cgroup. Or maybe replace the > > mem_cgroup pointer with a single pointer to struct css_set. Both of > > those ideas, though, probably just add unwanted extra overhead now to gain > > generality which may or may not be wanted in the future. > > The pointer to css_set sounds good, but it would add additional space to …
creator: Vivek Goyal
date: 2011-02-22T23:06:30-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Tue, Feb 22, 2011 at 04:22:53PM -0500, Vivek Goyal wrote: > On Tue, Feb 22, 2011 at 06:12:54PM +0100, Andrea Righi wrote: > > The page_cgroup infrastructure, currently available only for the memory > > cgroup controller, can be used to store the owner of each page and > > opportunely track the writeback IO. This information is encoded in > > the upper 16-bits of the page_cgroup->flags. > > > > A owner can be identified using a generic ID number and the following > > interfaces are provided to store a retrieve this information: > > > > unsigned long page_cgroup_get_owner(struct page *page); > > int page_cgroup_set_owner(struct page *page, unsigned long id); > > int page_cgroup_copy_owner(struct page *npage, struct page *opage); > > > > The blkio.throttle controller can use the cgroup css_id() as the owner's > > ID number. > > > > Signed-off-by: Andrea Righi <arighi@develer.com> > > --- > > block/Kconfig | 2 + > > block/blk-cgroup.c | 6 ++ > > include/linux/memcontrol.h | 6 ++ > > include/linux/mmzone.h | 4 +- > > include/linux/page_cgroup.h | 33 ++++++++++- > > init/Kconfig | 4 + > > mm/Makefile | 3 +- > > mm/memcontrol.c | 6 ++ > > mm/page_cgroup.c | 129 +++++++++++++++++++++++++++++++++++++++---- > > 9 files changed, 176 insertions(+), 17 deletions(-) > > > > diff --git a/block/Kconfig b/block/Kconfig > > index 60be1e0..1351ea8 100644 > > --- a/block/Kconfig > > +++ b/block/Kconfig > > @@ -80,6 +80,8 @@ config BLK_DEV_INTEGRITY > > config BLK_DEV_THROTTLING > > bool "Block layer bio throttling support" > > depends on BLK_CGROUP=y && EXPERIMENTAL > > + select MM_OWNER > > + select PAGE_TRACKING > > default n > > ---help--- > > Block layer bio throttling support. It can be used to limit > > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > > index f283ae1..5c57f0a 100644 > > --- a/block/blk-cgroup.c > > +++ b/block/blk-cgroup.c > > @@ -107,6 +107,12 @@ blkio_policy_search_node(const struct blkio_cgroup *blkcg, dev_t dev, > > return NULL; > > } > > > > +bool b…
creator: Andrea Righi
date: 2011-02-22T23:08:40-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Tue, 22 Feb 2011 18:06:30 -0500 Vivek Goyal <vgoyal@redhat.com> wrote: > I think John suggested replacing mem_cgroup pointer with css_set so that > size of the strcuture does not increase but it leads extra level of > indirection. That is what I was thinking. But I did also say it's probably premature generalization at this point, especially given that there'd be a runtime cost. jon _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Jonathan Corbet
date: 2011-02-22T23:21:47-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Wed, 23 Feb 2011 00:01:47 +0100 Andrea Righi <arighi@develer.com> wrote: > > My immediate observation is that you're not really tracking the "owner" > > here - you're tracking an opaque 16-bit token known only to the block > > controller in a field which - if changed by anybody other than the block > > controller - will lead to mayhem in the block controller. I think it > > might be clearer - and safer - to say "blkcg" or some such instead of > > "owner" here. > > Basically the idea here was to be as generic as possible and make this > feature potentially available also to other subsystems, so that cgroup > subsystems may represent whatever they want with the 16-bit token. > However, no more than a single subsystem may be able to use this feature > at the same time. That makes me nervous; it can't really be used that way unless we want to say that certain controllers are fundamentally incompatible and can't be allowed to play together. For whatever my $0.02 are worth (given the state of the US dollar, that's not a whole lot), I'd suggest keeping the current mechanism, but make it clear that it belongs to your controller. If and when another controller comes along with a need for similar functionality, somebody can worry about making it more general. jon _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Jonathan Corbet
date: 2011-02-22T23:27:29-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Tue, Feb 22, 2011 at 06:06:30PM -0500, Vivek Goyal wrote: > On Wed, Feb 23, 2011 at 12:01:47AM +0100, Andrea Righi wrote: > > On Tue, Feb 22, 2011 at 01:01:45PM -0700, Jonathan Corbet wrote: > > > On Tue, 22 Feb 2011 18:12:54 +0100 > > > Andrea Righi <arighi@develer.com> wrote: > > > > > > > The page_cgroup infrastructure, currently available only for the memory > > > > cgroup controller, can be used to store the owner of each page and > > > > opportunely track the writeback IO. This information is encoded in > > > > the upper 16-bits of the page_cgroup->flags. > > > > > > > > A owner can be identified using a generic ID number and the following > > > > interfaces are provided to store a retrieve this information: > > > > > > > > unsigned long page_cgroup_get_owner(struct page *page); > > > > int page_cgroup_set_owner(struct page *page, unsigned long id); > > > > int page_cgroup_copy_owner(struct page *npage, struct page *opage); > > > > > > My immediate observation is that you're not really tracking the "owner" > > > here - you're tracking an opaque 16-bit token known only to the block > > > controller in a field which - if changed by anybody other than the block > > > controller - will lead to mayhem in the block controller. I think it > > > might be clearer - and safer - to say "blkcg" or some such instead of > > > "owner" here. > > > > > > > Basically the idea here was to be as generic as possible and make this > > feature potentially available also to other subsystems, so that cgroup > > subsystems may represent whatever they want with the 16-bit token. > > However, no more than a single subsystem may be able to use this feature > > at the same time. > > > > > I'm tempted to say it might be better to just add a pointer to your > > > throtl_grp structure into struct page_cgroup. Or maybe replace the > > > mem_cgroup pointer with a single pointer to struct css_set. Both of > > > those ideas, though, probably just add unwanted extra overhead now to gain > > > g…
creator: Andrea Righi
date: 2011-02-22T23:37:18-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Tue, Feb 22, 2011 at 04:27:29PM -0700, Jonathan Corbet wrote: > On Wed, 23 Feb 2011 00:01:47 +0100 > Andrea Righi <arighi@develer.com> wrote: > > > > My immediate observation is that you're not really tracking the "owner" > > > here - you're tracking an opaque 16-bit token known only to the block > > > controller in a field which - if changed by anybody other than the block > > > controller - will lead to mayhem in the block controller. I think it > > > might be clearer - and safer - to say "blkcg" or some such instead of > > > "owner" here. > > > > Basically the idea here was to be as generic as possible and make this > > feature potentially available also to other subsystems, so that cgroup > > subsystems may represent whatever they want with the 16-bit token. > > However, no more than a single subsystem may be able to use this feature > > at the same time. > > That makes me nervous; it can't really be used that way unless we want to > say that certain controllers are fundamentally incompatible and can't be > allowed to play together. For whatever my $0.02 are worth (given the > state of the US dollar, that's not a whole lot), I'd suggest keeping the > current mechanism, but make it clear that it belongs to your controller. > If and when another controller comes along with a need for similar > functionality, somebody can worry about making it more general. OK, I understand. I'll use "blkio" instead of "owner". Also because I wouldn't like to introduce additional logic and overhead to check if two controllers are using this feature at the same time. Better to hard-code this information in the name of the functions. Probably the most generic solution is the one that you suggested: replace the mem_cgroup with a pointer to css_set. I'll also try to investigate this way. Thanks, -Andrea _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Andrea Righi
date: 2011-02-22T23:48:33-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: On Tue, Feb 22, 2011 at 11:41:41PM +0100, Andrea Righi wrote: > On Tue, Feb 22, 2011 at 02:34:03PM -0500, Vivek Goyal wrote: > > On Tue, Feb 22, 2011 at 06:12:51PM +0100, Andrea Righi wrote: > > > Currently the blkio.throttle controller only support synchronous IO requests. > > > This means that we always look at the current task to identify the "owner" of > > > each IO request. > > > > > > However dirty pages in the page cache can be wrote to disk asynchronously by > > > the per-bdi flusher kernel threads or by any other thread in the system, > > > according to the writeback policy. > > > > > > For this reason the real writes to the underlying block devices may > > > occur in a different IO context respect to the task that originally > > > generated the dirty pages involved in the IO operation. This makes the > > > tracking and throttling of writeback IO more complicate respect to the > > > synchronous IO from the blkio controller's perspective. > > > > > > The same concept is also valid for anonymous pages involed in IO operations > > > (swap). > > > > > > This patch allow to track the cgroup that originally dirtied each page in page > > > cache and each anonymous page and pass these informations to the blk-throttle > > > controller. These informations can be used to provide a better service level > > > differentiation of buffered writes swap IO between different cgroups. > > > > > > > Hi Andrea, > > > > Thanks for the patches. Before I look deeper into patches, had few > > general queries/thoughts. > > > > - So this requires memory controller to be enabled. Does it also require > > these to be co-mounted? > > No and no. The blkio controller enables and uses the page_cgroup > functionality, but it doesn't depend on the memory controller. It > automatically selects CONFIG_MM_OWNER and CONFIG_PAGE_TRACKING (last > one added in PATCH 3/5) and this is sufficient to make page_cgroup > usable from any generic controller. > > > > > - Currently in throttling there is no l…
creator: Vivek Goyal
date: 2011-02-23T00:03:58-00:00

Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: On Wed, Feb 23, 2011 at 12:05:34AM +0100, Andrea Righi wrote: > On Tue, Feb 22, 2011 at 04:00:30PM -0500, Vivek Goyal wrote: > > On Tue, Feb 22, 2011 at 06:12:55PM +0100, Andrea Righi wrote: > > > Add the tracking of buffered (writeback) and anonymous pages. > > > > > > Dirty pages in the page cache can be processed asynchronously by the > > > per-bdi flusher kernel threads or by any other thread in the system, > > > according to the writeback policy. > > > > > > For this reason the real writes to the underlying block devices may > > > occur in a different IO context respect to the task that originally > > > generated the dirty pages involved in the IO operation. This makes > > > the tracking and throttling of writeback IO more complicate respect to > > > the synchronous IO from the blkio controller's point of view. > > > > > > The idea is to save the cgroup owner of each anonymous page and dirty > > > page in page cache. A page is associated to a cgroup the first time it > > > is dirtied in memory (for file cache pages) or when it is set as > > > swap-backed (for anonymous pages). This information is stored using the > > > page_cgroup functionality. > > > > > > Then, at the block layer, it is possible to retrieve the throttle group > > > looking at the bio_page(bio). If the page was not explicitly associated > > > to any cgroup the IO operation is charged to the current task/cgroup, as > > > it was done by the previous implementation. > > > > > > Signed-off-by: Andrea Righi <arighi@develer.com> > > > --- > > > block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- > > > include/linux/blkdev.h | 26 ++++++++++++++- > > > 2 files changed, 111 insertions(+), 2 deletions(-) > > > > > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > > > index 9ad3d1e..a50ee04 100644 > > > --- a/block/blk-throttle.c > > > +++ b/block/blk-throttle.c > > > @@ -8,6 +8,10 @@ > > > #include <linux/slab.h> > > > #include <linux/blkdev.h> > > > #include <linux/bio…
creator: Vivek Goyal
date: 2011-02-23T00:07:19-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Wed, 23 Feb 2011 00:37:18 +0100 Andrea Righi <arighi@develer.com> wrote: > On Tue, Feb 22, 2011 at 06:06:30PM -0500, Vivek Goyal wrote: > > On Wed, Feb 23, 2011 at 12:01:47AM +0100, Andrea Righi wrote: > > > On Tue, Feb 22, 2011 at 01:01:45PM -0700, Jonathan Corbet wrote: > > > > On Tue, 22 Feb 2011 18:12:54 +0100 > > > > Andrea Righi <arighi@develer.com> wrote: > > > > > > > > > The page_cgroup infrastructure, currently available only for the memory > > > > > cgroup controller, can be used to store the owner of each page and > > > > > opportunely track the writeback IO. This information is encoded in > > > > > the upper 16-bits of the page_cgroup->flags. > > > > > > > > > > A owner can be identified using a generic ID number and the following > > > > > interfaces are provided to store a retrieve this information: > > > > > > > > > > unsigned long page_cgroup_get_owner(struct page *page); > > > > > int page_cgroup_set_owner(struct page *page, unsigned long id); > > > > > int page_cgroup_copy_owner(struct page *npage, struct page *opage); > > > > > > > > My immediate observation is that you're not really tracking the "owner" > > > > here - you're tracking an opaque 16-bit token known only to the block > > > > controller in a field which - if changed by anybody other than the block > > > > controller - will lead to mayhem in the block controller. I think it > > > > might be clearer - and safer - to say "blkcg" or some such instead of > > > > "owner" here. > > > > > > > > > > Basically the idea here was to be as generic as possible and make this > > > feature potentially available also to other subsystems, so that cgroup > > > subsystems may represent whatever they want with the 16-bit token. > > > However, no more than a single subsystem may be able to use this feature > > > at the same time. > > > > > > > I'm tempted to say it might be better to just add a pointer to your > > > > throtl_grp structure into struct page_cgroup. Or maybe replace the > > > > mem_cgroup…
creator: KAMEZAWA Hiroyuki
date: 2011-02-23T04:49:10-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: On Tue, Feb 22, 2011 at 07:03:58PM -0500, Vivek Goyal wrote: > > I think we should accept to have an inode granularity. We could redesign > > the writeback code to work per-cgroup / per-page, etc. but that would > > add a huge overhead. The limit of inode granularity could be an > > acceptable tradeoff, cgroups are supposed to work to different files > > usually, well.. except when databases come into play (ouch!). > > Agreed. Granularity of per inode level might be accetable in many > cases. Again, I am worried faster group getting stuck behind slower > group. > > I am wondering if we are trying to solve the problem of ASYNC write throttling > at wrong layer. Should ASYNC IO be throttled before we allow task to write to > page cache. The way we throttle the process based on dirty ratio, can we > just check for throttle limits also there or something like that.(I think > that's what you had done in your initial throttling controller implementation?) Right. This is exactly the same approach I've used in my old throttling controller: throttle sync READs and WRITEs at the block layer and async WRITEs when the task is dirtying memory pages. This is probably the simplest way to resolve the problem of faster group getting blocked by slower group, but the controller will be a little bit more leaky, because the writeback IO will be never throttled and we'll see some limited IO spikes during the writeback. However, this is always a better solution IMHO respect to the current implementation that is affected by that kind of priority inversion problem. I can try to add this logic to the current blk-throttle controller if you think it is worth to test it. -Andrea _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Andrea Righi
date: 2011-02-23T08:32:06-00:00

Re: [PATCH 4/5] blk-throttle: track buffered and anonymous pages
type: item
description: On Tue, Feb 22, 2011 at 07:07:19PM -0500, Vivek Goyal wrote: > On Wed, Feb 23, 2011 at 12:05:34AM +0100, Andrea Righi wrote: > > On Tue, Feb 22, 2011 at 04:00:30PM -0500, Vivek Goyal wrote: > > > On Tue, Feb 22, 2011 at 06:12:55PM +0100, Andrea Righi wrote: > > > > Add the tracking of buffered (writeback) and anonymous pages. > > > > > > > > Dirty pages in the page cache can be processed asynchronously by the > > > > per-bdi flusher kernel threads or by any other thread in the system, > > > > according to the writeback policy. > > > > > > > > For this reason the real writes to the underlying block devices may > > > > occur in a different IO context respect to the task that originally > > > > generated the dirty pages involved in the IO operation. This makes > > > > the tracking and throttling of writeback IO more complicate respect to > > > > the synchronous IO from the blkio controller's point of view. > > > > > > > > The idea is to save the cgroup owner of each anonymous page and dirty > > > > page in page cache. A page is associated to a cgroup the first time it > > > > is dirtied in memory (for file cache pages) or when it is set as > > > > swap-backed (for anonymous pages). This information is stored using the > > > > page_cgroup functionality. > > > > > > > > Then, at the block layer, it is possible to retrieve the throttle group > > > > looking at the bio_page(bio). If the page was not explicitly associated > > > > to any cgroup the IO operation is charged to the current task/cgroup, as > > > > it was done by the previous implementation. > > > > > > > > Signed-off-by: Andrea Righi <arighi@develer.com> > > > > --- > > > > block/blk-throttle.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++- > > > > include/linux/blkdev.h | 26 ++++++++++++++- > > > > 2 files changed, 111 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > > > > index 9ad3d1e..a50ee04 100644 > > > > --- a/block/blk-throttle.c > > > > +++ …
creator: Andrea Righi
date: 2011-02-23T08:37:40-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Wed, Feb 23, 2011 at 01:49:10PM +0900, KAMEZAWA Hiroyuki wrote: > On Wed, 23 Feb 2011 00:37:18 +0100 > Andrea Righi <arighi@develer.com> wrote: > > > On Tue, Feb 22, 2011 at 06:06:30PM -0500, Vivek Goyal wrote: > > > On Wed, Feb 23, 2011 at 12:01:47AM +0100, Andrea Righi wrote: > > > > On Tue, Feb 22, 2011 at 01:01:45PM -0700, Jonathan Corbet wrote: > > > > > On Tue, 22 Feb 2011 18:12:54 +0100 > > > > > Andrea Righi <arighi@develer.com> wrote: > > > > > > > > > > > The page_cgroup infrastructure, currently available only for the memory > > > > > > cgroup controller, can be used to store the owner of each page and > > > > > > opportunely track the writeback IO. This information is encoded in > > > > > > the upper 16-bits of the page_cgroup->flags. > > > > > > > > > > > > A owner can be identified using a generic ID number and the following > > > > > > interfaces are provided to store a retrieve this information: > > > > > > > > > > > > unsigned long page_cgroup_get_owner(struct page *page); > > > > > > int page_cgroup_set_owner(struct page *page, unsigned long id); > > > > > > int page_cgroup_copy_owner(struct page *npage, struct page *opage); > > > > > > > > > > My immediate observation is that you're not really tracking the "owner" > > > > > here - you're tracking an opaque 16-bit token known only to the block > > > > > controller in a field which - if changed by anybody other than the block > > > > > controller - will lead to mayhem in the block controller. I think it > > > > > might be clearer - and safer - to say "blkcg" or some such instead of > > > > > "owner" here. > > > > > > > > > > > > > Basically the idea here was to be as generic as possible and make this > > > > feature potentially available also to other subsystems, so that cgroup > > > > subsystems may represent whatever they want with the 16-bit token. > > > > However, no more than a single subsystem may be able to use this feature > > > > at the same time. > > > > > > > > > I'm tempted to say it …
creator: Andrea Righi
date: 2011-02-23T08:59:11-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: > > Agreed. Granularity of per inode level might be accetable in many > > cases. Again, I am worried faster group getting stuck behind slower > > group. > > > > I am wondering if we are trying to solve the problem of ASYNC write throttling > > at wrong layer. Should ASYNC IO be throttled before we allow task to write to > > page cache. The way we throttle the process based on dirty ratio, can we > > just check for throttle limits also there or something like that.(I think > > that's what you had done in your initial throttling controller implementation?) > > Right. This is exactly the same approach I've used in my old throttling > controller: throttle sync READs and WRITEs at the block layer and async > WRITEs when the task is dirtying memory pages. > > This is probably the simplest way to resolve the problem of faster group > getting blocked by slower group, but the controller will be a little bit > more leaky, because the writeback IO will be never throttled and we'll > see some limited IO spikes during the writeback. Yes writeback will not be throttled. Not sure how big a problem that is. - We have controlled the input rate. So that should help a bit. - May be one can put some high limit on root cgroup to in blkio throttle controller to limit overall WRITE rate of the system. - For SATA disks, try to use CFQ which can try to minimize the impact of WRITE. It will atleast provide consistent bandwindth experience to application. >However, this is always > a better solution IMHO respect to the current implementation that is > affected by that kind of priority inversion problem. > > I can try to add this logic to the current blk-throttle controller if > you think it is worth to test it. At this point of time I have few concerns with this approach. - Configuration issues. Asking user to plan for SYNC ans ASYNC IO separately is inconvenient. One has to know the nature of workload. - Most likely we will come up with global limits (atleast to begin with), and not per devi…
creator: Vivek Goyal
date: 2011-02-23T15:23:54-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: On Wed, Feb 23, 2011 at 10:23:54AM -0500, Vivek Goyal wrote: > > > Agreed. Granularity of per inode level might be accetable in many > > > cases. Again, I am worried faster group getting stuck behind slower > > > group. > > > > > > I am wondering if we are trying to solve the problem of ASYNC write throttling > > > at wrong layer. Should ASYNC IO be throttled before we allow task to write to > > > page cache. The way we throttle the process based on dirty ratio, can we > > > just check for throttle limits also there or something like that.(I think > > > that's what you had done in your initial throttling controller implementation?) > > > > Right. This is exactly the same approach I've used in my old throttling > > controller: throttle sync READs and WRITEs at the block layer and async > > WRITEs when the task is dirtying memory pages. > > > > This is probably the simplest way to resolve the problem of faster group > > getting blocked by slower group, but the controller will be a little bit > > more leaky, because the writeback IO will be never throttled and we'll > > see some limited IO spikes during the writeback. > > Yes writeback will not be throttled. Not sure how big a problem that is. > > - We have controlled the input rate. So that should help a bit. > - May be one can put some high limit on root cgroup to in blkio throttle > controller to limit overall WRITE rate of the system. > - For SATA disks, try to use CFQ which can try to minimize the impact of > WRITE. > > It will atleast provide consistent bandwindth experience to application. Right. > > >However, this is always > > a better solution IMHO respect to the current implementation that is > > affected by that kind of priority inversion problem. > > > > I can try to add this logic to the current blk-throttle controller if > > you think it is worth to test it. > > At this point of time I have few concerns with this approach. > > - Configuration issues. Asking user to plan for SYNC ans ASYNC IO > separately…
creator: Andrea Righi
date: 2011-02-23T23:14:11-00:00

Re: [PATCH 3/5] page_cgroup: make page tracking available for blkio
type: item
description: On Wed, 23 Feb 2011 09:59:11 +0100 Andrea Righi <arighi@develer.com> wrote: > > > > I wonder I can make pc->mem_cgroup to be pc->memid(16bit), then, > > == > > static inline struct mem_cgroup *get_memcg_from_pc(struct page_cgroup *pc) > > { > > struct cgroup_subsys_state *css = css_lookup(&mem_cgroup_subsys, pc->memid); > > return container_of(css, struct mem_cgroup, css); > > } > > == > > Overhead will be seen at updating file statistics and LRU management. > > > > But, hmm, can't you do that tracking without page_cgroup ? > > Because the number of dirty/writeback pages are far smaller than total pages, > > chasing I/O with dynamic structure is not very bad.. > > > > prepareing [pfn -> blkio] record table and move that information to struct bio > > in dynamic way is very difficult ? > > This would be ok for dirty pages, but consider that we're also tracking > anonymous pages. So, if we want to control the swap IO we actually need > to save this information for a lot of pages and at the end I think we'll > basically duplicate the page_cgroup code. > swap io is always started with bio and the task/mm_struct. So, if we can record information in bio, no page tracking is required. You can record information to bio just by reading mm->owner. Thanks, -Kame _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: KAMEZAWA Hiroyuki
date: 2011-02-23T23:58:05-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: On Thu, Feb 24, 2011 at 12:14:11AM +0100, Andrea Righi wrote: > On Wed, Feb 23, 2011 at 10:23:54AM -0500, Vivek Goyal wrote: > > > > Agreed. Granularity of per inode level might be accetable in many > > > > cases. Again, I am worried faster group getting stuck behind slower > > > > group. > > > > > > > > I am wondering if we are trying to solve the problem of ASYNC write throttling > > > > at wrong layer. Should ASYNC IO be throttled before we allow task to write to > > > > page cache. The way we throttle the process based on dirty ratio, can we > > > > just check for throttle limits also there or something like that.(I think > > > > that's what you had done in your initial throttling controller implementation?) > > > > > > Right. This is exactly the same approach I've used in my old throttling > > > controller: throttle sync READs and WRITEs at the block layer and async > > > WRITEs when the task is dirtying memory pages. > > > > > > This is probably the simplest way to resolve the problem of faster group > > > getting blocked by slower group, but the controller will be a little bit > > > more leaky, because the writeback IO will be never throttled and we'll > > > see some limited IO spikes during the writeback. > > > > Yes writeback will not be throttled. Not sure how big a problem that is. > > > > - We have controlled the input rate. So that should help a bit. > > - May be one can put some high limit on root cgroup to in blkio throttle > > controller to limit overall WRITE rate of the system. > > - For SATA disks, try to use CFQ which can try to minimize the impact of > > WRITE. > > > > It will atleast provide consistent bandwindth experience to application. > > Right. > > > > > >However, this is always > > > a better solution IMHO respect to the current implementation that is > > > affected by that kind of priority inversion problem. > > > > > > I can try to add this logic to the current blk-throttle controller if > > > you think it is worth to test it. > > > > …
creator: Vivek Goyal
date: 2011-02-24T00:10:33-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: On Wed, 23 Feb 2011 19:10:33 -0500 Vivek Goyal <vgoyal@redhat.com> wrote: > On Thu, Feb 24, 2011 at 12:14:11AM +0100, Andrea Righi wrote: > > On Wed, Feb 23, 2011 at 10:23:54AM -0500, Vivek Goyal wrote: > > > > > Agreed. Granularity of per inode level might be accetable in many > > > > > cases. Again, I am worried faster group getting stuck behind slower > > > > > group. > > > > > > > > > > I am wondering if we are trying to solve the problem of ASYNC write throttling > > > > > at wrong layer. Should ASYNC IO be throttled before we allow task to write to > > > > > page cache. The way we throttle the process based on dirty ratio, can we > > > > > just check for throttle limits also there or something like that.(I think > > > > > that's what you had done in your initial throttling controller implementation?) > > > > > > > > Right. This is exactly the same approach I've used in my old throttling > > > > controller: throttle sync READs and WRITEs at the block layer and async > > > > WRITEs when the task is dirtying memory pages. > > > > > > > > This is probably the simplest way to resolve the problem of faster group > > > > getting blocked by slower group, but the controller will be a little bit > > > > more leaky, because the writeback IO will be never throttled and we'll > > > > see some limited IO spikes during the writeback. > > > > > > Yes writeback will not be throttled. Not sure how big a problem that is. > > > > > > - We have controlled the input rate. So that should help a bit. > > > - May be one can put some high limit on root cgroup to in blkio throttle > > > controller to limit overall WRITE rate of the system. > > > - For SATA disks, try to use CFQ which can try to minimize the impact of > > > WRITE. > > > > > > It will atleast provide consistent bandwindth experience to application. > > > > Right. > > > > > > > > >However, this is always > > > > a better solution IMHO respect to the current implementation that is > > > > affected by that kind of priority i…
creator: KAMEZAWA Hiroyuki
date: 2011-02-24T00:40:39-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: On Wed, Feb 23, 2011 at 4:40 PM, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 23 Feb 2011 19:10:33 -0500 > Vivek Goyal <vgoyal@redhat.com> wrote: > >> On Thu, Feb 24, 2011 at 12:14:11AM +0100, Andrea Righi wrote: >> > On Wed, Feb 23, 2011 at 10:23:54AM -0500, Vivek Goyal wrote: >> > > > > Agreed. Granularity of per inode level might be accetable in many >> > > > > cases. Again, I am worried faster group getting stuck behind slower >> > > > > group. >> > > > > >> > > > > I am wondering if we are trying to solve the problem of ASYNC write throttling >> > > > > at wrong layer. Should ASYNC IO be throttled before we allow task to write to >> > > > > page cache. The way we throttle the process based on dirty ratio, can we >> > > > > just check for throttle limits also there or something like that.(I think >> > > > > that's what you had done in your initial throttling controller implementation?) >> > > > >> > > > Right. This is exactly the same approach I've used in my old throttling >> > > > controller: throttle sync READs and WRITEs at the block layer and async >> > > > WRITEs when the task is dirtying memory pages. >> > > > >> > > > This is probably the simplest way to resolve the problem of faster group >> > > > getting blocked by slower group, but the controller will be a little bit >> > > > more leaky, because the writeback IO will be never throttled and we'll >> > > > see some limited IO spikes during the writeback. >> > > >> > > Yes writeback will not be throttled. Not sure how big a problem that is. >> > > >> > > - We have controlled the input rate. So that should help a bit. >> > > - May be one can put some high limit on root cgroup to in blkio throttle >> > > controller to limit overall WRITE rate of the system. >> > > - For SATA disks, try to use CFQ which can try to minimize the impact of >> > > WRITE. >> > > >> > > It will atleast provide consistent bandwindth experience to application. >> > >> > Right. >> > >> > > >> > > >However, thi…
creator: Greg Thelen
date: 2011-02-24T02:01:22-00:00

Re: [PATCH 0/5] blk-throttle: writeback and swap IO control
type: item
description: * Andrea Righi <arighi@develer.com> [2011-02-22 18:12:51]: > Currently the blkio.throttle controller only support synchronous IO requests. > This means that we always look at the current task to identify the "owner" of > each IO request. > > However dirty pages in the page cache can be wrote to disk asynchronously by > the per-bdi flusher kernel threads or by any other thread in the system, > according to the writeback policy. > > For this reason the real writes to the underlying block devices may > occur in a different IO context respect to the task that originally > generated the dirty pages involved in the IO operation. This makes the > tracking and throttling of writeback IO more complicate respect to the > synchronous IO from the blkio controller's perspective. > > The same concept is also valid for anonymous pages involed in IO operations > (swap). > > This patch allow to track the cgroup that originally dirtied each page in page > cache and each anonymous page and pass these informations to the blk-throttle > controller. These informations can be used to provide a better service level > differentiation of buffered writes swap IO between different cgroups. > > Testcase > ======== > - create a cgroup with 1MiB/s write limit: > # mount -t cgroup -o blkio none /mnt/cgroup > # mkdir /mnt/cgroup/foo > # echo 8:0 $((1024 * 1024)) > /mnt/cgroup/foo/blkio.throttle.write_bps_device > > - move a task into the cgroup and run a dd to generate some writeback IO > > Results: > - 2.6.38-rc6 vanilla: > $ cat /proc/$$/cgroup > 1:blkio:/foo > $ dd if=/dev/zero of=zero bs=1M count=1024 & > $ dstat -df > --dsk/sda-- > read writ > 0 19M > 0 19M > 0 0 > 0 0 > 0 19M > ... > > - 2.6.38-rc6 + blk-throttle writeback IO control: > $ cat /proc/$$/cgroup > 1:blkio:/foo > $ dd if=/dev/zero of=zero bs=1M count=1024 & > $ dstat -df > --dsk/sda-- > read writ > 0 1024 > 0 1024 > 0 1024 > 0 1024 > 0 1024 > ... > Thanks for looking into this, further review follows. -- Three Cheers, Balbir _____…
creator: Balbir Singh
date: 2011-02-24T06:08:53-00:00