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

[RFC][-mm PATCH 0/8] Memory controller introduction (v3)
type: item
description: Changelog since version 2 1. Improved error handling in mm/memory.c (spotted by YAMAMOTO Takashi) 2. Test results included 3. try_to_free_mem_container_pages() bug fix (sc->may_writepage is now set to !laptop_mode) 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 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 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 (this is being actively worked on by Pavel Emelianov) 4. Make page_referenced() container aware 5. Figure out a better CLUI for the controller 6. Add better statistics 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 minimally on a power box. Any test output, feedback, comments, suggestions are welcome! NOTE: We use css_put(), container API. This API is going to change and soon become non-blocking. If you face any problem running this container, please do report it. We expect the new changes to …
creator: Balbir Singh
date: 2007-07-20T08:23:52-00:00

[RFC][-mm PATCH 1/8] Memory controller resource counters (v3)
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@linux.vnet.ibm.com> --- include/linux/res_counter.h | 102 +++++++++++++++++++++++++++++++++++++ init/Kconfig | 4 + kernel/Makefile | 1 kernel/res_counter.c | 120 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 227 insertions(+) diff -puN /dev/null include/linux/res_counter.h --- /dev/null 2007-06-01 20:42:04.000000000 +0530 +++ linux-2.6.22-rc6-balbir/include/linux/res_counter.h 2007-07-20 13:13:24.000000000 +0530 @@ -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 unsuccessful 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 + * + * @counter: 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 …
creator: Balbir Singh
date: 2007-07-20T08:24:03-00:00

[RFC][-mm PATCH 2/8] Memory controller containers setup (v3)
type: item
description: Setup the memory container and add basic hooks and controls to integrate and work with the container. Signed-off-by: <balbir@linux.vnet.ibm.com> --- include/linux/container_subsys.h | 6 + include/linux/memcontrol.h | 21 +++++ init/Kconfig | 8 ++ mm/Makefile | 1 mm/memcontrol.c | 143 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 179 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-20 13:13:24.000000000 +0530 +++ linux-2.6.22-rc6-balbir/include/linux/container_subsys.h 2007-07-20 13:13:24.000000000 +0530 @@ -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-20 13:13:24.000000000 +0530 +++ linux-2.6.22-rc6-balbir/init/Kconfig 2007-07-20 13:13:24.000000000 +0530 @@ -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-20 13:13:24.000000000 +0530 +++ linux-2.6.22-rc6-balbir/mm/Makefile 2007-07-20 13:13:24.000000000 +0530 @@ -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 20:42:04.000000000 +0530 +++ linux-2.6.22-rc6-balbir/mm/memcontrol.c 2007-07-20 13:13:24.000000000 +0530 @@ -0,0 +1,143 @@…
creator: Balbir Singh
date: 2007-07-20T08:24:16-00:00

[RFC][-mm PATCH 3/8] Memory controller accounting setup (v3)
type: item
description: From: Pavel Emelianov <xemul@openvz.org> 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: Pavel Emelianov <xemul@openvz.org> Signed-off-by: <balbir@linux.vnet.ibm.com> --- include/linux/memcontrol.h | 35 +++++++++++++++++++++++++++++++ include/linux/mm_types.h | 4 +++ include/linux/sched.h | 4 +++ kernel/fork.c | 10 ++++++--- mm/memcontrol.c | 50 ++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 97 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-20 13:13:25.000000000 +0530 +++ linux-2.6.22-rc6-balbir/include/linux/memcontrol.h 2007-07-20 13:13:25.000000000 +0530 @@ -3,6 +3,9 @@ * Copyright IBM Corporation, 2007 * Author Balbir Singh <balbir@linux.vnet.ibm.com> * + * Copyright 2007 OpenVZ SWsoft Inc + * Author: Pavel Emelianov <xemul@openvz.org> + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or @@ -17,5 +20,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_pag…
creator: Balbir Singh
date: 2007-07-20T08:24:29-00:00

[RFC][-mm PATCH 4/8] Memory controller memory accounting (v3)
type: item
description: Changelog 1. Improved error handling, uncharge on errors and check to see if we are leaking pages (review by YAMAMOTO Takashi) 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@linux.vnet.ibm.com> --- include/linux/memcontrol.h | 11 +++ include/linux/page-flags.h | 3 + mm/filemap.c | 12 +++- mm/memcontrol.c | 134 ++++++++++++++++++++++++++++++++++++++++++++- mm/memory.c | 44 +++++++++++++- mm/migrate.c | 6 ++ mm/page_alloc.c | 3 + mm/rmap.c | 2 mm/swap_state.c | 12 +++- mm/swapfile.c | 40 +++++++------ 10 files changed, 243 insertions(+), 24 deletions(-) diff -puN fs/exec.c~mem-control-accounting fs/exec.c 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-20 13:13:25.000000000 +0530 +++ linux-2.6.22-rc6-balbir/include/linux/memcontrol.h 2007-07-20 13:13:25.000000000 +0530 @@ -29,6 +29,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 *page); +extern int mem_container_charge(struct page *page, struct mm_struct *mm); +extern void mem_container_uncharge(struct meta_page *mp); #else /* CONFIG_CONTAINER_MEM_CONT */ static inline void mm_init_container(struct mm_struct *mm…
creator: Balbir Singh
date: 2007-07-20T08:24:40-00:00

[RFC][-mm PATCH 5/8] Memory controller task migration (v3)
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@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-20 13:13:25.000000000 +0530 +++ linux-2.6.22-rc6-balbir/mm/memcontrol.c 2007-07-20 13:13:25.000000000 +0530 @@ -309,11 +309,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-20T08:24:54-00:00

[RFC][-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v3)
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: Pavel Emelianov <xemul@openvz.org> Signed-off-by: <balbir@linux.vnet.ibm.com> --- include/linux/memcontrol.h | 11 +++ include/linux/res_counter.h | 23 ++++++++ include/linux/swap.h | 3 + mm/memcontrol.c | 120 +++++++++++++++++++++++++++++++++++++++++ mm/swap.c | 2 mm/vmscan.c | 126 +++++++++++++++++++++++++++++++++++--------- 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-20 13:13:25.000000000 +0530 +++ linux-2.6.22-rc6-balbir/include/linux/memcontrol.h 2007-07-20 13:13:25.000000000 +0530 @@ -31,6 +31,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, @@ -61,6 +68,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-…
creator: Balbir Singh
date: 2007-07-20T08:25:04-00:00

[RFC][-mm PATCH 7/8] Memory controller OOM handling (v3)
type: item
description: From: Pavel Emelianov <xemul@openvz.org> 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: Pavel Emelianov <xemul@openvz.org> Signed-off-by: <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-20 13:13:26.000000000 +0530 +++ linux-2.6.22-rc6-balbir/include/linux/memcontrol.h 2007-07-20 13:13:26.000000000 +0530 @@ -38,6 +38,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-20 13:13:26.000000000 +0530 +++ linux-2.6.22-rc6-balbir/mm/memcontrol.c 2007-07-20 13:13:26.000000000 +0530 @@ -272,6 +272,7 @@ int mem_container_charge(struct page *pa continue; css_put(&mem->css); + 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-20 13:13:26.000000000 +0530 +++ linux-2.6.22-rc6-balbir/mm/oom_kill.c 2007-07-20 13:13:26.000000000 +0530 @@ -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 s…
creator: Balbir Singh
date: 2007-07-20T08:25:18-00:00

[RFC][-mm PATCH 8/8] Add switch to control what type of pages to limit (v3)
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@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-20 13:13:26.000000000 +0530 +++ linux-2.6.22-rc6-balbir/mm/memcontrol.c 2007-07-20 13:13:26.000000000 +0530 @@ -27,6 +27,8 @@ #include <linux/swap.h> #include <linux/spinlock.h> +#include <asm/uaccess.h> + struct container_subsys mem_container_subsys; /* @@ -57,6 +59,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 */ }; /* @@ -70,6 +73,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 *cont) @@ -307,6 +318,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. */ @@ -317,7 +344,9 @@ void mem_container_uncharge(struct meta_ unsigned long…
creator: Balbir Singh
date: 2007-07-20T08:25:28-00:00

Re: [RFC][-mm PATCH 4/8] Memory controller memory accounting (v3)
type: item
description: On 7/20/07, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > +void __always_inline unlock_meta_page(struct page *page) > +{ > + bit_spin_unlock(PG_metapage, &page->flags); > +} Maybe add a BUG_ON(!test_bit(PG_metapage, &page->flags)) at least for development? > + mem = rcu_dereference(mm->mem_container); > + /* > + * For every charge from the container, increment reference > + * count > + */ > + css_get(&mem->css); > + rcu_read_unlock(); It's not clear to me that this is safe. If > + > + /* > + * If we created the meta_page, we should free it on exceeding > + * the container limit. > + */ > + if (res_counter_charge(&mem->res, 1)) { > + css_put(&mem->css); > + goto free_mp; > + } > + > + lock_meta_page(page); > + /* > + * Check if somebody else beat us to allocating the meta_page > + */ > + if (page_get_meta_page(page)) { I think you need to add something like kfree(mp); mp = page_get_meta_page(page); otherwise you're going to leak the new but unneeded metapage. > + atomic_inc(&mp->ref_cnt); > + res_counter_uncharge(&mem->res, 1); > + goto done; > + } > + > + atomic_set(&mp->ref_cnt, 1); > + mp->mem_container = mem; > + mp->page = page; > + page_assign_meta_page(page, mp); Would it make sense to have the "mp->page = page" be part of page_assign_meta_page() for consistency? > +err: > + unlock_meta_page(page); > + return -ENOMEM; The only jump to err: is from a location where the metapage is already unlocked. Maybe scrap err: and just do a return -ENOMEM when the allocation fails? > +out_uncharge: > + mem_container_uncharge(page_get_meta_page(page)); Wanting to call mem_container_uncharge() on a page and hence having to call page_get_meta_page() seems to be more common than wanting to call it on a meta page that you already have available. Maybe make mem_container_uncharge() be a wrapper that take a struct page and does something like mem_container_uncharge_mp(page_get_meta_page(page)) where mem_container_uncharge_mp() is the raw meta-page version? Paul ______________…
creator: Paul Menage
date: 2007-07-20T21:03:36-00:00

Re: [RFC][-mm PATCH 4/8] Memory controller memory accounting (v3)
type: item
description: On 7/20/07, Paul Menage <menage@google.com> wrote: > > + mem = rcu_dereference(mm->mem_container); > > + /* > > + * For every charge from the container, increment reference > > + * count > > + */ > > + css_get(&mem->css); > > + rcu_read_unlock(); > > It's not clear to me that this is safe. > Sorry, didn't finish this thought. Right after the rcu_dereference() the mm could be moved to another container by some other process. Since there's an rcu_synchronize() after the movement, and the rcu_synchronize() caller holds container_mutex then I guess it's not possible for a third process to delete the container during the RCU section, since it won't be able to acquire the mutex until after the RCU read section completes. So OK, it is safe, at least based on the guarantees made by the current implementation. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2007-07-20T21:09:09-00:00

Re: [RFC][-mm PATCH 8/8] Add switch to control what type of pages to limit (v3)
type: item
description: On 7/20/07, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > -static struct mem_container init_mem_container; > +static ssize_t mem_control_type_read(struct container *cont, > + struct cftype *cft, > + struct file *file, char __user *userbuf, > + size_t nbytes, loff_t *ppos) > +{ > + unsigned long val; > + char buf[64], *s; > + struct mem_container *mem; > + > + mem = mem_container_from_cont(cont); > + s = buf; > + val = mem->control_type; > + s += sprintf(s, "%lu\n", val); > + return simple_read_from_buffer((void __user *)userbuf, nbytes, > + ppos, buf, s - buf); > +} This could just use the read_uint64() hook and be something like static u64 mem_container_control_type_read(struct container *cont, struct cftype *cft) { return mem_container_from_cont(cont)->control_type; } Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2007-07-20T21:41:22-00:00

Re: [RFC][-mm PATCH 4/8] Memory controller memory accounting (v3)
type: item
description: Paul Menage wrote: > On 7/20/07, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > >> +void __always_inline unlock_meta_page(struct page *page) >> +{ >> + bit_spin_unlock(PG_metapage, &page->flags); >> +} > > Maybe add a BUG_ON(!test_bit(PG_metapage, &page->flags)) at least for > development? > I'd rather make that a VM_BUG_ON, but that's a good suggestion >> + mem = rcu_dereference(mm->mem_container); >> + /* >> + * For every charge from the container, increment reference >> + * count >> + */ >> + css_get(&mem->css); >> + rcu_read_unlock(); > > It's not clear to me that this is safe. > > If > >> + >> + /* >> + * If we created the meta_page, we should free it on exceeding >> + * the container limit. >> + */ >> + if (res_counter_charge(&mem->res, 1)) { >> + css_put(&mem->css); >> + goto free_mp; >> + } >> + >> + lock_meta_page(page); >> + /* >> + * Check if somebody else beat us to allocating the meta_page >> + */ >> + if (page_get_meta_page(page)) { > > I think you need to add something like > > kfree(mp); > mp = page_get_meta_page(page); > > otherwise you're going to leak the new but unneeded metapage. > Yes, good catch! I am surprised I did not check for that. >> + atomic_inc(&mp->ref_cnt); >> + res_counter_uncharge(&mem->res, 1); >> + goto done; >> + } >> + >> + atomic_set(&mp->ref_cnt, 1); >> + mp->mem_container = mem; >> + mp->page = page; >> + page_assign_meta_page(page, mp); > > Would it make sense to have the "mp->page = page" be part of > page_assign_meta_page() for consistency? > Yes, that could be done easily. >> +err: >> + unlock_meta_page(page); >> + return -ENOMEM; > > The only jump to err: is from a location where the metapage is already > unlocked. Maybe scrap err: and just do a return -ENOMEM when the > allocation fails? > Sounds good, let me revisit the code. >> +out_uncharge: >> + mem_container_uncharge(page_get_meta_page(page)); > > Wanting to call mem_container_uncharge() on a page and hence having to > call page_get_meta_page() seems to be more…
creator: Balbir Singh
date: 2007-07-21T17:11:37-00:00

Re: [RFC][-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v3)
type: item
description: hi, > +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) > +{ > + unsigned long nr_taken = 0; > + struct page *page; > + unsigned long scan; > + LIST_HEAD(mp_list); > + struct list_head *src; > + struct meta_page *mp; > + > + if (active) > + src = &mem_cont->active_list; > + else > + src = &mem_cont->inactive_list; > + > + for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) { > + mp = list_entry(src->prev, struct meta_page, lru); > + page = mp->page; > + - is it safe to pick the lists without mem_cont->lru_lock held? - what prevents mem_container_uncharge from freeing this meta_page behind us? YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2007-07-24T11:51:00-00:00

Re: [RFC][-mm PATCH 6/8] Memory controller add per container LRU and reclaim (v3)
type: item
description: YAMAMOTO Takashi wrote: > hi, > >> +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) >> +{ >> + unsigned long nr_taken = 0; >> + struct page *page; >> + unsigned long scan; >> + LIST_HEAD(mp_list); >> + struct list_head *src; >> + struct meta_page *mp; >> + >> + if (active) >> + src = &mem_cont->active_list; >> + else >> + src = &mem_cont->inactive_list; >> + >> + for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) { >> + mp = list_entry(src->prev, struct meta_page, lru); >> + page = mp->page; >> + > > - is it safe to pick the lists without mem_cont->lru_lock held? > > - what prevents mem_container_uncharge from freeing this meta_page > behind us? > > YAMAMOTO Takashi Hi, YAMAMOTO, We do take the lru_lock before deleting the page from the list and in mem_container_move_lists(). But, I guess like you point out page = mp->page might not be a safe operation. I'll fix the problem in the next release. Thanks for the review, -- 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-24T12:14:09-00:00