OpenVZ Forum - RDF feed
type: channel
items: [-mm PATCH 0/8] Memory controller introduction (v2)
items: [-mm PATCH 1/8] Memory controller resource counters (v2)
items: [-mm PATCH 2/8] Memory controller containers setup (v2)
items: [-mm PATCH 3/8] Memory controller accounting setup (v2)
items: [-mm PATCH 4/8] Memory controller memory accounting (v2)
items: [-mm PATCH 5/8] Memory controller task migration (v2)
items: [-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v2)
items: [-mm PATCH 7/8] Memory controller OOM handling (v2)
items: [-mm PATCH 8/8] Add switch to control what type of pages to limit (v2)
items: Re: [-mm PATCH 0/8] Memory controller introduction (v2)
items: Re: [-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v2)
items: Re: [-mm PATCH 1/8] Memory controller resource counters (v2)
items: Re: [-mm PATCH 2/8] Memory controller containers setup (v2)
items: Re: [-mm PATCH 4/8] Memory controller memory accounting (v2)

[-mm PATCH 0/8] Memory controller introduction (v2)
type: item
description: Changelog since version 1 1. Fixed some compile time errors (in mm/migrate.c from Vaidyanathan S) 2. Fixed a panic seen when LIST_DEBUG is enabled 3. Added a mechanism to control whether we track page cache or both page cache and mapped pages (as requested by Pavel) This patchset implements another version of the memory controller. These patches have been through a big churn, the first set of patches were posted last year and earlier this year at http://lkml.org/lkml/2007/2/19/10 Ever since, the RSS controller has been through four revisions, the latest one being http://lwn.net/Articles/236817/ This patchset draws from the patches listed above and from some of the contents of the patches posted by Vaidyanathan for page cache control. http://lkml.org/lkml/2007/6/20/92 Pavel, Vaidy could you look at the patches and add your signed off by where relevant? At OLS, the resource management BOF, it was discussed that we need to manage RSS and unmapped page cache together. This patchset is a step towards that TODO's 1. Add memory controller water mark support. Reclaim on high water mark 2. Add support for shrinking on limit change 3. Add per zone per container LRU lists 4. Make page_referenced() container aware 5. Figure out a better CLUI for the controller In case you have been using/testing the RSS controller, you'll find that this controller works slower than the RSS controller. The reason being that both swap cache and page cache is accounted for, so pages do go out to swap upon reclaim (they cannot live in the swap cache). I've test compiled the framework without the controller enabled, tested the code on UML and minimally on a power box. Any test output, feedback, comments, suggestions are welcome! series res_counters_infra.patch mem-control-setup.patch mem-control-accounting-setup.patch mem-control-accounting.patch mem-control-task-migration.patch mem-control-lru-and-reclaim.patch mem-control-out-of-memory.patch -- Warm Regards, Balbir Singh Linux Technology Center IB…
creator: Balbir Singh
date: 2007-07-06T05:20:29-00:00

[-mm PATCH 1/8] Memory controller resource counters (v2)
type: item
description: From: Pavel Emelianov <xemul@openvz.org> Introduce generic structures and routines for resource accounting. Each resource accounting container is supposed to aggregate it, container_subsystem_state and its resource-specific members within. Signed-off-by: Pavel Emelianov <xemul@openvz.org> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- include/linux/res_counter.h | 102 +++++++++++++++++++++++++++++++++++++ init/Kconfig | 4 + kernel/Makefile | 1 kernel/res_counter.c | 121 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 228 insertions(+) diff -puN /dev/null include/linux/res_counter.h --- /dev/null 2007-06-01 08:12:04.000000000 -0700 +++ linux-2.6.22-rc6-balbir/include/linux/res_counter.h 2007-07-05 13:45:17.000000000 -0700 @@ -0,0 +1,102 @@ +#ifndef __RES_COUNTER_H__ +#define __RES_COUNTER_H__ + +/* + * resource counters + * contain common data types and routines for resource accounting + * + * Copyright 2007 OpenVZ SWsoft Inc + * + * Author: Pavel Emelianov <xemul@openvz.org> + * + */ + +#include <linux/container.h> + +/* + * the core object. the container that wishes to account for some + * resource may include this counter into its structures and use + * the helpers described beyond + */ + +struct res_counter { + /* + * the current resource consumption level + */ + unsigned long usage; + /* + * the limit that usage cannot exceed + */ + unsigned long limit; + /* + * the number of insuccessful attempts to consume the resource + */ + unsigned long failcnt; + /* + * the lock to protect all of the above. + * the routines below consider this to be IRQ-safe + */ + spinlock_t lock; +}; + +/* + * helpers to interact with userspace + * res_counter_read/_write - put/get the specified fields from the + * res_counter struct to/from the user + * + * @cnt: the counter in question + * @member: the field to work with (see RES_xxx below) + * @buf: the buffer to opeate on,... + * @nbytes: its size... + * @pos: and the offset. + */ + +ssize_t res_counter_rea…
creator: Balbir Singh
date: 2007-07-06T05:20:43-00:00

[-mm PATCH 2/8] Memory controller containers setup (v2)
type: item
description: Setup the memory container and add basic hooks and controls to integrate and work with the container. Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- include/linux/container_subsys.h | 6 + include/linux/memcontrol.h | 19 +++++ init/Kconfig | 8 ++ mm/Makefile | 1 mm/memcontrol.c | 141 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 175 insertions(+) diff -puN include/linux/container_subsys.h~mem-control-setup include/linux/container_subsys.h --- linux-2.6.22-rc6/include/linux/container_subsys.h~mem-control-setup 2007-07-05 13:45:17.000000000 -0700 +++ linux-2.6.22-rc6-balbir/include/linux/container_subsys.h 2007-07-05 13:45:17.000000000 -0700 @@ -30,3 +30,9 @@ SUBSYS(ns) #endif /* */ + +#ifdef CONFIG_CONTAINER_MEM_CONT +SUBSYS(mem_container) +#endif + +/* */ diff -puN init/Kconfig~mem-control-setup init/Kconfig --- linux-2.6.22-rc6/init/Kconfig~mem-control-setup 2007-07-05 13:45:17.000000000 -0700 +++ linux-2.6.22-rc6-balbir/init/Kconfig 2007-07-05 13:45:17.000000000 -0700 @@ -360,6 +360,14 @@ config CONTAINER_NS for instance virtual servers and checkpoint/restart jobs. +config CONTAINER_MEM_CONT + bool "Memory controller for containers" + select CONTAINERS + select RESOURCE_COUNTERS + help + Provides a memory controller that manages both page cache and + RSS memory. + config PROC_PID_CPUSET bool "Include legacy /proc/<pid>/cpuset file" depends on CPUSETS diff -puN mm/Makefile~mem-control-setup mm/Makefile --- linux-2.6.22-rc6/mm/Makefile~mem-control-setup 2007-07-05 13:45:17.000000000 -0700 +++ linux-2.6.22-rc6-balbir/mm/Makefile 2007-07-05 13:45:17.000000000 -0700 @@ -30,4 +30,5 @@ obj-$(CONFIG_FS_XIP) += filemap_xip.o obj-$(CONFIG_MIGRATION) += migrate.o obj-$(CONFIG_SMP) += allocpercpu.o obj-$(CONFIG_QUICKLIST) += quicklist.o +obj-$(CONFIG_CONTAINER_MEM_CONT) += memcontrol.o diff -puN /dev/null mm/memcontrol.c --- /dev/null 2007-06-01 08:12:04.000000000 -0700 +++ linux-2.6.22-rc6-balbir/mm/memcontrol.c 2007-07-05 13:45:17.000000000 -0700 @@ -…
creator: Balbir Singh
date: 2007-07-06T05:21:03-00:00

[-mm PATCH 3/8] Memory controller accounting setup (v2)
type: item
description: Basic setup routines, the mm_struct has a pointer to the container that it belongs to and the the page has a meta_page associated with it. Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- include/linux/memcontrol.h | 32 ++++++++++++++++++++++++++++++ include/linux/mm_types.h | 4 +++ include/linux/sched.h | 4 +++ kernel/fork.c | 10 ++++++--- mm/memcontrol.c | 47 ++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 91 insertions(+), 6 deletions(-) diff -puN include/linux/memcontrol.h~mem-control-accounting-setup include/linux/memcontrol.h --- linux-2.6.22-rc6/include/linux/memcontrol.h~mem-control-accounting-setup 2007-07-05 13:45:17.000000000 -0700 +++ linux-2.6.22-rc6-balbir/include/linux/memcontrol.h 2007-07-05 13:45:17.000000000 -0700 @@ -15,5 +15,37 @@ #ifndef _LINUX_MEMCONTROL_H #define _LINUX_MEMCONTROL_H +struct mem_container; +struct meta_page; + +#ifdef CONFIG_CONTAINER_MEM_CONT + +extern void mm_init_container(struct mm_struct *mm, struct task_struct *p); +extern void mm_free_container(struct mm_struct *mm); +extern void page_assign_meta_page(struct page *page, struct meta_page *mp); +extern struct meta_page *page_get_meta_page(struct page *page); + +#else /* CONFIG_CONTAINER_MEM_CONT */ +static inline void mm_init_container(struct mm_struct *mm, + struct task_struct *p) +{ +} + +static inline void mm_free_container(struct mm_struct *mm) +{ +} + +static inline void page_assign_meta_page(struct page *page, + struct meta_page *mp) +{ +} + +static inline struct meta_page *page_get_meta_page(struct page *page) +{ + return NULL; +} + +#endif /* CONFIG_CONTAINER_MEM_CONT */ + #endif /* _LINUX_MEMCONTROL_H */ diff -puN include/linux/mm_types.h~mem-control-accounting-setup include/linux/mm_types.h --- linux-2.6.22-rc6/include/linux/mm_types.h~mem-control-accounting-setup 2007-07-05 13:45:17.000000000 -0700 +++ linux-2.6.22-rc6-balbir/include/linux/mm_types.h 2007-07-05 13:45:17.000000000 -0700 @@ -5,6 +5,7 @@ #include <linux/threads.h> #include…
creator: Balbir Singh
date: 2007-07-06T05:21:18-00:00

[-mm PATCH 4/8] Memory controller memory accounting (v2)
type: item
description: Add the accounting hooks. The accounting is carried out for RSS and Page Cache (unmapped) pages. There is now a common limit and accounting for both. The RSS accounting is accounted at page_add_*_rmap() and page_remove_rmap() time. Page cache is accounted at add_to_page_cache(), __delete_from_page_cache(). Swap cache is also accounted for. Each page's meta_page is protected with a bit in page flags, this makes handling of race conditions involving simultaneous mappings of a page easier. A reference count is kept in the meta_page to deal with cases where a page might be unmapped from the RSS of all tasks, but still lives in the page cache. Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- fs/exec.c | 1 include/linux/memcontrol.h | 11 +++ include/linux/page-flags.h | 3 + mm/filemap.c | 8 ++ mm/memcontrol.c | 132 ++++++++++++++++++++++++++++++++++++++++++++- mm/memory.c | 22 +++++++ mm/migrate.c | 6 ++ mm/page_alloc.c | 3 + mm/rmap.c | 2 mm/swap_state.c | 8 ++ mm/swapfile.c | 40 +++++++------ 11 files changed, 218 insertions(+), 18 deletions(-) diff -puN fs/exec.c~mem-control-accounting fs/exec.c --- linux-2.6.22-rc6/fs/exec.c~mem-control-accounting 2007-07-05 13:45:18.000000000 -0700 +++ linux-2.6.22-rc6-balbir/fs/exec.c 2007-07-05 13:45:18.000000000 -0700 @@ -51,6 +51,7 @@ #include <linux/cn_proc.h> #include <linux/audit.h> #include <linux/signalfd.h> +#include <linux/memcontrol.h> #include <asm/uaccess.h> #include <asm/mmu_context.h> diff -puN include/linux/memcontrol.h~mem-control-accounting include/linux/memcontrol.h --- linux-2.6.22-rc6/include/linux/memcontrol.h~mem-control-accounting 2007-07-05 13:45:18.000000000 -0700 +++ linux-2.6.22-rc6-balbir/include/linux/memcontrol.h 2007-07-05 18:27:26.000000000 -0700 @@ -24,6 +24,8 @@ extern void mm_init_container(struct mm_ extern void mm_free_container(struct mm_struct *mm); extern void page_assign_meta_page(struct page *page, struct meta_page *mp); extern struct meta_page *page_get_meta_page(struct page *pa…
creator: Balbir Singh
date: 2007-07-06T05:21:35-00:00

[-mm PATCH 5/8] Memory controller task migration (v2)
type: item
description: Allow tasks to migrate from one container to the other. We migrate mm_struct's mem_container only when the thread group id migrates. Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- mm/memcontrol.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff -puN mm/memcontrol.c~mem-control-task-migration mm/memcontrol.c --- linux-2.6.22-rc6/mm/memcontrol.c~mem-control-task-migration 2007-07-05 13:45:18.000000000 -0700 +++ linux-2.6.22-rc6-balbir/mm/memcontrol.c 2007-07-05 13:45:18.000000000 -0700 @@ -302,11 +302,46 @@ err: return rc; } +static void mem_container_move_task(struct container_subsys *ss, + struct container *cont, + struct container *old_cont, + struct task_struct *p) +{ + struct mm_struct *mm; + struct mem_container *mem, *old_mem; + + mm = get_task_mm(p); + if (mm == NULL) + return; + + mem = mem_container_from_cont(cont); + old_mem = mem_container_from_cont(old_cont); + + if (mem == old_mem) + goto out; + + /* + * Only thread group leaders are allowed to migrate, the mm_struct is + * in effect owned by the leader + */ + if (p->tgid != p->pid) + goto out; + + css_get(&mem->css); + rcu_assign_pointer(mm->mem_container, mem); + css_put(&old_mem->css); + +out: + mmput(mm); + return; +} + struct container_subsys mem_container_subsys = { .name = "mem_container", .subsys_id = mem_container_subsys_id, .create = mem_container_create, .destroy = mem_container_destroy, .populate = mem_container_populate, + .attach = mem_container_move_task, .early_init = 1, }; _ -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-06T05:21:55-00:00

[-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v2)
type: item
description: Add the meta_page to the per container LRU. The reclaim algorithm has been modified to make the isolate_lru_pages() as a pluggable component. The scan_control data structure now accepts the container on behalf of which reclaims are carried out. try_to_free_pages() has been extended to become container aware. Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- include/linux/memcontrol.h | 11 +++ include/linux/res_counter.h | 23 ++++++++ include/linux/swap.h | 3 + mm/memcontrol.c | 121 ++++++++++++++++++++++++++++++++++++++++++ mm/swap.c | 2 mm/vmscan.c | 125 +++++++++++++++++++++++++++++++++++--------- 6 files changed, 259 insertions(+), 26 deletions(-) diff -puN include/linux/memcontrol.h~mem-control-lru-and-reclaim include/linux/memcontrol.h --- linux-2.6.22-rc6/include/linux/memcontrol.h~mem-control-lru-and-reclaim 2007-07-05 18:28:12.000000000 -0700 +++ linux-2.6.22-rc6-balbir/include/linux/memcontrol.h 2007-07-05 18:45:57.000000000 -0700 @@ -26,6 +26,13 @@ extern void page_assign_meta_page(struct extern struct meta_page *page_get_meta_page(struct page *page); extern int mem_container_charge(struct page *page, struct mm_struct *mm); extern void mem_container_uncharge(struct meta_page *mp); +extern void mem_container_move_lists(struct meta_page *mp, bool active); +extern unsigned long mem_container_isolate_pages(unsigned long nr_to_scan, + struct list_head *dst, + unsigned long *scanned, int order, + int mode, struct zone *z, + struct mem_container *mem_cont, + int active); #else /* CONFIG_CONTAINER_MEM_CONT */ static inline void mm_init_container(struct mm_struct *mm, @@ -56,6 +63,10 @@ static inline void mem_container_uncharg { } +static inline void mem_container_move_lists(struct meta_page *mp, bool active) +{ +} + #endif /* CONFIG_CONTAINER_MEM_CONT */ #endif /* _LINUX_MEMCONTROL_H */ diff -puN include/linux/swap.h~mem-control-lru-and-reclaim include/linux/swap.h --- linux-2.6.22-rc6/include/linux/swap.h~mem-control-lru-and-reclaim 2007-07-05 18:28:12.…
creator: Balbir Singh
date: 2007-07-06T05:22:12-00:00

[-mm PATCH 7/8] Memory controller OOM handling (v2)
type: item
description: Out of memory handling for containers over their limit. A task from the container over limit is chosen using the existing OOM logic and killed. TODO: 1. As discussed in the OLS BOF session, consider implementing a user space policy for OOM handling. Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- include/linux/memcontrol.h | 1 + mm/memcontrol.c | 1 + mm/oom_kill.c | 42 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 40 insertions(+), 4 deletions(-) diff -puN include/linux/memcontrol.h~mem-control-out-of-memory include/linux/memcontrol.h --- linux-2.6.22-rc6/include/linux/memcontrol.h~mem-control-out-of-memory 2007-07-05 18:49:35.000000000 -0700 +++ linux-2.6.22-rc6-balbir/include/linux/memcontrol.h 2007-07-05 18:49:35.000000000 -0700 @@ -33,6 +33,7 @@ extern unsigned long mem_container_isola int mode, struct zone *z, struct mem_container *mem_cont, int active); +extern void mem_container_out_of_memory(struct mem_container *mem); #else /* CONFIG_CONTAINER_MEM_CONT */ static inline void mm_init_container(struct mm_struct *mm, diff -puN mm/memcontrol.c~mem-control-out-of-memory mm/memcontrol.c --- linux-2.6.22-rc6/mm/memcontrol.c~mem-control-out-of-memory 2007-07-05 18:49:35.000000000 -0700 +++ linux-2.6.22-rc6-balbir/mm/memcontrol.c 2007-07-05 18:49:35.000000000 -0700 @@ -266,6 +266,7 @@ int mem_container_charge(struct page *pa if (res_counter_check_under_limit(&mem->res)) continue; + mem_container_out_of_memory(mem); goto free_mp; } diff -puN mm/oom_kill.c~mem-control-out-of-memory mm/oom_kill.c --- linux-2.6.22-rc6/mm/oom_kill.c~mem-control-out-of-memory 2007-07-05 18:49:35.000000000 -0700 +++ linux-2.6.22-rc6-balbir/mm/oom_kill.c 2007-07-05 18:49:35.000000000 -0700 @@ -24,6 +24,7 @@ #include <linux/cpuset.h> #include <linux/module.h> #include <linux/notifier.h> +#include <linux/memcontrol.h> int sysctl_panic_on_oom; /* #define DEBUG */ @@ -47,7 +48,8 @@ int sysctl_panic_on_oom; * of least surprise ... (be careful when you change it) */ -unsi…
creator: Balbir Singh
date: 2007-07-06T05:22:32-00:00

[-mm PATCH 8/8] Add switch to control what type of pages to limit (v2)
type: item
description: Choose if we want cached pages to be accounted or not. By default both are accounted for. A new set of tunables are added. echo -n 1 > mem_control_type switches the accounting to account for only mapped pages echo -n 2 > mem_control_type switches the behaviour back Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- include/linux/memcontrol.h | 9 +++ mm/filemap.c | 2 mm/memcontrol.c | 129 ++++++++++++++++++++++++++++++++++++++------- mm/swap_state.c | 2 4 files changed, 122 insertions(+), 20 deletions(-) diff -puN mm/memcontrol.c~mem-control-choose-rss-vs-rss-and-pagecache mm/memcontrol.c --- linux-2.6.22-rc6/mm/memcontrol.c~mem-control-choose-rss-vs-rss-and-pagecache 2007-07-05 20:00:07.000000000 -0700 +++ linux-2.6.22-rc6-balbir/mm/memcontrol.c 2007-07-05 20:09:40.000000000 -0700 @@ -22,6 +22,8 @@ #include <linux/swap.h> #include <linux/spinlock.h> +#include <asm/uaccess.h> + struct container_subsys mem_container_subsys; /* @@ -52,6 +54,7 @@ struct mem_container { * spin_lock to protect the per container LRU */ spinlock_t lru_lock; + unsigned long control_type; /* control RSS or RSS+Pagecache */ }; /* @@ -65,6 +68,14 @@ struct meta_page { atomic_t ref_cnt; }; +enum { + MEM_CONTAINER_TYPE_UNSPEC = 0, + MEM_CONTAINER_TYPE_MAPPED, + MEM_CONTAINER_TYPE_ALL, + MEM_CONTAINER_TYPE_MAX, +} mem_control_type; + +static struct mem_container init_mem_container; static inline struct mem_container *mem_container_from_cont(struct container *cnt) @@ -301,6 +312,22 @@ err: } /* + * See if the cached pages should be charged at all? + */ +int mem_container_cache_charge(struct page *page, struct mm_struct *mm) +{ + struct mem_container *mem; + if (!mm) + mm = &init_mm; + + mem = rcu_dereference(mm->mem_container); + if (mem->control_type & MEM_CONTAINER_TYPE_ALL) + return mem_container_charge(page, mm); + else + return 0; +} + +/* * Uncharging is always a welcome operation, we never complain, simply * uncharge. */ @@ -311,7 +338,9 @@ void mem_container_uncharge(struct meta_ u…
creator: Balbir Singh
date: 2007-07-06T05:22:53-00:00

Re: [-mm PATCH 0/8] Memory controller introduction (v2)
type: item
description: Balbir Singh wrote: > Changelog since version 1 > > 1. Fixed some compile time errors (in mm/migrate.c from Vaidyanathan S) > 2. Fixed a panic seen when LIST_DEBUG is enabled > 3. Added a mechanism to control whether we track page cache or both > page cache and mapped pages (as requested by Pavel) > Here is one combined patch for ease of testing. For those wanting to do a quick test. -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-06T05:55:53-00:00

Re: [-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v2)
type: item
description: Balbir Singh wrote: > +unsigned long mem_container_isolate_pages(unsigned long nr_to_scan, > + struct list_head *dst, > + unsigned long *scanned, int order, > + int mode, struct zone *z, > + struct mem_container *mem_cont, > + int active) [snip] > +{ > + /* > + * Check if the meta page went away from under us > + */ > + if (!list_empty(&mp->list) There is a small typo here, we need an extra brace at the end (I should have done a refpatch :( ) -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-06T07:05:12-00:00

Re: [-mm PATCH 0/8] Memory controller introduction (v2)
type: item
description: Balbir Singh wrote: In mem_container_move_lists() > + /* > + * Check if the meta page went away from under us > + */ > + if (!list_empty(&mp->list) You'll need an extra brace here to get it compile. I forgot to refpatch :( -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-06T07:09:00-00:00

Re: [-mm PATCH 1/8] Memory controller resource counters (v2)
type: item
description: On Thu, 2007-07-05 at 22:20 -0700, Balbir Singh wrote: > +/* > + * the core object. the container that wishes to account for some > + * resource may include this counter into its structures and use > + * the helpers described beyond > + */ I'm going to nitpick a bit here. Nothing major, I promise. ;) Could we make these comments into nice sentences with capitalization? I think it makes them easier to read in long comments. How about something like this for the comment: /* * A container wishing to account for a resource should include this * structure into one of its own. It may use the helpers below. */ The one above is worded a little bit strangely. > +struct res_counter { > + /* > + * the current resource consumption level > + */ > + unsigned long usage; > + /* > + * the limit that usage cannot exceed > + */ > + unsigned long limit; > + /* > + * the number of insuccessful attempts to consume the resource > + */ unsuccessful > + unsigned long failcnt; > + /* > + * the lock to protect all of the above. > + * the routines below consider this to be IRQ-safe > + */ > + spinlock_t lock; > +}; Do we really need all of these comments? Some of them are a wee bit self-explanatory. I think we mostly know what a limit is. ;) > +/* > + * helpers to interact with userspace > + * res_counter_read/_write - put/get the specified fields from the > + * res_counter struct to/from the user > + * > + * @cnt: the counter in question > + * @member: the field to work with (see RES_xxx below) > + * @buf: the buffer to opeate on,... > + * @nbytes: its size... > + * @pos: and the offset. > + */ > + > +ssize_t res_counter_read(struct res_counter *cnt, int member, > + const char __user *buf, size_t nbytes, loff_t *pos); > +ssize_t res_counter_write(struct res_counter *cnt, int member, > + const char __user *buf, size_t nbytes, loff_t *pos); > + > +/* > + * the field descriptors. one for each member of res_counter > + */ > + > +enum { > + RES_USAGE, > + RES_LIMIT, > + RES_FAILCNT, > +}; > + > +…
creator: Dave Hansen
date: 2007-07-06T17:24:01-00:00

Re: [-mm PATCH 2/8] Memory controller containers setup (v2)
type: item
description: On Thu, 2007-07-05 at 22:21 -0700, Balbir Singh wrote: > +struct mem_container { > + struct container_subsys_state css; > + /* > + * the counter to account for memory usage > + */ > + struct res_counter res; > +}; How about we call it "memory_usage"? That would kill two birds with one stone: get rid of the comment, and keep people from needing to refer to the comment to figure out what "res" *IS*. > +/* > + * A meta page is associated with every page descriptor. The meta page > + * helps us identify information about the container > + */ > +struct meta_page { > + struct list_head list; /* per container LRU list */ > + struct page *page; > + struct mem_container *mem_container; > +}; Why not just rename "list" to "lru_list" or "container_lru"? > + > +static inline struct mem_container *mem_container_from_cont(struct container > + *cnt) I'd probably break that line up differently: static inline struct mem_container *mem_container_from_cont(struct container *cnt) BTW, do I see "cnt" meaning "container" now instead of "cnt"? ;) Is somebody's favorite dog named "cnt" and you're just trying to remind yourself of them as often as possible? -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-07-06T17:30:09-00:00

Re: [-mm PATCH 1/8] Memory controller resource counters (v2)
type: item
description: Dave Hansen wrote: > On Thu, 2007-07-05 at 22:20 -0700, Balbir Singh wrote: >> +/* >> + * the core object. the container that wishes to account for some >> + * resource may include this counter into its structures and use >> + * the helpers described beyond >> + */ > > I'm going to nitpick a bit here. Nothing major, I promise. ;) > > Could we make these comments into nice sentences with capitalization? I > think it makes them easier to read in long comments. > > How about something like this for the comment: > > /* > * A container wishing to account for a resource should include this > * structure into one of its own. It may use the helpers below. > */ > > The one above is worded a little bit strangely. > Hi, Dave, These patches were posted by Pavel, I've carried them forward as is. Suggestions are always welcome. >> +struct res_counter { >> + /* >> + * the current resource consumption level >> + */ >> + unsigned long usage; >> + /* >> + * the limit that usage cannot exceed >> + */ >> + unsigned long limit; >> + /* >> + * the number of insuccessful attempts to consume the resource >> + */ > > unsuccessful > Thanks, fixed. >> + unsigned long failcnt; >> + /* >> + * the lock to protect all of the above. >> + * the routines below consider this to be IRQ-safe >> + */ >> + spinlock_t lock; >> +}; > > Do we really need all of these comments? Some of them are a wee bit > self-explanatory. I think we mostly know what a limit is. ;) > I'll leave the decision on the comments exclusion to Pavel. > > More nitpicking... > > Can we leave the normal control flow in the lowest indentation level, > and have only errors in the indented if(){} blocks? Something like > this: > Sounds good, done! >> +int res_counter_charge_locked(struct res_counter *cnt, unsigned long > val) >> +{ >> + if (cnt->usage > cnt->limit - val) { >> + cnt->failcnt++; >> + return -ENOMEM; >> + } >> + cnt->usage += val; >> + return 0; >> +} > > Also, can you do my poor brain a favor an expand "cnt" to "counter"? …
creator: Balbir Singh
date: 2007-07-06T21:03:58-00:00

Re: [-mm PATCH 2/8] Memory controller containers setup (v2)
type: item
description: Dave Hansen wrote: > On Thu, 2007-07-05 at 22:21 -0700, Balbir Singh wrote: >> +struct mem_container { >> + struct container_subsys_state css; >> + /* >> + * the counter to account for memory usage >> + */ >> + struct res_counter res; >> +}; > > How about we call it "memory_usage"? That would kill two birds with one > stone: get rid of the comment, and keep people from needing to refer to > the comment to figure out what "res" *IS*. > Hmm.. res is the closest to resource counter. res_cnt is confusing. res is a generic resource definition to indicate that we are dealing with generic resource counters. >> +/* >> + * A meta page is associated with every page descriptor. The meta page >> + * helps us identify information about the container >> + */ >> +struct meta_page { >> + struct list_head list; /* per container LRU list */ >> + struct page *page; >> + struct mem_container *mem_container; >> +}; > > Why not just rename "list" to "lru_list" or "container_lru"? > I think just lru might be fine, meta_page->lru == container LRU. >> + >> +static inline struct mem_container *mem_container_from_cont(struct container >> + *cnt) > > I'd probably break that line up differently: > > static inline > struct mem_container *mem_container_from_cont(struct container *cnt) > Yes, that's better. > BTW, do I see "cnt" meaning "container" now instead of "cnt"? ;) > Nope, I'll fix it to be cont > Is somebody's favorite dog named "cnt" and you're just trying to remind > yourself of them as often as possible? > It's an easy shorthands to use, like i or ref, num. > -- Dave > -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-06T21:07:19-00:00

Re: [-mm PATCH 1/8] Memory controller resource counters (v2)
type: item
description: On Fri, 2007-07-06 at 14:03 -0700, Balbir Singh wrote: > > >> +ssize_t res_counter_read(struct res_counter *cnt, int member, > >> + const char __user *userbuf, size_t nbytes, loff_t > *pos) > >> +{ > >> + unsigned long *val; > >> + char buf[64], *s; > >> + > >> + s = buf; > >> + val = res_counter_member(cnt, member); > >> + s += sprintf(s, "%lu\n", *val); > >> + return simple_read_from_buffer((void __user *)userbuf, nbytes, > >> + pos, buf, s - buf); > >> +} > > > > Why do we need that cast? > > > > u mean the __user? If I remember correctly it's a attribute for > sparse. The userbuf is already __user. This just appears to be making a 'const char *' into a 'void *'. I wondered what the reason for that part is. -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-07-06T21:10:05-00:00

Re: [-mm PATCH 1/8] Memory controller resource counters (v2)
type: item
description: Dave Hansen wrote: > On Fri, 2007-07-06 at 14:03 -0700, Balbir Singh wrote: >>>> +ssize_t res_counter_read(struct res_counter *cnt, int member, >>>> + const char __user *userbuf, size_t nbytes, loff_t >> *pos) >>>> +{ >>>> + unsigned long *val; >>>> + char buf[64], *s; >>>> + >>>> + s = buf; >>>> + val = res_counter_member(cnt, member); >>>> + s += sprintf(s, "%lu\n", *val); >>>> + return simple_read_from_buffer((void __user *)userbuf, nbytes, >>>> + pos, buf, s - buf); >>>> +} >>> Why do we need that cast? >>> >> u mean the __user? If I remember correctly it's a attribute for >> sparse. > > The userbuf is already __user. This just appears to be making a 'const > char *' into a 'void *'. I wondered what the reason for that part is. > Aah.. yes.. good point. I'll look into it. > -- Dave > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: email@kvack.org -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-06T21:24:00-00:00

Re: [-mm PATCH 1/8] Memory controller resource counters (v2)
type: item
description: Dave Hansen wrote: > On Thu, 2007-07-05 at 22:20 -0700, Balbir Singh wrote: >> +/* >> + * the core object. the container that wishes to account for some >> + * resource may include this counter into its structures and use >> + * the helpers described beyond >> + */ > > I'm going to nitpick a bit here. Nothing major, I promise. ;) > > Could we make these comments into nice sentences with capitalization? I > think it makes them easier to read in long comments. > > How about something like this for the comment: > > /* > * A container wishing to account for a resource should include this > * structure into one of its own. It may use the helpers below. > */ > > The one above is worded a little bit strangely. > >> +struct res_counter { >> + /* >> + * the current resource consumption level >> + */ >> + unsigned long usage; >> + /* >> + * the limit that usage cannot exceed >> + */ >> + unsigned long limit; >> + /* >> + * the number of insuccessful attempts to consume the resource >> + */ > > unsuccessful > >> + unsigned long failcnt; >> + /* >> + * the lock to protect all of the above. >> + * the routines below consider this to be IRQ-safe >> + */ >> + spinlock_t lock; >> +}; > > Do we really need all of these comments? Some of them are a wee bit > self-explanatory. I think we mostly know what a limit is. ;) Since this is a new entities in the kernel and not many people deal with the resource management, I think that nothing bad in having them. page->_count, signal_struct->shared_pending, mm_struct->mm_users and others do not bother anyone with their comments either. >> +/* >> + * helpers to interact with userspace >> + * res_counter_read/_write - put/get the specified fields from the >> + * res_counter struct to/from the user >> + * >> + * @cnt: the counter in question >> + * @member: the field to work with (see RES_xxx below) >> + * @buf: the buffer to opeate on,... >> + * @nbytes: its size... >> + * @pos: and the offset. >> + */ >> + >> +ssize_t res_counter_read(struct r…
creator: Pavel Emelianov
date: 2007-07-09T07:16:24-00:00

Re: [-mm PATCH 1/8] Memory controller resource counters (v2)
type: item
description: On Mon, 2007-07-09 at 11:16 +0400, Pavel Emelianov wrote: > Dave Hansen wrote: > > On Thu, 2007-07-05 at 22:20 -0700, Balbir Singh wrote: > >> +/* > >> + * the core object. the container that wishes to account for some > >> + * resource may include this counter into its structures and use > >> + * the helpers described beyond > >> + */ > > > > I'm going to nitpick a bit here. Nothing major, I promise. ;) > > > > Could we make these comments into nice sentences with capitalization? I > > think it makes them easier to read in long comments. > > > > How about something like this for the comment: > > > > /* > > * A container wishing to account for a resource should include this > > * structure into one of its own. It may use the helpers below. > > */ > > > > The one above is worded a little bit strangely. > > > >> +struct res_counter { > >> + /* > >> + * the current resource consumption level > >> + */ > >> + unsigned long usage; > >> + /* > >> + * the limit that usage cannot exceed > >> + */ > >> + unsigned long limit; > >> + /* > >> + * the number of insuccessful attempts to consume the resource > >> + */ > > > > unsuccessful > > > >> + unsigned long failcnt; > >> + /* > >> + * the lock to protect all of the above. > >> + * the routines below consider this to be IRQ-safe > >> + */ > >> + spinlock_t lock; > >> +}; > > > > Do we really need all of these comments? Some of them are a wee bit > > self-explanatory. I think we mostly know what a limit is. ;) > > Since this is a new entities in the kernel and not many people > deal with the resource management, I think that nothing bad in > having them. They waste space. It makes the code harder to read. > >> +/* > >> + * helpers to interact with userspace > >> + * res_counter_read/_write - put/get the specified fields from the > >> + * res_counter struct to/from the user > >> + * > >> + * @cnt: the counter in question > >> + * @member: the field to work with (see RES_xxx below) > >> + * @buf: the buffer to opeate on,... > >>…
creator: Dave Hansen
date: 2007-07-09T19:56:48-00:00

Re: [-mm PATCH 4/8] Memory controller memory accounting (v2)
type: item
description: hi, > diff -puN mm/memory.c~mem-control-accounting mm/memory.c > --- linux-2.6.22-rc6/mm/memory.c~mem-control-accounting 2007-07-05 13:45:18.000000000 -0700 > +++ linux-2.6.22-rc6-balbir/mm/memory.c 2007-07-05 13:45:18.000000000 -0700 > @@ -1731,6 +1736,9 @@ gotten: > cow_user_page(new_page, old_page, address, vma); > } > > + if (mem_container_charge(new_page, mm)) > + goto oom; > + > /* > * Re-check the pte - we dropped the lock > */ it seems that the page will be leaked on error. > @@ -2188,6 +2196,11 @@ static int do_swap_page(struct mm_struct > } > > delayacct_clear_flag(DELAYACCT_PF_SWAPIN); > + if (mem_container_charge(page, mm)) { > + ret = VM_FAULT_OOM; > + goto out; > + } > + > mark_page_accessed(page); > lock_page(page); > ditto. > @@ -2264,6 +2278,9 @@ static int do_anonymous_page(struct mm_s > if (!page) > goto oom; > > + if (mem_container_charge(page, mm)) > + goto oom; > + > entry = mk_pte(page, vma->vm_page_prot); > entry = maybe_mkwrite(pte_mkdirty(entry), vma); > ditto. can you check the rest of the patch by yourself? thanks. YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2007-07-10T07:26:51-00:00

Re: [-mm PATCH 4/8] Memory controller memory accounting (v2)
type: item
description: On 7/10/07, YAMAMOTO Takashi <yamamoto@valinux.co.jp> wrote: > hi, > > > diff -puN mm/memory.c~mem-control-accounting mm/memory.c > > --- linux-2.6.22-rc6/mm/memory.c~mem-control-accounting 2007-07-05 13:45:18.000000000 -0700 > > +++ linux-2.6.22-rc6-balbir/mm/memory.c 2007-07-05 13:45:18.000000000 -0700 > > > @@ -1731,6 +1736,9 @@ gotten: > > cow_user_page(new_page, old_page, address, vma); > > } > > > > + if (mem_container_charge(new_page, mm)) > > + goto oom; > > + > > /* > > * Re-check the pte - we dropped the lock > > */ > > it seems that the page will be leaked on error. You mean meta_page right? > > > @@ -2188,6 +2196,11 @@ static int do_swap_page(struct mm_struct > > } > > > > delayacct_clear_flag(DELAYACCT_PF_SWAPIN); > > + if (mem_container_charge(page, mm)) { > > + ret = VM_FAULT_OOM; > > + goto out; > > + } > > + > > mark_page_accessed(page); > > lock_page(page); > > > > ditto. > > > @@ -2264,6 +2278,9 @@ static int do_anonymous_page(struct mm_s > > if (!page) > > goto oom; > > > > + if (mem_container_charge(page, mm)) > > + goto oom; > > + > > entry = mk_pte(page, vma->vm_page_prot); > > entry = maybe_mkwrite(pte_mkdirty(entry), vma); > > > > ditto. > > can you check the rest of the patch by yourself? thanks. > Excellent catch! I'll review the accounting framework and post the updated version soon Balbir _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-10T08:41:18-00:00

Re: [-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v2)
type: item
description: > Add the meta_page to the per container LRU. The reclaim algorithm has been > modified to make the isolate_lru_pages() as a pluggable component. The > scan_control data structure now accepts the container on behalf of which > reclaims are carried out. try_to_free_pages() has been extended to become > container aware. > > Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> it seems that the number of pages to scan (nr_active/nr_inactive in shrink_zone) is calculated from NR_ACTIVE and NR_INACTIVE of the zone, even in the case of per-container reclaim. is it intended? YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2007-07-10T08:41:53-00:00

Re: [-mm PATCH 4/8] Memory controller memory accounting (v2)
type: item
description: > On 7/10/07, YAMAMOTO Takashi <yamamoto@valinux.co.jp> wrote: > > hi, > > > > > diff -puN mm/memory.c~mem-control-accounting mm/memory.c > > > --- linux-2.6.22-rc6/mm/memory.c~mem-control-accounting 2007-07-05 13:45:18.000000000 -0700 > > > +++ linux-2.6.22-rc6-balbir/mm/memory.c 2007-07-05 13:45:18.000000000 -0700 > > > > > @@ -1731,6 +1736,9 @@ gotten: > > > cow_user_page(new_page, old_page, address, vma); > > > } > > > > > > + if (mem_container_charge(new_page, mm)) > > > + goto oom; > > > + > > > /* > > > * Re-check the pte - we dropped the lock > > > */ > > > > it seems that the page will be leaked on error. > > You mean meta_page right? no. i meant 'new_page'. YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2007-07-10T08:44:27-00:00

Re: [-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v2)
type: item
description: YAMAMOTO Takashi wrote: >> Add the meta_page to the per container LRU. The reclaim algorithm has been >> modified to make the isolate_lru_pages() as a pluggable component. The >> scan_control data structure now accepts the container on behalf of which >> reclaims are carried out. try_to_free_pages() has been extended to become >> container aware. >> >> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> > > it seems that the number of pages to scan (nr_active/nr_inactive > in shrink_zone) is calculated from NR_ACTIVE and NR_INACTIVE of the zone, > even in the case of per-container reclaim. is it intended? > > YAMAMOTO Takashi Good catch again! We do that for now since the per zone LRU is a superset of the container LRU. I see this as an important TODO item for us -- to move to reclaim statistics based on per container nr_active and nr_inactive (to be added). -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-10T15:38:59-00:00

Re: [-mm PATCH 4/8] Memory controller memory accounting (v2)
type: item
description: YAMAMOTO Takashi wrote: >> On 7/10/07, YAMAMOTO Takashi <yamamoto@valinux.co.jp> wrote: >>> hi, >>> >>>> diff -puN mm/memory.c~mem-control-accounting mm/memory.c >>>> --- linux-2.6.22-rc6/mm/memory.c~mem-control-accounting 2007-07-05 13:45:18.000000000 -0700 >>>> +++ linux-2.6.22-rc6-balbir/mm/memory.c 2007-07-05 13:45:18.000000000 -0700 >>>> @@ -1731,6 +1736,9 @@ gotten: >>>> cow_user_page(new_page, old_page, address, vma); >>>> } >>>> >>>> + if (mem_container_charge(new_page, mm)) >>>> + goto oom; >>>> + >>>> /* >>>> * Re-check the pte - we dropped the lock >>>> */ >>> it seems that the page will be leaked on error. >> You mean meta_page right? > > no. i meant 'new_page'. > > YAMAMOTO Takashi Yes, I see. Thanks for clarifying. -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-07-10T15:42:05-00:00