OpenVZ Forum - RDF feed
type: channel
items: [RFC][PATCH][0/4] Memory controller (RSS Control)
items: [RFC][PATCH][1/4] RSS controller setup
items: [RFC][PATCH][2/4] Add RSS accounting and control
items: [RFC][PATCH][3/4] Add reclaim support
items: [RFC][PATCH][4/4] RSS controller documentation
items: Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
items: Re: [RFC][PATCH][1/4] RSS controller setup
items: Re: [RFC][PATCH][2/4] Add RSS accounting and control
items: Re: [RFC][PATCH][3/4] Add reclaim support
items: Re: [ckrm-tech] [RFC][PATCH][0/4] Memory controller (RSS Control)

[RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: This patch applies on top of Paul Menage's container patches (V7) posted at http://lkml.org/lkml/2007/2/12/88 It implements a controller within the containers framework for limiting memory usage (RSS usage). The memory controller was discussed at length in the RFC posted to lkml http://lkml.org/lkml/2006/10/30/51 Steps to use the controller -------------------------- 0. Download the patches, apply the patches 1. Turn on CONFIG_CONTAINER_MEMCTLR in kernel config, build the kernel and boot into the new kernel 2. mount -t container container -o memctlr /<mount point> 3. cd /<mount point> optionally do (mkdir <directory>; cd <directory>) under /<mount point> 4. echo $$ > tasks (attaches the current shell to the container) 5. echo -n (limit value) > memctlr_limit 6. cat memctlr_usage 7. Run tasks, check the usage of the controller, reclaim behaviour 8. Report bugs, get bug fixes and iterate (goto step 0). Advantages of the patchset -------------------------- 1. Zero overhead in struct page (struct page is not expanded) 2. Minimal changes to the core-mm code 3. Shared pages are not reclaimed unless all mappings belong to overlimit containers. 4. It can be used to debug drivers/applications/kernel components in a constrained memory environment (similar to mem=XXX option), except that several containers can be created simultaneously without rebooting and the limits can be changed. NOTE: There is no support for limiting kernel memory allocations and page cache control (presently). Testing ------- Ran kernbench and lmbench with containers enabled (container filesystem not mounted), they seemed to run fine Created containers, attached tasks to containers with lower limits than the memory the tasks require (memory hog tests) and ran some basic tests on them TODO's and improvement areas ---------------------------- 1. Come up with cool page replacement algorithms for containers (if possible without any changes to struct page) 2. Add page cache control 3. Add kernel memory alloca…
creator: Balbir Singh
date: 2007-02-19T06:50:19-00:00

[RFC][PATCH][1/4] RSS controller setup
type: item
description: This patch sets up the basic controller infrastructure on top of the containers infrastructure. Two files are provided for monitoring and control memctlr_usage and memctlr_limit. memctlr_usage shows the current usage (in pages, of RSS) and the limit set by the user. memctlr_limit can be used to set a limit on the RSS usage of the resource. A special value of 0, indicates that the usage is unlimited. The limit is set in units of pages. Signed-off-by: <balbir@in.ibm.com> --- include/linux/memctlr.h | 22 ++++++ init/Kconfig | 7 + mm/Makefile | 1 mm/memctlr.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+) diff -puN /dev/null include/linux/memctlr.h --- /dev/null 2007-02-02 22:51:23.000000000 +0530 +++ linux-2.6.20-balbir/include/linux/memctlr.h 2007-02-16 00:22:11.000000000 +0530 @@ -0,0 +1,22 @@ +/* memctlr.h - Memory Controller for containers + * + * Copyright (C) Balbir Singh, IBM Corp. 2006-2007 + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2.1 of the GNU Lesser General Public License + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it would be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + */ + +#ifndef _LINUX_MEMCTLR_H +#define _LINUX_MEMCTLR_H + +#ifdef CONFIG_CONTAINER_MEMCTLR + +#else /* CONFIG_CONTAINER_MEMCTLR */ + +#endif /* CONFIG_CONTAINER_MEMCTLR */ +#endif /* _LINUX_MEMCTLR_H */ diff -puN init/Kconfig~memctlr-setup init/Kconfig --- linux-2.6.20/init/Kconfig~memctlr-setup 2007-02-15 21:58:42.000000000 +0530 +++ linux-2.6.20-balbir/init/Kconfig 2007-02-15 21:58:42.000000000 +0530 @@ -306,6 +306,13 @@ config CONTAINER_NS for instance virtual servers and checkpoint/restart jobs. +config CONTAINER_MEMCTLR + bool "A simple RSS based memory controller" + select CONTAINERS + help + Provides a simple Resource Controlle…
creator: Balbir Singh
date: 2007-02-19T06:50:26-00:00

[RFC][PATCH][2/4] Add RSS accounting and control
type: item
description: This patch adds the basic accounting hooks to account for pages allocated into the RSS of a process. Accounting is maintained at two levels, in the mm_struct of each task and in the memory controller data structure associated with each node in the container. When the limit specified for the container is exceeded, the task is killed. RSS accounting is consistent with the current definition of RSS in the kernel. Shared pages are accounted into the RSS of each process as is done in the kernel currently. The code is flexible in that it can be easily modified to work with any definition of RSS. Signed-off-by: <balbir@in.ibm.com> --- fs/exec.c | 4 + include/linux/memctlr.h | 38 ++++++++++++ include/linux/sched.h | 11 +++ kernel/fork.c | 10 +++ mm/memctlr.c | 148 ++++++++++++++++++++++++++++++++++++++++++------ mm/memory.c | 33 +++++++++- mm/rmap.c | 5 + mm/swapfile.c | 2 8 files changed, 232 insertions(+), 19 deletions(-) diff -puN fs/exec.c~memctlr-acct fs/exec.c --- linux-2.6.20/fs/exec.c~memctlr-acct 2007-02-18 22:55:50.000000000 +0530 +++ linux-2.6.20-balbir/fs/exec.c 2007-02-18 22:55:50.000000000 +0530 @@ -50,6 +50,7 @@ #include <linux/tsacct_kern.h> #include <linux/cn_proc.h> #include <linux/audit.h> +#include <linux/memctlr.h> #include <asm/uaccess.h> #include <asm/mmu_context.h> @@ -313,6 +314,9 @@ void install_arg_page(struct vm_area_str if (unlikely(anon_vma_prepare(vma))) goto out; + if (!memctlr_update_rss(mm, 1, MEMCTLR_CHECK_LIMIT)) + goto out; + flush_dcache_page(page); pte = get_locked_pte(mm, address, &ptl); if (!pte) diff -puN include/linux/memctlr.h~memctlr-acct include/linux/memctlr.h --- linux-2.6.20/include/linux/memctlr.h~memctlr-acct 2007-02-18 22:55:50.000000000 +0530 +++ linux-2.6.20-balbir/include/linux/memctlr.h 2007-02-18 23:28:16.000000000 +0530 @@ -14,9 +14,47 @@ #ifndef _LINUX_MEMCTLR_H #define _LINUX_MEMCTLR_H +enum { + MEMCTLR_CHECK_LIMIT = true, + MEMCTLR_DONT_CHECK_LIMIT = false, +}; + #ifdef CONFIG_CONTAINER_MEMCTLR +struct res_counter…
creator: Balbir Singh
date: 2007-02-19T06:50:34-00:00

[RFC][PATCH][3/4] Add reclaim support
type: item
description: This patch reclaims pages from a container when the container limit is hit. The executable is oom'ed only when the container it is running in, is overlimit and we could not reclaim any pages belonging to the container A parameter called pushback, controls how much memory is reclaimed when the limit is hit. It should be easy to expose this knob to user space, but currently it is hard coded to 20% of the total limit of the container. isolate_lru_pages() has been modified to isolate pages belonging to a particular container, so that reclaim code will reclaim only container pages. For shared pages, reclaim does not unmap all mappings of the page, it only unmaps those mappings that are over their limit. This ensures that other containers are not penalized while reclaiming shared pages. Parallel reclaim per container is not allowed. Each controller has a wait queue that ensures that only one task per control is running reclaim on that container. Signed-off-by: <balbir@in.ibm.com> --- include/linux/memctlr.h | 8 ++ include/linux/rmap.h | 13 +++- include/linux/swap.h | 4 + mm/memctlr.c | 137 ++++++++++++++++++++++++++++++++++++++++++++---- mm/migrate.c | 2 mm/rmap.c | 96 +++++++++++++++++++++++++++++++-- mm/vmscan.c | 94 ++++++++++++++++++++++++++++---- 7 files changed, 324 insertions(+), 30 deletions(-) diff -puN include/linux/memctlr.h~memctlr-reclaim-on-limit include/linux/memctlr.h --- linux-2.6.20/include/linux/memctlr.h~memctlr-reclaim-on-limi t 2007-02-18 23:29:14.000000000 +0530 +++ linux-2.6.20-balbir/include/linux/memctlr.h 2007-02-18 23:29:14.000000000 +0530 @@ -20,6 +20,7 @@ enum { }; #ifdef CONFIG_CONTAINER_MEMCTLR +#include <linux/wait.h> struct res_counter { atomic_long_t usage; /* The current usage of the resource being */ @@ -33,6 +34,9 @@ extern void memctlr_mm_free(struct mm_st extern void memctlr_mm_assign_container(struct mm_struct *mm, struct task_struct *p); extern int memctlr_update_rss(struct mm_struct *mm, int count, bool check); +extern int memctl…
creator: Balbir Singh
date: 2007-02-19T06:50:42-00:00

[RFC][PATCH][4/4] RSS controller documentation
type: item
description: Signed-off-by: <balbir@in.ibm.com> --- Documentation/memctlr.txt | 70 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff -puN /dev/null Documentation/memctlr.txt --- /dev/null 2007-02-02 22:51:23.000000000 +0530 +++ linux-2.6.20-balbir/Documentation/memctlr.txt 2007-02-19 00:51:44.000000000 +0530 @@ -0,0 +1,70 @@ +Introduction +------------ + +The memory controller is a controller module written under the containers +framework. It can be used to limit the resource usage of a group of +tasks grouped by the container. + +Accounting +---------- + +The memory controller tracks the RSS usage of the tasks in the container. +The definition of RSS was debated on lkml in the following thread + + http://lkml.org/lkml/2006/10/10/130 + +This patch is flexible, it is easy to adapt the patch to any definition +of RSS. The current accounting is based on the current definition of +RSS. Each page mapped is charged to the container. + +The accounting is done at two levels, each process has RSS accounting in +the mm_struct and in the container it belongs to. The mm_struct accounting +is used when a task switches (migrates to a different) container(s). The +accounting information for the task is subtracted from the source container +and added to the destination container. If as result of the migration, the +destination container goes over limit, no action is taken until some task +in the destination container runs and tries to map a new page in its +page table. + +The current RSS usage can be seen in the memctlr_usage file. The value +is in units of pages. + +Control +------- + +The memctlr_limit file allows the user to set a limit on the number of +pages that can be mapped by the processes in the container. A special +value of 0 (which is the default limit of any new container), indicates +that the container can use unlimited amount of RSS. + +Reclaim +------- + +When the limit set in the container is hit, the memory controller starts +reclaiming page…
creator: Balbir Singh
date: 2007-02-19T06:50:50-00:00

Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: On Mon, 19 Feb 2007 12:20:19 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > This patch applies on top of Paul Menage's container patches (V7) posted at > > http://lkml.org/lkml/2007/2/12/88 > > It implements a controller within the containers framework for limiting > memory usage (RSS usage). It's good to see someone building on someone else's work for once, rather than everyone going off in different directions. It makes one hope that we might actually achieve something at last. The key part of this patchset is the reclaim algorithm: > @@ -636,6 +642,15 @@ static unsigned long isolate_lru_pages(u > > list_del(&page->lru); > target = src; > + /* > + * For containers, do not scan the page unless it > + * belongs to the container we are reclaiming for > + */ > + if (container && !page_in_container(page, zone, container)) { > + scan--; > + goto done; > + } Alas, I fear this might have quite bad worst-case behaviour. One small container which is under constant memory pressure will churn the system-wide LRUs like mad, and will consume rather a lot of system time. So it's a point at which container A can deleteriously affect things which are running in other containers, which is exactly what we're supposed to not do.
creator: Andrew Morton
date: 2007-02-19T08:54:41-00:00

Re: [RFC][PATCH][1/4] RSS controller setup
type: item
description: On Mon, 19 Feb 2007 12:20:26 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > > This patch sets up the basic controller infrastructure on top of the > containers infrastructure. Two files are provided for monitoring > and control memctlr_usage and memctlr_limit. The patches use the identifier "memctlr" a lot. It is hard to remember, and unpronounceable. Something like memcontrol or mem_controller or memory_controller would be more typical. > ... > > + BUG_ON(!mem); > + if ((buffer = kmalloc(nbytes + 1, GFP_KERNEL)) == 0) > + return -ENOMEM; Please prefer to do buffer = kmalloc(nbytes + 1, GFP_KERNEL); if (buffer == NULL) reutrn -ENOMEM; ie: avoid the assign-and-test-in-the-same-statement thing. This affects the whole patchset. Also, please don't compare pointers to literal zero like that. It makes me get buried it patches to convert it to "NULL". I think this is a sparse thing. > + buffer[nbytes] = 0; > + if (copy_from_user(buffer, userbuf, nbytes)) { > + ret = -EFAULT; > + goto out_err; > + } > + > + container_manage_lock(); > + if (container_is_removed(cont)) { > + ret = -ENODEV; > + goto out_unlock; > + } > + > + limit = simple_strtoul(buffer, NULL, 10); > + /* > + * 0 is a valid limit (unlimited resource usage) > + */ > + if (!limit && strcmp(buffer, "0")) > + goto out_unlock; > + > + spin_lock(&mem->lock); > + mem->counter.limit = limit; > + spin_unlock(&mem->lock); The patches do this a lot: a single atomic assignment with a pointless-looking lock/unlock around it. It's often the case that this idiom indicates a bug, or needless locking. I think the only case where it makes sense is when there's some other code somewhere which is doing spin_lock(&mem->lock); ... use1(mem->counter.limit); ... use2(mem->counter.limit); ... spin_unlock(&mem->lock); where use1() and use2() expect the two reads of mem->counter.limit to return the same value. Is that the case in these patches? If not, we might have a problem in there. > + > +static ssize_t memctlr_read(struct contain…
creator: Andrew Morton
date: 2007-02-19T08:57:27-00:00

Re: [RFC][PATCH][2/4] Add RSS accounting and control
type: item
description: On Mon, 19 Feb 2007 12:20:34 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > > This patch adds the basic accounting hooks to account for pages allocated > into the RSS of a process. Accounting is maintained at two levels, in > the mm_struct of each task and in the memory controller data structure > associated with each node in the container. > > When the limit specified for the container is exceeded, the task is killed. > RSS accounting is consistent with the current definition of RSS in the > kernel. Shared pages are accounted into the RSS of each process as is > done in the kernel currently. The code is flexible in that it can be easily > modified to work with any definition of RSS. > > .. > > +static inline int memctlr_mm_init(struct mm_struct *mm) > +{ > + return 0; > +} So it returns zero on success. OK. > --- linux-2.6.20/kernel/fork.c~memctlr-acct 2007-02-18 22:55:50.000000000 +0530 > +++ linux-2.6.20-balbir/kernel/fork.c 2007-02-18 22:55:50.000000000 +0530 > @@ -50,6 +50,7 @@ > #include <linux/taskstats_kern.h> > #include <linux/random.h> > #include <linux/numtasks.h> > +#include <linux/memctlr.h> > > #include <asm/pgtable.h> > #include <asm/pgalloc.h> > @@ -342,10 +343,15 @@ static struct mm_struct * mm_init(struct > mm->free_area_cache = TASK_UNMAPPED_BASE; > mm->cached_hole_size = ~0UL; > > + if (!memctlr_mm_init(mm)) > + goto err; > + But here we treat zero as an error? > if (likely(!mm_alloc_pgd(mm))) { > mm->def_flags = 0; > return mm; > } > + > +err: > free_mm(mm); > return NULL; > } > > ... > > +int memctlr_mm_init(struct mm_struct *mm) > +{ > + mm->counter = kmalloc(sizeof(struct res_counter), GFP_KERNEL); > + if (!mm->counter) > + return 0; > + atomic_long_set(&mm->counter->usage, 0); > + atomic_long_set(&mm->counter->limit, 0); > + rwlock_init(&mm->container_lock); > + return 1; > +} ah-ha, we have another Documentation/SubmitChecklist customer. It would be more conventional to make this return -EFOO on error, zero on success. > +void memctlr_mm_f…
creator: Andrew Morton
date: 2007-02-19T08:58:28-00:00

Re: [RFC][PATCH][3/4] Add reclaim support
type: item
description: On Mon, 19 Feb 2007 12:20:42 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > > This patch reclaims pages from a container when the container limit is hit. > The executable is oom'ed only when the container it is running in, is overlimit > and we could not reclaim any pages belonging to the container > > A parameter called pushback, controls how much memory is reclaimed when the > limit is hit. It should be easy to expose this knob to user space, but > currently it is hard coded to 20% of the total limit of the container. > > isolate_lru_pages() has been modified to isolate pages belonging to a > particular container, so that reclaim code will reclaim only container > pages. For shared pages, reclaim does not unmap all mappings of the page, > it only unmaps those mappings that are over their limit. This ensures > that other containers are not penalized while reclaiming shared pages. > > Parallel reclaim per container is not allowed. Each controller has a wait > queue that ensures that only one task per control is running reclaim on > that container. > > > ... > > --- linux-2.6.20/include/linux/rmap.h~memctlr-reclaim-on-limit 2007-02-18 23:29:14.000000000 +0530 > +++ linux-2.6.20-balbir/include/linux/rmap.h 2007-02-18 23:29:14.000000000 +0530 > @@ -90,7 +90,15 @@ static inline void page_dup_rmap(struct > * Called from mm/vmscan.c to handle paging out > */ > int page_referenced(struct page *, int is_locked); > -int try_to_unmap(struct page *, int ignore_refs); > +int try_to_unmap(struct page *, int ignore_refs, void *container); > +#ifdef CONFIG_CONTAINER_MEMCTLR > +bool page_in_container(struct page *page, struct zone *zone, void *container); > +#else > +static inline bool page_in_container(struct page *page, struct zone *zone, void *container) > +{ > + return true; > +} > +#endif /* CONFIG_CONTAINER_MEMCTLR */ > > /* > * Called from mm/filemap_xip.c to unmap empty zero page > @@ -118,7 +126,8 @@ int page_mkclean(struct page *); > #define anon_vma_link(vma) do {} whil…
creator: Andrew Morton
date: 2007-02-19T08:59:12-00:00

Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: > > Alas, I fear this might have quite bad worst-case behaviour. One small > container which is under constant memory pressure will churn the > system-wide LRUs like mad, and will consume rather a lot of system time. > So it's a point at which container A can deleteriously affect things which > are running in other containers, which is exactly what we're supposed to > not do. I think it's OK for a container to consume lots of system time during reclaim, as long as we can account that time to the container involved (i.e. if it's done during direct reclaim rather than by something like kswapd). Churning the LRU could well be bad though, I agree. Paul
creator: Paul Menage
date: 2007-02-19T09:06:10-00:00

Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: > On Mon, 19 Feb 2007 12:20:19 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > > > This patch applies on top of Paul Menage's container patches (V7) posted at > > > > http://lkml.org/lkml/2007/2/12/88 > > > > It implements a controller within the containers framework for limiting > > memory usage (RSS usage). > The key part of this patchset is the reclaim algorithm: > > Alas, I fear this might have quite bad worst-case behaviour. One small > container which is under constant memory pressure will churn the > system-wide LRUs like mad, and will consume rather a lot of system time. > So it's a point at which container A can deleteriously affect things which > are running in other containers, which is exactly what we're supposed to > not do. Nice with a simple memory controller. The downside seems to be that it doesn't scale very well when it comes to reclaim, but maybe that just comes with being simple. Step by step, and maybe this is a good first step? Ideally I'd like to see unmapped pages handled on a per-container LRU with a fallback to the system-wide LRUs. Shared/mapped pages could be handled using PTE ageing/unmapping instead of page ageing, but that may consume too much resources to be practical. / magnus
creator: Magnus Damm
date: 2007-02-19T09:16:42-00:00

Re: [RFC][PATCH][1/4] RSS controller setup
type: item
description: On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: > > This output is hard to parse and to extend. I'd suggest either two > separate files, or multi-line output: > > usage: %lu kB > limit: %lu kB Two separate files would be the container usage model that I envisaged, inherited from the way cpusets does things. And in this case, it should definitely be the limit in one file, readable and writeable, and the usage in another, probably only readable. Having to read a file called memctlr_usage to find the current limit sounds wrong. Hmm, I don't appear to have documented this yet, but I think a good naming scheme for container files is <subsystem>.<whatever> - i.e. these should be memctlr.usage and memctlr.limit. The existing grandfathered Cpusets names violate this, but I'm not sure there's a lot we can do about that. > > +static int memctlr_populate(struct container_subsys *ss, > > + struct container *cont) > > +{ > > + int rc; > > + if ((rc = container_add_file(cont, &memctlr_usage)) < 0) > > + return rc; > > + if ((rc = container_add_file(cont, &memctlr_limit)) < 0) > > Clean up the first file here? Containers don't currently provide an API for a subsystem to clean up files from a directory - that's done automatically when the directory is deleted. I think I'll probably change the API for container_add_file to return void, but mark an error in the container itself if something goes wrong - that way rather than all the subsystems having to check for error, container_populate_dir() can do so at the end of calling all the subsystems' populate methods. Paul
creator: Paul Menage
date: 2007-02-19T09:18:55-00:00

Re: [ckrm-tech] [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: > On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: > >>Alas, I fear this might have quite bad worst-case behaviour. One small >>container which is under constant memory pressure will churn the >>system-wide LRUs like mad, and will consume rather a lot of system time. >>So it's a point at which container A can deleteriously affect things which >>are running in other containers, which is exactly what we're supposed to >>not do. > > > I think it's OK for a container to consume lots of system time during > reclaim, as long as we can account that time to the container involved > (i.e. if it's done during direct reclaim rather than by something like > kswapd). hmm, is it ok to scan 100Gb of RAM for 10MB RAM container? in UBC patch set we used page beancounters to track containter pages. This allows to make efficient scan contoler and reclamation. Thanks, Kirill
creator: dev
date: 2007-02-19T09:46:15-00:00

Re: [RFC][PATCH][3/4] Add reclaim support
type: item
description: On Mon, 19 Feb 2007 12:20:42 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > +int memctlr_mm_overlimit(struct mm_struct *mm, void *sc_cont) > +{ > + struct container *cont; > + struct memctlr *mem; > + long usage, limit; > + int ret = 1; > + > + if (!sc_cont) > + goto out; > + > + read_lock(&mm->container_lock); > + cont = mm->container; > +out: > + read_unlock(&mm->container_lock); > + return ret; > +} > + should be == out_and_unlock: read_unlock(&mm->container_lock); out_: return ret; -Kame
creator: KAMEZAWA Hiroyuki
date: 2007-02-19T09:48:39-00:00

Re: [ckrm-tech] [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: On 2/19/07, Kirill Korotaev <dev@sw.ru> wrote: > > > > I think it's OK for a container to consume lots of system time during > > reclaim, as long as we can account that time to the container involved > > (i.e. if it's done during direct reclaim rather than by something like > > kswapd). > hmm, is it ok to scan 100Gb of RAM for 10MB RAM container? > in UBC patch set we used page beancounters to track containter pages. > This allows to make efficient scan contoler and reclamation. I don't mean that we shouldn't go for the most efficient method that's practical. If we can do reclaim without spinning across so much of the LRU, then that's obviously better. But if the best approach in the general case results in a process in the container spending lots of CPU time trying to do the reclaim, that's probably OK as long as we can account for that time and (once we have a CPU controller) throttle back the container in that case. So then, a container can only hurt itself by thrashing/reclaiming, rather than hurting other containers. (LRU churn notwithstanding ...) Paul
creator: Paul Menage
date: 2007-02-19T09:50:22-00:00

Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: Andrew Morton wrote: > On Mon, 19 Feb 2007 12:20:19 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > >> This patch applies on top of Paul Menage's container patches (V7) posted at >> >> http://lkml.org/lkml/2007/2/12/88 >> >> It implements a controller within the containers framework for limiting >> memory usage (RSS usage). > > It's good to see someone building on someone else's work for once, rather > than everyone going off in different directions. It makes one hope that we > might actually achieve something at last. > Thanks! It's good to know we are headed in the right direction. > > The key part of this patchset is the reclaim algorithm: > >> @@ -636,6 +642,15 @@ static unsigned long isolate_lru_pages(u >> >> list_del(&page->lru); >> target = src; >> + /* >> + * For containers, do not scan the page unless it >> + * belongs to the container we are reclaiming for >> + */ >> + if (container && !page_in_container(page, zone, container)) { >> + scan--; >> + goto done; >> + } > > Alas, I fear this might have quite bad worst-case behaviour. One small > container which is under constant memory pressure will churn the > system-wide LRUs like mad, and will consume rather a lot of system time. > So it's a point at which container A can deleteriously affect things which > are running in other containers, which is exactly what we're supposed to > not do. > Hmm.. I guess it's space vs time then :-) A CPU controller could control how much time is spent reclaiming ;) Coming back, I see the problem you mentioned and we have been thinking of several possible solutions. In my introduction I pointed out "Come up with cool page replacement algorithms for containers (if possible without any changes to struct page)" The solutions we have looked at are 1. Overload the LRU list_head in struct page to have a global LRU + a per container LRU +------+ prev +------+ | page +--------------------->| page +--------- | 0 |<---------------------+ 1 |<-------- +------+ next +------+ Global LRU +…
creator: Balbir Singh
date: 2007-02-19T10:00:39-00:00

Re: [RFC][PATCH][1/4] RSS controller setup
type: item
description: Andrew Morton wrote: > On Mon, 19 Feb 2007 12:20:26 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > >> This patch sets up the basic controller infrastructure on top of the >> containers infrastructure. Two files are provided for monitoring >> and control memctlr_usage and memctlr_limit. > > The patches use the identifier "memctlr" a lot. It is hard to remember, > and unpronounceable. Something like memcontrol or mem_controller or > memory_controller would be more typical. > I'll change the name to memory_controller >> ... >> >> + BUG_ON(!mem); >> + if ((buffer = kmalloc(nbytes + 1, GFP_KERNEL)) == 0) >> + return -ENOMEM; > > Please prefer to do > > buffer = kmalloc(nbytes + 1, GFP_KERNEL); > if (buffer == NULL) > reutrn -ENOMEM; > > ie: avoid the assign-and-test-in-the-same-statement thing. This affects > the whole patchset. > I'll fix that > Also, please don't compare pointers to literal zero like that. It makes me > get buried it patches to convert it to "NULL". I think this is a sparse > thing. > Good point, I'll fix it. >> + buffer[nbytes] = 0; >> + if (copy_from_user(buffer, userbuf, nbytes)) { >> + ret = -EFAULT; >> + goto out_err; >> + } >> + >> + container_manage_lock(); >> + if (container_is_removed(cont)) { >> + ret = -ENODEV; >> + goto out_unlock; >> + } >> + >> + limit = simple_strtoul(buffer, NULL, 10); >> + /* >> + * 0 is a valid limit (unlimited resource usage) >> + */ >> + if (!limit && strcmp(buffer, "0")) >> + goto out_unlock; >> + >> + spin_lock(&mem->lock); >> + mem->counter.limit = limit; >> + spin_unlock(&mem->lock); > > The patches do this a lot: a single atomic assignment with a > pointless-looking lock/unlock around it. It's often the case that this > idiom indicates a bug, or needless locking. I think the only case where it > makes sense is when there's some other code somewhere which is doing > > spin_lock(&mem->lock); > ... > use1(mem->counter.limit); > ... > use2(mem->counter.limit); > ... > spin_unlock(&mem->lock); > > where use1() and …
creator: Balbir Singh
date: 2007-02-19T10:06:49-00:00

Re: [ckrm-tech] [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: Kirill Korotaev wrote: >> On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: >> >>> Alas, I fear this might have quite bad worst-case behaviour. One small >>> container which is under constant memory pressure will churn the >>> system-wide LRUs like mad, and will consume rather a lot of system time. >>> So it's a point at which container A can deleteriously affect things which >>> are running in other containers, which is exactly what we're supposed to >>> not do. >> >> I think it's OK for a container to consume lots of system time during >> reclaim, as long as we can account that time to the container involved >> (i.e. if it's done during direct reclaim rather than by something like >> kswapd). > hmm, is it ok to scan 100Gb of RAM for 10MB RAM container? > in UBC patch set we used page beancounters to track containter pages. > This allows to make efficient scan contoler and reclamation. > > Thanks, > Kirill Hi, Kirill, Yes, that's a problem, but I think it's a problem that can be solved in steps. First step, add reclaim. Second step, optimize reclaim. -- Warm Regards, Balbir Singh
creator: Balbir Singh
date: 2007-02-19T10:24:07-00:00

Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: Paul Menage wrote: > On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: >> >> Alas, I fear this might have quite bad worst-case behaviour. One small >> container which is under constant memory pressure will churn the >> system-wide LRUs like mad, and will consume rather a lot of system time. >> So it's a point at which container A can deleteriously affect things >> which >> are running in other containers, which is exactly what we're supposed to >> not do. > > I think it's OK for a container to consume lots of system time during > reclaim, as long as we can account that time to the container involved > (i.e. if it's done during direct reclaim rather than by something like > kswapd). > > Churning the LRU could well be bad though, I agree. > I completely agree with you on reclaim consuming time. Churning the LRU can be avoided by the means I mentioned before 1. Add a container pointer (per page struct), it is also useful for the page cache controller 2. Check if the page belongs to a particular container before the list_del(&page->lru), so that those pages can be skipped. 3. Use a double LRU list by overloading the lru list_head of struct page. > Paul > -- Warm Regards, Balbir Singh
creator: Balbir Singh
date: 2007-02-19T10:39:26-00:00

Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: Magnus Damm wrote: > On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: >> On Mon, 19 Feb 2007 12:20:19 +0530 Balbir Singh <balbir@in.ibm.com> >> wrote: >> >> > This patch applies on top of Paul Menage's container patches (V7) >> posted at >> > >> > http://lkml.org/lkml/2007/2/12/88 >> > >> > It implements a controller within the containers framework for limiting >> > memory usage (RSS usage). > >> The key part of this patchset is the reclaim algorithm: >> >> Alas, I fear this might have quite bad worst-case behaviour. One small >> container which is under constant memory pressure will churn the >> system-wide LRUs like mad, and will consume rather a lot of system time. >> So it's a point at which container A can deleteriously affect things >> which >> are running in other containers, which is exactly what we're supposed to >> not do. > > Nice with a simple memory controller. The downside seems to be that it > doesn't scale very well when it comes to reclaim, but maybe that just > comes with being simple. Step by step, and maybe this is a good first > step? > Thanks, I totally agree. > Ideally I'd like to see unmapped pages handled on a per-container LRU > with a fallback to the system-wide LRUs. Shared/mapped pages could be > handled using PTE ageing/unmapping instead of page ageing, but that > may consume too much resources to be practical. > > / magnus Keeping unmapped pages per container sounds interesting. I am not quite sure what PTE ageing, will it look it up. -- Warm Regards, Balbir Singh
creator: Balbir Singh
date: 2007-02-19T10:45:01-00:00

Re: [RFC][PATCH][3/4] Add reclaim support
type: item
description: Andrew Morton wrote: > On Mon, 19 Feb 2007 12:20:42 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > >> This patch reclaims pages from a container when the container limit is hit. >> The executable is oom'ed only when the container it is running in, is overlimit >> and we could not reclaim any pages belonging to the container >> >> A parameter called pushback, controls how much memory is reclaimed when the >> limit is hit. It should be easy to expose this knob to user space, but >> currently it is hard coded to 20% of the total limit of the container. >> >> isolate_lru_pages() has been modified to isolate pages belonging to a >> particular container, so that reclaim code will reclaim only container >> pages. For shared pages, reclaim does not unmap all mappings of the page, >> it only unmaps those mappings that are over their limit. This ensures >> that other containers are not penalized while reclaiming shared pages. >> >> Parallel reclaim per container is not allowed. Each controller has a wait >> queue that ensures that only one task per control is running reclaim on >> that container. >> >> >> ... >> >> --- linux-2.6.20/include/linux/rmap.h~memctlr-reclaim-on-limit 2007-02-18 23:29:14.000000000 +0530 >> +++ linux-2.6.20-balbir/include/linux/rmap.h 2007-02-18 23:29:14.000000000 +0530 >> @@ -90,7 +90,15 @@ static inline void page_dup_rmap(struct >> * Called from mm/vmscan.c to handle paging out >> */ >> int page_referenced(struct page *, int is_locked); >> -int try_to_unmap(struct page *, int ignore_refs); >> +int try_to_unmap(struct page *, int ignore_refs, void *container); >> +#ifdef CONFIG_CONTAINER_MEMCTLR >> +bool page_in_container(struct page *page, struct zone *zone, void *container); >> +#else >> +static inline bool page_in_container(struct page *page, struct zone *zone, void *container) >> +{ >> + return true; >> +} >> +#endif /* CONFIG_CONTAINER_MEMCTLR */ >> >> /* >> * Called from mm/filemap_xip.c to unmap empty zero page >> @@ -118,7 +126,8 @@ int page…
creator: Balbir Singh
date: 2007-02-19T10:50:53-00:00

Re: [RFC][PATCH][3/4] Add reclaim support
type: item
description: KAMEZAWA Hiroyuki wrote: > On Mon, 19 Feb 2007 12:20:42 +0530 > Balbir Singh <balbir@in.ibm.com> wrote: > >> +int memctlr_mm_overlimit(struct mm_struct *mm, void *sc_cont) >> +{ >> + struct container *cont; >> + struct memctlr *mem; >> + long usage, limit; >> + int ret = 1; >> + >> + if (!sc_cont) >> + goto out; >> + >> + read_lock(&mm->container_lock); >> + cont = mm->container; > >> +out: >> + read_unlock(&mm->container_lock); >> + return ret; >> +} >> + > > should be > == > out_and_unlock: > read_unlock(&mm->container_lock); > out_: > return ret; > Thanks, that's a much convention! > > -Kame > -- Warm Regards, Balbir Singh
creator: Balbir Singh
date: 2007-02-19T10:52:35-00:00

Re: [RFC][PATCH][3/4] Add reclaim support
type: item
description: On Mon, 19 Feb 2007 16:20:53 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > >> + * so, is the container over it's limit. Returns 1 if the container is above > >> + * its limit. > >> + */ > >> +int memctlr_mm_overlimit(struct mm_struct *mm, void *sc_cont) > >> +{ > >> + struct container *cont; > >> + struct memctlr *mem; > >> + long usage, limit; > >> + int ret = 1; > >> + > >> + if (!sc_cont) > >> + goto out; > >> + > >> + read_lock(&mm->container_lock); > >> + cont = mm->container; > >> + > >> + /* > >> + * Regular reclaim, let it proceed as usual > >> + */ > >> + if (!sc_cont) > >> + goto out; > >> + > >> + ret = 0; > >> + if (cont != sc_cont) > >> + goto out; > >> + > >> + mem = memctlr_from_cont(cont); > >> + usage = atomic_long_read(&mem->counter.usage); > >> + limit = atomic_long_read(&mem->counter.limit); > >> + if (limit && (usage > limit)) > >> + ret = 1; > >> +out: > >> + read_unlock(&mm->container_lock); > >> + return ret; > >> +} > > > > hm, I wonder how much additional lock traffic all this adds. > > > > It's a read_lock() and most of the locks are read_locks > which allow for concurrent access, until the container > changes or goes away read_lock isn't free, and I suspect we're calling this function pretty often (every pagefault?) It'll be measurable on some workloads, on some hardware. It probably won't be terribly bad because each lock-taking is associated with a clear_page(). But still, if there's any possibility of lightening the locking up, now is the time to think about it. > >> @@ -66,6 +67,9 @@ struct scan_control { > >> int swappiness; > >> > >> int all_unreclaimable; > >> + > >> + void *container; /* Used by containers for reclaiming */ > >> + /* pages when the limit is exceeded */ > >> }; > > > > eww. Why void*? > > > > I did not want to expose struct container in mm/vmscan.c. It's already there, via rmap.h > An additional > thought was that no matter what container goes in the field would be > useful for reclaim. Am having trouble parsing …
creator: Andrew Morton
date: 2007-02-19T11:10:17-00:00

Re: [RFC][PATCH][1/4] RSS controller setup
type: item
description: Paul Menage wrote: > On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: >> >> This output is hard to parse and to extend. I'd suggest either two >> separate files, or multi-line output: >> >> usage: %lu kB >> limit: %lu kB > > Two separate files would be the container usage model that I > envisaged, inherited from the way cpusets does things. > > And in this case, it should definitely be the limit in one file, > readable and writeable, and the usage in another, probably only > readable. > > Having to read a file called memctlr_usage to find the current limit > sounds wrong. > That sound right, I'll fix this. > Hmm, I don't appear to have documented this yet, but I think a good > naming scheme for container files is <subsystem>.<whatever> - i.e. > these should be memctlr.usage and memctlr.limit. The existing > grandfathered Cpusets names violate this, but I'm not sure there's a > lot we can do about that. > Why <subsystem>.<whatever>, dots are harder to parse using regular expressions and sound DOS'ish. I'd prefer "_" to separate the subsystem and whatever :-) >> > +static int memctlr_populate(struct container_subsys *ss, >> > + struct container *cont) >> > +{ >> > + int rc; >> > + if ((rc = container_add_file(cont, &memctlr_usage)) < 0) >> > + return rc; >> > + if ((rc = container_add_file(cont, &memctlr_limit)) < 0) >> >> Clean up the first file here? > > Containers don't currently provide an API for a subsystem to clean up > files from a directory - that's done automatically when the directory > is deleted. > > I think I'll probably change the API for container_add_file to return > void, but mark an error in the container itself if something goes > wrong - that way rather than all the subsystems having to check for > error, container_populate_dir() can do so at the end of calling all > the subsystems' populate methods. > It should be easy to add container_remove_file() instead of marking an error. > Paul -- Warm Regards, Balbir Singh
creator: Balbir Singh
date: 2007-02-19T11:13:24-00:00

Re: [RFC][PATCH][3/4] Add reclaim support
type: item
description: Andrew Morton wrote: > On Mon, 19 Feb 2007 16:20:53 +0530 Balbir Singh <balbir@in.ibm.com> wrote: > >>>> + * so, is the container over it's limit. Returns 1 if the container is above >>>> + * its limit. >>>> + */ >>>> +int memctlr_mm_overlimit(struct mm_struct *mm, void *sc_cont) >>>> +{ >>>> + struct container *cont; >>>> + struct memctlr *mem; >>>> + long usage, limit; >>>> + int ret = 1; >>>> + >>>> + if (!sc_cont) >>>> + goto out; >>>> + >>>> + read_lock(&mm->container_lock); >>>> + cont = mm->container; >>>> + >>>> + /* >>>> + * Regular reclaim, let it proceed as usual >>>> + */ >>>> + if (!sc_cont) >>>> + goto out; >>>> + >>>> + ret = 0; >>>> + if (cont != sc_cont) >>>> + goto out; >>>> + >>>> + mem = memctlr_from_cont(cont); >>>> + usage = atomic_long_read(&mem->counter.usage); >>>> + limit = atomic_long_read(&mem->counter.limit); >>>> + if (limit && (usage > limit)) >>>> + ret = 1; >>>> +out: >>>> + read_unlock(&mm->container_lock); >>>> + return ret; >>>> +} >>> hm, I wonder how much additional lock traffic all this adds. >>> >> It's a read_lock() and most of the locks are read_locks >> which allow for concurrent access, until the container >> changes or goes away > > read_lock isn't free, and I suspect we're calling this function pretty > often (every pagefault?) It'll be measurable on some workloads, on some > hardware. > > It probably won't be terribly bad because each lock-taking is associated > with a clear_page(). But still, if there's any possibility of lightening > the locking up, now is the time to think about it. > Yes, good point. I'll revisit to see if barriers can replace the locking or if the locking is required at all? >>>> @@ -66,6 +67,9 @@ struct scan_control { >>>> int swappiness; >>>> >>>> int all_unreclaimable; >>>> + >>>> + void *container; /* Used by containers for reclaiming */ >>>> + /* pages when the limit is exceeded */ >>>> }; >>> eww. Why void*? >>> >> I did not want to expose struct container in mm/vmscan.c. > > It's already the…
creator: Balbir Singh
date: 2007-02-19T11:16:33-00:00

Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: On 2/19/07, Balbir Singh <balbir@in.ibm.com> wrote: > Magnus Damm wrote: > > On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: > >> On Mon, 19 Feb 2007 12:20:19 +0530 Balbir Singh <balbir@in.ibm.com> > >> wrote: > >> > >> > This patch applies on top of Paul Menage's container patches (V7) > >> posted at > >> > > >> > http://lkml.org/lkml/2007/2/12/88 > >> > > >> > It implements a controller within the containers framework for limiting > >> > memory usage (RSS usage). > > > >> The key part of this patchset is the reclaim algorithm: > >> > >> Alas, I fear this might have quite bad worst-case behaviour. One small > >> container which is under constant memory pressure will churn the > >> system-wide LRUs like mad, and will consume rather a lot of system time. > >> So it's a point at which container A can deleteriously affect things > >> which > >> are running in other containers, which is exactly what we're supposed to > >> not do. > > > > Nice with a simple memory controller. The downside seems to be that it > > doesn't scale very well when it comes to reclaim, but maybe that just > > comes with being simple. Step by step, and maybe this is a good first > > step? > > > > Thanks, I totally agree. > > > Ideally I'd like to see unmapped pages handled on a per-container LRU > > with a fallback to the system-wide LRUs. Shared/mapped pages could be > > handled using PTE ageing/unmapping instead of page ageing, but that > > may consume too much resources to be practical. > > > > / magnus > > Keeping unmapped pages per container sounds interesting. I am not quite > sure what PTE ageing, will it look it up. You will most likely have no luck looking it up, so here is what I mean by PTE ageing: The most common unit for memory resource control seems to be physical pages. Keeping track of pages is simple in the case of a single user per page, but for shared pages tracking the owner becomes more complex. I consider unmapped pages to only have a single user at a time, so t…
creator: Magnus Damm
date: 2007-02-19T11:56:06-00:00

Re: [RFC][PATCH][0/4] Memory controller (RSS Control)
type: item
description: Magnus Damm wrote: > On 2/19/07, Balbir Singh <balbir@in.ibm.com> wrote: >> Magnus Damm wrote: >> > On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: >> >> On Mon, 19 Feb 2007 12:20:19 +0530 Balbir Singh <balbir@in.ibm.com> >> >> wrote: >> >> >> >> > This patch applies on top of Paul Menage's container patches (V7) >> >> posted at >> >> > >> >> > http://lkml.org/lkml/2007/2/12/88 >> >> > >> >> > It implements a controller within the containers framework for >> limiting >> >> > memory usage (RSS usage). >> > >> >> The key part of this patchset is the reclaim algorithm: >> >> >> >> Alas, I fear this might have quite bad worst-case behaviour. One >> small >> >> container which is under constant memory pressure will churn the >> >> system-wide LRUs like mad, and will consume rather a lot of system >> time. >> >> So it's a point at which container A can deleteriously affect things >> >> which >> >> are running in other containers, which is exactly what we're >> supposed to >> >> not do. >> > >> > Nice with a simple memory controller. The downside seems to be that it >> > doesn't scale very well when it comes to reclaim, but maybe that just >> > comes with being simple. Step by step, and maybe this is a good first >> > step? >> > >> >> Thanks, I totally agree. >> >> > Ideally I'd like to see unmapped pages handled on a per-container LRU >> > with a fallback to the system-wide LRUs. Shared/mapped pages could be >> > handled using PTE ageing/unmapping instead of page ageing, but that >> > may consume too much resources to be practical. >> > >> > / magnus >> >> Keeping unmapped pages per container sounds interesting. I am not quite >> sure what PTE ageing, will it look it up. > > You will most likely have no luck looking it up, so here is what I > mean by PTE ageing: > > The most common unit for memory resource control seems to be physical > pages. Keeping track of pages is simple in the case of a single user > per page, but for shared pages tracking the owner bec…
creator: Balbir Singh
date: 2007-02-19T14:07:07-00:00

Re: [RFC][PATCH][1/4] RSS controller setup
type: item
description: On Mon, 2007-02-19 at 16:43 +0530, Balbir Singh wrote: > Paul Menage wrote: > > On 2/19/07, Andrew Morton <akpm@linux-foundation.org> wrote: <snip> > > Hmm, I don't appear to have documented this yet, but I think a good > > naming scheme for container files is <subsystem>.<whatever> - i.e. > > these should be memctlr.usage and memctlr.limit. The existing > > grandfathered Cpusets names violate this, but I'm not sure there's a > > lot we can do about that. > > > > Why <subsystem>.<whatever>, dots are harder to parse using regular > expressions and sound DOS'ish. I'd prefer "_" to separate the > subsystem and whatever :-) "_" is useful for names with "spaces". Names like mem_controller. "." seems reasonable despite its regex nastyness. Alternatively there's always ":". <snip> Cheers, -Matt Helsley
creator: Matt Helsley
date: 2007-02-19T19:43:46-00:00