OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/8] RSS controller based on process containers (v2)
items: [PATCH 1/8] Resource counters
items: [PATCH 2/8] Add container pointer on struct page
items: [PATCH 3/8] Add container pointer on mm_struct
items: [PATCH 4/8] RSS container core
items: [PATCH 5/8] RSS accounting hooks over the code
items: [PATCH 6/8] Per container OOM killer
items: [PATCH 7/8] Page scanner changes needed to implement per-container scanner
items: [PATCH 8/8] Per-container pages reclamation
items: Re: [PATCH 0/8] RSS controller based on process containers (v2)
items: Re: [PATCH 2/8] Add container pointer on struct page
items: Re: [PATCH 8/8] Per-container pages reclamation

[PATCH 0/8] RSS controller based on process containers (v2)
type: item
description: Adds RSS accounting and control within a container. Major change: current scanner code reuse. Tasks and files accounting is not included as these containers are simple enough to be implemented later. Based on top of Paul Menage's container subsystem v8. Note, that only first three patches from Paul's patchset should be used as BeanCounters, CPU accounting etc are not needed for RSS container. RSS container includes the per-container RSS accounting and reclamation, and out-of-memory killer. The container behaves like a standalone machine - when it runs out of resources, it tries to reclaim some pages, and if it doesn't succeed, kills some task which mm_struct belongs to the container in question. Changes from previous version: * reclamation code reuse. I.e. the scanner code used to reclaim pages on global memory shortage is used as-is in per-container page reclamation; * active/inactive per-container lists (a bit reworked patch from Balbir); * more elegant top container creation (thanks Paul for clarifying).
creator: xemul
date: 2007-04-09T12:18:52-00:00

[PATCH 1/8] Resource counters
type: item
description: 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. diff -upr linux-2.6.20.orig/include/linux/res_counter.h linux-2.6.20-2/include/linux/res_counter.h --- linux-2.6.20.orig/include/linux/res_counter.h 2007-04-09 11:26:12.000000000 +0400 +++ linux-2.6.20-2/include/linux/res_counter.h 2007-04-09 11:26:06.000000000 +0400 @@ -0,0 +1,83 @@ +#ifndef __RES_COUNTER_H__ +#define __RES_COUNTER_H__ +/* + * resource counters + * + * Copyright 2007 OpenVZ SWsoft Inc + * + * Author: Pavel Emelianov <xemul@openvz.org> + * + */ + +#include <linux/container.h> + +struct res_counter { + unsigned long usage; + unsigned long limit; + unsigned long failcnt; + spinlock_t lock; +}; + +enum { + RES_USAGE, + RES_LIMIT, + RES_FAILCNT, +}; + +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); + +static inline void res_counter_init(struct res_counter *cnt) +{ + spin_lock_init(&cnt->lock); + cnt->limit = (unsigned long)LONG_MAX; +} + +static inline int res_counter_charge_locked(struct res_counter *cnt, + unsigned long val) +{ + if (cnt->usage <= cnt->limit - val) { + cnt->usage += val; + return 0; + } + + cnt->failcnt++; + return -ENOMEM; +} + +static inline int res_counter_charge(struct res_counter *cnt, + unsigned long val) +{ + int ret; + unsigned long flags; + + spin_lock_irqsave(&cnt->lock, flags); + ret = res_counter_charge_locked(cnt, val); + spin_unlock_irqrestore(&cnt->lock, flags); + return ret; +} + +static inline void res_counter_uncharge_locked(struct res_counter *cnt, + unsigned long val) +{ + if (unlikely(cnt->usage < val)) { + WARN_ON(1); + val = cnt->usage; + } + + cnt->usage -= val; +} + +static inline void res_counter_uncharge(struct res_counter *cnt…
creator: xemul
date: 2007-04-09T12:31:58-00:00

[PATCH 2/8] Add container pointer on struct page
type: item
description: Each page is supposed to have an owner - the container that touched the page first. The owner stays alive during the page lifetime even if the task that touched the page dies or moves to another container. This ownership is the forerunner for the "fair" page sharing accounting, in which page has as may owners as it is really used by. diff -upr linux-2.6.20.orig/include/linux/mm.h linux-2.6.20-2/include/linux/mm.h --- linux-2.6.20.orig/include/linux/mm.h 2007-03-06 19:09:50.000000000 +0300 +++ linux-2.6.20-2/include/linux/mm.h 2007-04-09 11:26:06.000000000 +0400 @@ -220,6 +220,12 @@ struct vm_operations_struct { struct mmu_gather; struct inode; +#ifdef CONFIG_RSS_CONTAINER +#define page_container(page) (page->rss_container) +#else +#define page_container(page) (NULL) +#endif + #define page_private(page) ((page)->private) #define set_page_private(page, v) ((page)->private = (v)) diff -upr linux-2.6.20.orig/include/linux/mm_types.h linux-2.6.20-2/include/linux/mm_types.h --- linux-2.6.20.orig/include/linux/mm_types.h 2007-03-06 19:09:50.000000000 +0300 +++ linux-2.6.20-2/include/linux/mm_types.h 2007-04-09 11:26:06.000000000 +0400 @@ -62,6 +62,9 @@ struct page { void *virtual; /* Kernel virtual address (NULL if not kmapped, ie. highmem) */ #endif /* WANT_PAGE_VIRTUAL */ +#ifdef CONFIG_RSS_CONTAINER + struct page_container *rss_container; +#endif }; #endif /* _LINUX_MM_TYPES_H */
creator: xemul
date: 2007-04-09T12:37:43-00:00

[PATCH 3/8] Add container pointer on mm_struct
type: item
description: Naturally mm_struct determines the resource consumer in memory accounting. So each mm_struct should have a pointer on container it belongs to. When a new task is created its mm_struct is assigned to the container this task belongs to. diff -upr linux-2.6.20.orig/include/linux/sched.h linux-2.6.20-2/include/linux/sched.h --- linux-2.6.20.orig/include/linux/sched.h 2007-04-09 11:26:06.000000000 +0400 +++ linux-2.6.20-2/include/linux/sched.h 2007-04-09 11:26:06.000000000 +0400 @@ -373,6 +373,9 @@ struct mm_struct { /* aio bits */ rwlock_t ioctx_list_lock; struct kioctx *ioctx_list; +#ifdef CONFIG_RSS_CONTAINER + struct rss_container *rss_container; +#endif }; struct sighand_struct { diff -upr linux-2.6.20.orig/kernel/fork.c linux-2.6.20-2/kernel/fork.c --- linux-2.6.20.orig/kernel/fork.c 2007-04-09 11:26:06.000000000 +0400 +++ linux-2.6.20-2/kernel/fork.c 2007-04-09 11:26:06.000000000 +0400 @@ -57,6 +57,8 @@ #include <asm/cacheflush.h> #include <asm/tlbflush.h> +#include <linux/rss_container.h> + /* * Protected counters by write_lock_irq(&tasklist_lock) */ @@ -325,7 +327,7 @@ static inline void mm_free_pgd(struct mm #include <linux/init_task.h> -static struct mm_struct * mm_init(struct mm_struct * mm) +static struct mm_struct * mm_init(struct mm_struct *mm, struct task_struct *tsk) { atomic_set(&mm->mm_users, 1); atomic_set(&mm->mm_count, 1); @@ -340,11 +342,14 @@ static struct mm_struct * mm_init(struct mm->ioctx_list = NULL; mm->free_area_cache = TASK_UNMAPPED_BASE; mm->cached_hole_size = ~0UL; + mm_init_container(mm, tsk); if (likely(!mm_alloc_pgd(mm))) { mm->def_flags = 0; return mm; } + + mm_free_container(mm); free_mm(mm); return NULL; } @@ -359,7 +364,7 @@ struct mm_struct * mm_alloc(void) mm = allocate_mm(); if (mm) { memset(mm, 0, sizeof(*mm)); - mm = mm_init(mm); + mm = mm_init(mm, current); } return mm; } @@ -373,6 +378,7 @@ void fastcall __mmdrop(struct mm_struct { BUG_ON(mm == &init_mm); mm_free_pgd(mm); + mm_free_container(mm); destroy_context(mm); free_m…
creator: xemul
date: 2007-04-09T12:43:17-00:00

[PATCH 4/8] RSS container core
type: item
description: This includes * definition of rss_container as container subsystem combined with resource counter; * registration of RSS container in generic containers; * routines for pages tracking. diff -upr linux-2.6.20.orig/include/linux/container_subsys.h linux-2.6.20-2/include/linux/container_subsys.h --- linux-2.6.20.orig/include/linux/container_subsys.h 2007-04-09 11:26:06.000000000 +0400 +++ linux-2.6.20-2/include/linux/container_subsys.h 2007-04-09 11:26:06.000000000 +0400 @@ -9,6 +9,10 @@ SUBSYS(cpuset) #endif +#ifdef CONFIG_RSS_CONTAINER +SUBSYS(rss) +#endif + /* */ /* */ diff -upr linux-2.6.20.orig/include/linux/rss_container.h linux-2.6.20-2/include/linux/rss_container.h --- linux-2.6.20.orig/include/linux/rss_container.h 2007-04-09 11:26:12.000000000 +0400 +++ linux-2.6.20-2/include/linux/rss_container.h 2007-04-09 11:26:06.000000000 +0400 @@ -0,0 +1,55 @@ +#ifndef __RSS_CONTAINER_H__ +#define __RSS_CONTAINER_H__ +/* + * RSS container + * + * Copyright 2007 OpenVZ SWsoft Inc + * + * Author: Pavel Emelianov <xemul@openvz.org> + * + */ + +struct page_container; +struct rss_container; + +#ifdef CONFIG_RSS_CONTAINER +int container_rss_prepare(struct page *, struct vm_area_struct *vma, + struct page_container **); + +void container_rss_add(struct page_container *); +void container_rss_del(struct page_container *); +void container_rss_release(struct page_container *); + +void mm_init_container(struct mm_struct *mm, struct task_struct *tsk); +void mm_free_container(struct mm_struct *mm); + +#else +static inline int container_rss_prepare(struct page *pg, + struct vm_area_struct *vma, struct page_container **pc) +{ + *pc = NULL; /* to make gcc happy */ + return 0; +} + +static inline void container_rss_add(struct page_container *pc) +{ +} + +static inline void container_rss_del(struct page_container *pc) +{ +} + +static inline void container_rss_release(struct page_container *pc) +{ +} + +static inline void mm_init_container(struct mm_struct *mm, struct task_struct *t) +{ +}…
creator: xemul
date: 2007-04-09T12:45:19-00:00

[PATCH 5/8] RSS accounting hooks over the code
type: item
description: As described above, pages are charged to their first touchers. The first toucher is determined using pages' mapcount manipulations in rmap calls. Page is charged in two stages: 1. preparation, in which the resource availability is checked. This stage may lead to page reclamation, thus it is performed in a might-sleep places; 2. the container assignment to page. This is done in an atomic code that handles multiple touches. diff -upr linux-2.6.20.orig/fs/exec.c linux-2.6.20-2/fs/exec.c --- linux-2.6.20.orig/fs/exec.c 2007-03-06 19:09:50.000000000 +0300 +++ linux-2.6.20-2/fs/exec.c 2007-04-09 11:26:06.000000000 +0400 @@ -58,6 +58,8 @@ #include <linux/kmod.h> #endif +#include <linux/rss_container.h> + int core_uses_pid; char core_pattern[128] = "core"; int suid_dumpable = 0; @@ -309,27 +311,34 @@ void install_arg_page(struct vm_area_str struct mm_struct *mm = vma->vm_mm; pte_t * pte; spinlock_t *ptl; + struct page_container *pcont; if (unlikely(anon_vma_prepare(vma))) goto out; + if (container_rss_prepare(page, vma, &pcont)) + goto out; + flush_dcache_page(page); pte = get_locked_pte(mm, address, &ptl); if (!pte) - goto out; + goto out_release; if (!pte_none(*pte)) { pte_unmap_unlock(pte, ptl); - goto out; + goto out_release; } inc_mm_counter(mm, anon_rss); lru_cache_add_active(page); set_pte_at(mm, address, pte, pte_mkdirty(pte_mkwrite(mk_pte( page, vma->vm_page_prot)))); - page_add_new_anon_rmap(page, vma, address); + page_add_new_anon_rmap(page, vma, address, pcont); pte_unmap_unlock(pte, ptl); /* no need for flush_tlb */ return; + +out_release: + container_rss_release(pcont); out: __free_page(page); force_sig(SIGKILL, current); diff -upr linux-2.6.20.orig/include/linux/rmap.h linux-2.6.20-2/include/linux/rmap.h --- linux-2.6.20.orig/include/linux/rmap.h 2007-03-06 19:09:50.000000000 +0300 +++ linux-2.6.20-2/include/linux/rmap.h 2007-04-09 11:26:06.000000000 +0400 @@ -69,9 +69,13 @@ void __anon_vma_link(struct vm_area_stru /* * rmap interfaces called when adding or r…
creator: xemul
date: 2007-04-09T12:50:21-00:00

[PATCH 6/8] Per container OOM killer
type: item
description: When container is completely out of memory some tasks should die. This is unfair to kill the current task, so a task with the largest RSS is chosen and killed. The code re-uses current OOM killer select_bad_process() for task selection. diff -upr linux-2.6.20.orig/include/linux/rss_container.h linux-2.6.20-2/include/linux/rss_container.h --- linux-2.6.20.orig/include/linux/rss_container.h 2007-04-09 11:26:12.000000000 +0400 +++ linux-2.6.20-2/include/linux/rss_container.h 2007-04-09 11:26:06.000000000 +0400 @@ -19,6 +19,7 @@ void container_rss_add(struct page_container *); void container_rss_del(struct page_container *); void container_rss_release(struct page_container *); +void container_out_of_memory(struct rss_container *); void mm_init_container(struct mm_struct *mm, struct task_struct *tsk); void mm_free_container(struct mm_struct *mm); diff -upr linux-2.6.20.orig/mm/oom_kill.c linux-2.6.20-2/mm/oom_kill.c --- linux-2.6.20.orig/mm/oom_kill.c 2007-03-06 19:09:50.000000000 +0300 +++ linux-2.6.20-2/mm/oom_kill.c 2007-04-09 11:26:06.000000000 +0400 @@ -24,6 +24,7 @@ #include <linux/cpuset.h> #include <linux/module.h> #include <linux/notifier.h> +#include <linux/rss_container.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) */ -unsigned long badness(struct task_struct *p, unsigned long uptime) +unsigned long badness(struct task_struct *p, unsigned long uptime, + struct rss_container *rss) { unsigned long points, cpu_time, run_time, s; struct mm_struct *mm; @@ -60,6 +62,13 @@ unsigned long badness(struct task_struct return 0; } +#ifdef CONFIG_RSS_CONTAINER + if (rss != NULL && mm->rss_container != rss) { + task_unlock(p); + return 0; + } +#endif + /* * The memory size of the process is the basis for the badness. */ @@ -200,7 +209,8 @@ static inline int constrained_alloc(stru * * (not docbooked, we don't want this one cluttering up the manual) */ -static struct task_struc…
creator: xemul
date: 2007-04-09T12:52:19-00:00

[PATCH 7/8] Page scanner changes needed to implement per-container scanner
type: item
description: Struct scan_control now carries: * the RSS container to free pages in; * pointer to an isolate_pages() function to isolate pages needed for the reclamation. diff -upr linux-2.6.20.orig/mm/vmscan.c linux-2.6.20-2/mm/vmscan.c --- linux-2.6.20.orig/mm/vmscan.c 2007-03-06 19:09:50.000000000 +0300 +++ linux-2.6.20-2/mm/vmscan.c 2007-04-09 11:26:06.000000000 +0400 @@ -45,6 +45,8 @@ #include "internal.h" +#include <linux/rss_container.h> + struct scan_control { /* Incremented by the number of inactive pages that were scanned */ unsigned long nr_scanned; @@ -66,6 +68,10 @@ struct scan_control { int swappiness; int all_unreclaimable; + struct rss_container *cnt; + unsigned long (*isolate_pages)(unsigned long nr, struct list_head *dst, + unsigned long *scanned, struct zone *zone, + struct rss_container *cont, int active); }; /* @@ -654,6 +660,17 @@ static unsigned long isolate_lru_pages(u return nr_taken; } +static unsigned long isolate_pages_global(unsigned long nr, + struct list_head *dst, unsigned long *scanned, + struct zone *z, struct rss_container *cont, + int active) +{ + if (active) + return isolate_lru_pages(nr, &z->active_list, dst, scanned); + else + return isolate_lru_pages(nr, &z->inactive_list, dst, scanned); +} + /* * shrink_inactive_list() is a helper for shrink_zone(). It returns the number * of reclaimed pages @@ -676,9 +693,9 @@ static unsigned long shrink_inactive_lis unsigned long nr_scan; unsigned long nr_freed; - nr_taken = isolate_lru_pages(sc->swap_cluster_max, - &zone->inactive_list, - &page_list, &nr_scan); + nr_taken = sc->isolate_pages(sc->swap_cluster_max, &page_list, + &nr_scan, zone, sc->cnt, 0); + zone->nr_inactive -= nr_taken; zone->pages_scanned += nr_scan; spin_unlock_irq(&zone->lru_lock); @@ -822,8 +839,8 @@ force_reclaim_mapped: lru_add_drain(); spin_lock_irq(&zone->lru_lock); - pgmoved = isolate_lru_pages(nr_pages, &zone->active_list, - &l_hold, &pgscanned); + pgmoved = sc->isolate_pages(nr_pages, &l_hold, &pgscanned, zone, + sc->cnt, 1)…
creator: xemul
date: 2007-04-09T12:57:22-00:00

[PATCH 8/8] Per-container pages reclamation
type: item
description: Implement try_to_free_pages_in_container() to free the pages in container that has run out of memory. The scan_control->isolate_pages() function isolates the container pages only. diff -upr linux-2.6.20.orig/include/linux/swap.h linux-2.6.20-2/include/linux/swap.h --- linux-2.6.20.orig/include/linux/swap.h 2007-03-06 19:09:50.000000000 +0300 +++ linux-2.6.20-2/include/linux/swap.h 2007-04-09 11:26:06.000000000 +0400 @@ -187,6 +187,9 @@ extern void swap_setup(void); /* linux/mm/vmscan.c */ extern unsigned long try_to_free_pages(struct zone **, gfp_t); + +struct rss_container; +extern unsigned long try_to_free_pages_in_container(struct rss_container *); extern unsigned long shrink_all_memory(unsigned long nr_pages); extern int vm_swappiness; extern int remove_mapping(struct address_space *mapping, struct page *page); diff -upr linux-2.6.20.orig/mm/vmscan.c linux-2.6.20-2/mm/vmscan.c --- linux-2.6.20.orig/mm/vmscan.c 2007-03-06 19:09:50.000000000 +0300 +++ linux-2.6.20-2/mm/vmscan.c 2007-04-09 11:26:06.000000000 +0400 @@ -872,6 +872,7 @@ force_reclaim_mapped: ClearPageActive(page); list_move(&page->lru, &zone->inactive_list); + container_rss_move_lists(page, 0); pgmoved++; if (!pagevec_add(&pvec, page)) { zone->nr_inactive += pgmoved; @@ -900,6 +901,7 @@ force_reclaim_mapped: SetPageLRU(page); VM_BUG_ON(!PageActive(page)); list_move(&page->lru, &zone->active_list); + container_rss_move_lists(page, 1); pgmoved++; if (!pagevec_add(&pvec, page)) { zone->nr_active += pgmoved; @@ -1110,6 +1112,35 @@ out: return ret; } +#ifdef CONFIG_RSS_CONTAINER +unsigned long try_to_free_pages_in_container(struct rss_container *cnt) +{ + struct scan_control sc = { + .gfp_mask = GFP_KERNEL, + .may_writepage = 1, + .swap_cluster_max = 1, + .may_swap = 1, + .swappiness = vm_swappiness, + .cnt = cnt, + .isolate_pages = isolate_pages_in_container, + }; + int node; + struct zone **zones; + + for_each_node(node) { +#ifdef CONFIG_HIGHMEM + zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zon…
creator: xemul
date: 2007-04-09T12:59:50-00:00

Re: [PATCH 0/8] RSS controller based on process containers (v2)
type: item
description: *ugh* /me no like. The basic premises seems to be that we can track page owners perfectly (although this patch set does not yet do so), through get/release operations (on _mapcount). This is simply not true for unmapped pagecache pages. Those receive no 'release' event; (the usage by find_get_page() could be seen as 'get'). Also, you don't seem to balance the active/inactive scanning on a per container basis. This skews the per container working set logic. Lastly, you don't call the slab shrinker for container reclaim; which would leave slab reclaim only for those few non process specific allocations, which would greatly skew the pagecache/slab balance. Let us call struct reclaim_struct { struct list_head active_list; struct list_head inactive_list; unsigned long nr_active; unsigned long nr_inactive; } Lets recognise three distinct page categories: - anonymous memory, - mapped pagecache, and - unmapped pagecache. We then keep anonymous pages on a per container reclaim_struct, these pages are fully accounted to each container. We keep mapped pagecache pages on per inode reclaim_structs, these files could be shared between containers and we could either just account all pages belonging to each file proportional to the number of containers involved, or do a more precise accounting. We keep unmapped pagecache pages on a global reclaim_struct, these pages can, in general, not be pinned to a specific container; all we can do is keep a floating proportion relative to container 'get' events (find_get_page() and perhaps add_to_page_cache()). Reclaim will then have to fairly reclaim pages from all of these lists. If we schedule such that it appears that these lists are parallel instead of serial - that is a each tail is really a tail, not the head of another list - the current reclaim semantics are preserved. The slab shrinker should be called proportional to the containers size relative to the machine. Global reclaim will have to call each container reclaim in proportional f…
creator: Peter Zijlstra
date: 2007-04-09T15:54:50-00:00

Re: [PATCH 0/8] RSS controller based on process containers (v2)
type: item
description: Peter Zijlstra wrote: > *ugh* /me no like. > > The basic premises seems to be that we can track page owners perfectly > (although this patch set does not yet do so), through get/release It looks like you have examined the patches not very carefully before concluding this. These patches DO track page owners. I know that a page may be shared among several containers and thus have many owners so we should track all of them. This is exactly what we decided not to do half-a-year ago. Page sharing accounting is performed in OpenVZ beancounters, and this functionality will be pushed to mainline after this simple container. > operations (on _mapcount). > > This is simply not true for unmapped pagecache pages. Those receive no > 'release' event; (the usage by find_get_page() could be seen as 'get'). These patches concern the mapped pagecache only. Unmapped pagecache control is out of the scope of it since we do not want one container to track all the resources. > Also, you don't seem to balance the active/inactive scanning on a per > container basis. This skews the per container working set logic. This is not true. Balbir sent a patch to the first version of this container that added active/inactive balancing to the container. I have included this (a bit reworked) patch into this version and pointed this fact in the zeroth letter. > Lastly, you don't call the slab shrinker for container reclaim; which > would leave slab reclaim only for those few non process specific > allocations, which would greatly skew the pagecache/slab balance. Of course I do not call the slab shrinker! We do not have the kernel memory control yet. Thus we can not shrink arbitrary kernel objects just because some container has run out of its *user* memory. Kernel memory control will come later. We decided to start from a simple RSS control. Please, refer to containers archives for more details. > > > Let us call > > struct reclaim_struct { > struct list_head active_list; > struct list_head inactive_lis…
creator: xemul
date: 2007-04-10T08:27:19-00:00

Re: [PATCH 2/8] Add container pointer on struct page
type: item
description: Hi Pavel, I have been implied in the work for the memory controller of res groups a few months ago. I see that you propose to modify the struct page to point to rss container struct. This has made some debate because of the struct page size increase, but this allows a quicker scan to reclaim pages (I mean having per-container lists of active/inactive pages). We (here at Bull and others) proposed this implementation for res groups and I am interested in knowing if this has a chance of being accepted today (hope so). I know this uses memory for internal management and increases a lot the memory size used for a large memory configuration, but in that case we have lot of memory, so where is the issue ? We tested this on a 28 GB server and it worked. Also we can use larger page size to reduce the overhead, and I believe this makes sense on large servers with big memory. So we balance between using more memory internally and so getting faster access to pages for reclaim, or do nothing. ;-) jean-pierre
creator: Jean-Pierre Dion
date: 2007-04-13T13:56:37-00:00

Re: [PATCH 2/8] Add container pointer on struct page
type: item
description: Jean-Pierre Dion wrote: > Hi Pavel, > > I have been implied in the work for the > memory controller of res groups a few months ago. > > I see that you propose to modify the struct > page to point to rss container struct. > This has made some debate because of the struct > page size increase, but this allows a quicker > scan to reclaim pages (I mean having per-container > lists of active/inactive pages). > We (here at Bull and others) proposed this implementation > for res groups and I am interested in knowing > if this has a chance of being accepted today (hope so). So do I :) I'm not the one who makes the final decision ;) > I know this uses memory for internal management > and increases a lot the memory size used for > a large memory configuration, but in that case > we have lot of memory, so where is the issue ? > We tested this on a 28 GB server and it worked. Thank you for additional testing on enterprise servers! Hope this will be a good argument in favour of the patches. > Also we can use larger page size to reduce > the overhead, and I believe this makes sense > on large servers with big memory. > > So we balance between using more memory internally > and so getting faster access to pages for reclaim, > or do nothing. ;-) That's right. I made some small testing which showed that moving this pointer in a mirrored array saves less than 0.1% of performance on 4CPU i386 node. I don't know how this will be on enterprise hardware, but I do believe that the results will be the same (or even better). > > jean-pierre > > > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
creator: xemul
date: 2007-04-13T14:48:00-00:00

Re: [PATCH 0/8] RSS controller based on process containers (v2)
type: item
description: Pavel Emelianov wrote: > Peter Zijlstra wrote: >> *ugh* /me no like. >> >> The basic premises seems to be that we can track page owners perfectly >> (although this patch set does not yet do so), through get/release > > It looks like you have examined the patches not very carefully > before concluding this. These patches DO track page owners. > > I know that a page may be shared among several containers and > thus have many owners so we should track all of them. This is > exactly what we decided not to do half-a-year ago. > > Page sharing accounting is performed in OpenVZ beancounters, and > this functionality will be pushed to mainline after this simple > container. > >> operations (on _mapcount). >> >> This is simply not true for unmapped pagecache pages. Those receive no >> 'release' event; (the usage by find_get_page() could be seen as 'get'). > > These patches concern the mapped pagecache only. Unmapped pagecache > control is out of the scope of it since we do not want one container > to track all the resources. Unmapped pagecache control and swapcache control is part of independent pagecache controller that is being developed. Initial version was posted at http://lkml.org/lkml/2007/3/06/51 I plan to post a new version based on this patchset in a couple of days. --Vaidy >> Also, you don't seem to balance the active/inactive scanning on a per >> container basis. This skews the per container working set logic. > > This is not true. Balbir sent a patch to the first version of this > container that added active/inactive balancing to the container. > I have included this (a bit reworked) patch into this version and > pointed this fact in the zeroth letter. > [snip]
creator: Vaidyanathan Srinivas
date: 2007-04-19T05:37:14-00:00

Re: [PATCH 8/8] Per-container pages reclamation
type: item
description: Pavel Emelianov wrote: > Implement try_to_free_pages_in_container() to free the > pages in container that has run out of memory. > > The scan_control->isolate_pages() function isolates the > container pages only. > Pavel, I've just started playing around with these patches, I preferred the approach of v1. Please see below > +static unsigned long isolate_container_pages(unsigned long nr_to_scan, > + struct list_head *src, struct list_head *dst, > + unsigned long *scanned, struct zone *zone) > +{ > + unsigned long nr_taken = 0; > + struct page *page; > + struct page_container *pc; > + unsigned long scan; > + LIST_HEAD(pc_list); > + > + for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) { > + pc = list_entry(src->prev, struct page_container, list); > + page = pc->page; > + if (page_zone(page) != zone) > + continue; shrink_zone() will walk all pages looking for pages belonging to this container and this slows down the reclaim quite a bit. Although we've reused code, we've ended up walking the entire list of the zone to find pages belonging to a particular container, this was the same problem I had with my RSS controller patches. > + > + list_move(&pc->list, &pc_list); > + -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
creator: Balbir Singh
date: 2007-04-24T09:47:25-00:00

Re: [PATCH 8/8] Per-container pages reclamation
type: item
description: Balbir Singh wrote: > Pavel Emelianov wrote: >> Implement try_to_free_pages_in_container() to free the >> pages in container that has run out of memory. >> >> The scan_control->isolate_pages() function isolates the >> container pages only. >> > > Pavel, > > I've just started playing around with these patches, I preferred > the approach of v1. Please see below > >> +static unsigned long isolate_container_pages(unsigned long nr_to_scan, >> + struct list_head *src, struct list_head *dst, >> + unsigned long *scanned, struct zone *zone) >> +{ >> + unsigned long nr_taken = 0; >> + struct page *page; >> + struct page_container *pc; >> + unsigned long scan; >> + LIST_HEAD(pc_list); >> + >> + for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) { >> + pc = list_entry(src->prev, struct page_container, list); >> + page = pc->page; >> + if (page_zone(page) != zone) >> + continue; > > shrink_zone() will walk all pages looking for pages belonging to this No. shrink_zone() will walk container pages looking for pages in the desired zone. Scann through the full zone is done on global memory shortage. > container and this slows down the reclaim quite a bit. Although we've > reused code, we've ended up walking the entire list of the zone to > find pages belonging to a particular container, this was the same > problem I had with my RSS controller patches. > >> + >> + list_move(&pc->list, &pc_list); >> + > >
creator: xemul
date: 2007-04-24T10:29:58-00:00

Re: [PATCH 8/8] Per-container pages reclamation
type: item
description: Pavel Emelianov wrote: > Balbir Singh wrote: >> Pavel Emelianov wrote: >>> Implement try_to_free_pages_in_container() to free the >>> pages in container that has run out of memory. >>> >>> The scan_control->isolate_pages() function isolates the >>> container pages only. >>> >> Pavel, >> >> I've just started playing around with these patches, I preferred >> the approach of v1. Please see below >> >>> +static unsigned long isolate_container_pages(unsigned long nr_to_scan, >>> + struct list_head *src, struct list_head *dst, >>> + unsigned long *scanned, struct zone *zone) >>> +{ >>> + unsigned long nr_taken = 0; >>> + struct page *page; >>> + struct page_container *pc; >>> + unsigned long scan; >>> + LIST_HEAD(pc_list); >>> + >>> + for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) { >>> + pc = list_entry(src->prev, struct page_container, list); >>> + page = pc->page; >>> + if (page_zone(page) != zone) >>> + continue; >> shrink_zone() will walk all pages looking for pages belonging to this > > No. shrink_zone() will walk container pages looking for pages in the desired zone. > Scann through the full zone is done on global memory shortage. > Yes, I see that now. But for each zone in the system, we walk through the containers list - right? I have some more fixes, improvements that I want to send across. I'll start sending them out to you as I test and verify them. >> container and this slows down the reclaim quite a bit. Although we've >> reused code, we've ended up walking the entire list of the zone to >> find pages belonging to a particular container, this was the same >> problem I had with my RSS controller patches. >> >>> + >>> + list_move(&pc->list, &pc_list); >>> + >> > -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
creator: Balbir Singh
date: 2007-04-24T11:01:07-00:00

Re: [PATCH 8/8] Per-container pages reclamation
type: item
description: Balbir Singh wrote: > Pavel Emelianov wrote: >> Balbir Singh wrote: >>> Pavel Emelianov wrote: >>>> Implement try_to_free_pages_in_container() to free the >>>> pages in container that has run out of memory. >>>> >>>> The scan_control->isolate_pages() function isolates the >>>> container pages only. >>>> >>> Pavel, >>> >>> I've just started playing around with these patches, I preferred >>> the approach of v1. Please see below >>> >>>> +static unsigned long isolate_container_pages(unsigned long nr_to_scan, >>>> + struct list_head *src, struct list_head *dst, >>>> + unsigned long *scanned, struct zone *zone) >>>> +{ >>>> + unsigned long nr_taken = 0; >>>> + struct page *page; >>>> + struct page_container *pc; >>>> + unsigned long scan; >>>> + LIST_HEAD(pc_list); >>>> + >>>> + for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) { >>>> + pc = list_entry(src->prev, struct page_container, list); >>>> + page = pc->page; >>>> + if (page_zone(page) != zone) >>>> + continue; >>> shrink_zone() will walk all pages looking for pages belonging to this >> >> No. shrink_zone() will walk container pages looking for pages in the >> desired zone. >> Scann through the full zone is done on global memory shortage. >> > > Yes, I see that now. But for each zone in the system, we walk through the > containers list - right? Right. > I have some more fixes, improvements that I want to send across. > I'll start sending them out to you as I test and verify them. That's great! :) Thanks for participation. > >>> container and this slows down the reclaim quite a bit. Although we've >>> reused code, we've ended up walking the entire list of the zone to >>> find pages belonging to a particular container, this was the same >>> problem I had with my RSS controller patches. >>> >>>> + >>>> + list_move(&pc->list, &pc_list); >>>> + >>> >> > >
creator: xemul
date: 2007-04-24T11:37:38-00:00

Re: [PATCH 8/8] Per-container pages reclamation
type: item
description: Pavel Emelianov wrote: > Implement try_to_free_pages_in_container() to free the > pages in container that has run out of memory. > > The scan_control->isolate_pages() function isolates the > container pages only. > > +#ifdef CONFIG_RSS_CONTAINER > +unsigned long try_to_free_pages_in_container(struct rss_container *cnt) > +{ > + struct scan_control sc = { > + .gfp_mask = GFP_KERNEL, > + .may_writepage = 1, > + .swap_cluster_max = 1, > + .may_swap = 1, > + .swappiness = vm_swappiness, > + .cnt = cnt, > + .isolate_pages = isolate_pages_in_container, > + }; > + int node; > + struct zone **zones; > + > + for_each_node(node) { > +#ifdef CONFIG_HIGHMEM > + zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zones; > + if (do_try_to_free_pages(zones, sc.gfp_mask, &sc)) > + return 1; > +#endif > + zones = NODE_DATA(node)->node_zonelists[ZONE_NORMAL].zones; > + if (do_try_to_free_pages(zones, sc.gfp_mask, &sc)) > + return 1; > + } > + return 0; > +} > +#endif > + Hi, Pavel, This patch fixes a bug in the RSS controller, where we walk through all nodes during reclaim in try_to_free_pages_in_container(). Instead of for_each_node(), we now use for_each_online_node() so that we do not end up with invalid zones from nodes that are not online. Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> --- mm/vmscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff -puN mm/vmscan.c~rss-fix-nodescan mm/vmscan.c --- linux-2.6.20/mm/vmscan.c~rss-fix-nodescan 2007-05-02 14:47:09.000000000 +0530 +++ linux-2.6.20-balbir/mm/vmscan.c 2007-05-02 14:47:25.000000000 +0530 @@ -1127,7 +1127,7 @@ unsigned long try_to_free_pages_in_conta int node; struct zone **zones; - for_each_node(node) { + for_each_online_node(node) { #ifdef CONFIG_HIGHMEM zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zones; if (do_try_to_free_pages(zones, sc.gfp_mask, &sc)) _ -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
creator: Balbir Singh
date: 2007-05-02T09:51:03-00:00

Re: [PATCH 8/8] Per-container pages reclamation
type: item
description: Pavel Emelianov wrote: > Implement try_to_free_pages_in_container() to free the > pages in container that has run out of memory. > > The scan_control->isolate_pages() function isolates the > container pages only. > > Hi, Pavel/Andrew, I've started running some basic tests like lmbench and LTP vm stress on the RSS controller. With the controller rss_limit set to 256 MB, I saw the following panic on a machine Unable to handle kernel NULL pointer dereference at 000000000000001c RIP: [<ffffffff80328581>] _raw_spin_lock+0xd/0xf6 PGD 3c841067 PUD 5d5d067 PMD 0 Oops: 0000 [1] SMP CPU 2 Modules linked in: ipv6 hidp rfcomm l2cap bluetooth sunrpc video button battery asus_acpi backlight ac lp parport_pc parport nvram pcspkr amd_rng rng_core i2c_amd756 i2c_core Pid: 13581, comm: mtest01 Not tainted 2.6.20-autokern1 #1 RIP: 0010:[<ffffffff80328581>] [<ffffffff80328581>] _raw_spin_lock+0xd/0xf6 RSP: 0000:ffff81003e6c9ce8 EFLAGS: 00010096 RAX: ffffffff8087f720 RBX: 0000000000000018 RCX: ffff81003f36f9d0 RDX: ffff8100807bb040 RSI: 0000000000000001 RDI: 0000000000000018 RBP: 0000000000000000 R08: ffff81003e6c8000 R09: 0000000000000002 R10: ffff810001021da8 R11: ffffffff8044658f R12: ffff81000c861e01 R13: 0000000000000018 R14: ffff81000c861eb8 R15: ffff810032d34138 FS: 00002abf7a1961e0(0000) GS:ffff81003edb94c0(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b CR2: 000000000000001c CR3: 000000002ba6e000 CR4: 00000000000006e0 Process mtest01 (pid: 13581, threadinfo ffff81003e6c8000, task ffff81003d8ec040) Stack: ffff810001003638 ffff810014a8c2c0 0000000000000000 ffff81000c861e01 0000000000000018 ffffffff80287166 ffff81000c861eb8 ffff81000000bac0 ffff81003f36f9a0 ffff81000c861e40 ffff81001d4b6a20 ffffffff8026a92e Call Trace: [<ffffffff80287166>] container_rss_move_lists+0x3b/0xaf [<ffffffff8026a92e>] activate_page+0xc1/0xd0 [<ffffffff80245f15>] wake_bit_function+0x0/0x23 [<ffffffff8026ab34>] mark_page_accessed+0x1b/0x2f [<ffffffff80265d25>] filemap_nopage+0…
creator: Balbir Singh
date: 2007-05-17T11:31:26-00:00

Re: [PATCH 8/8] Per-container pages reclamation
type: item
description: Balbir Singh wrote: > Pavel Emelianov wrote: >> Implement try_to_free_pages_in_container() to free the >> pages in container that has run out of memory. >> >> The scan_control->isolate_pages() function isolates the >> container pages only. Sorry for the late answer, but I have just managed to get to the patches. One comment is below. >> > > Hi, Pavel/Andrew, > > I've started running some basic tests like lmbench and LTP vm stress > on the RSS controller. > > With the controller rss_limit set to 256 MB, I saw the following panic > on a machine > > Unable to handle kernel NULL pointer dereference at 000000000000001c RIP: > [<ffffffff80328581>] _raw_spin_lock+0xd/0xf6 > PGD 3c841067 PUD 5d5d067 PMD 0 > Oops: 0000 [1] SMP > CPU 2 > Modules linked in: ipv6 hidp rfcomm l2cap bluetooth sunrpc video button battery asus_acpi backlight ac lp parport_pc parport nvram pcspkr amd_rng rng_core i2c_amd756 i2c_core > Pid: 13581, comm: mtest01 Not tainted 2.6.20-autokern1 #1 > RIP: 0010:[<ffffffff80328581>] [<ffffffff80328581>] _raw_spin_lock+0xd/0xf6 > RSP: 0000:ffff81003e6c9ce8 EFLAGS: 00010096 > RAX: ffffffff8087f720 RBX: 0000000000000018 RCX: ffff81003f36f9d0 > RDX: ffff8100807bb040 RSI: 0000000000000001 RDI: 0000000000000018 > RBP: 0000000000000000 R08: ffff81003e6c8000 R09: 0000000000000002 > R10: ffff810001021da8 R11: ffffffff8044658f R12: ffff81000c861e01 > R13: 0000000000000018 R14: ffff81000c861eb8 R15: ffff810032d34138 > FS: 00002abf7a1961e0(0000) GS:ffff81003edb94c0(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > CR2: 000000000000001c CR3: 000000002ba6e000 CR4: 00000000000006e0 > Process mtest01 (pid: 13581, threadinfo ffff81003e6c8000, task ffff81003d8ec040) > Stack: ffff810001003638 ffff810014a8c2c0 0000000000000000 ffff81000c861e01 > 0000000000000018 ffffffff80287166 ffff81000c861eb8 ffff81000000bac0 > ffff81003f36f9a0 ffff81000c861e40 ffff81001d4b6a20 ffffffff8026a92e > Call Trace: > [<ffffffff80287166>] container_rss_move_lists+0x3b…
creator: xemul
date: 2007-05-21T15:15:32-00:00

Re: [PATCH 8/8] Per-container pages reclamation
type: item
description: Pavel Emelianov wrote: >> Index: linux-2.6.20/mm/rss_container.c >> ============================================================ ======= >> --- linux-2.6.20.orig/mm/rss_container.c 2007-05-15 05:13:46.000000000 -0700 >> +++ linux-2.6.20/mm/rss_container.c 2007-05-16 20:45:45.000000000 -0700 >> @@ -212,6 +212,7 @@ void container_rss_del(struct page_conta >> >> css_put(&rss->css); >> kfree(pc); >> + init_page_container(page); > > This hunk is bad. > See, when the page drops its mapcount to 0 it may be reused right > after this if it belongs to a file map - another CPU can touch it. > Thus you're risking to reset the wrong container. > > The main idea if the accounting is that you cannot trust the > page_container(page) value after the page's mapcount became 0. > Good catch, I'll move the initialization to free_hot_cold_page(). I'm attaching a new patch. I've also gotten rid of the unused variable page in container_rss_del(). I've compile and boot tested the fix -- Thanks, Balbir Singh Linux Technology Center IBM, ISTL
creator: Balbir Singh
date: 2007-05-24T07:59:55-00:00