RSS controller based on process containers (v2)
Subsystems (12)
cover RSS controller based on process containers (v2)
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).
PATCH 1/8 Resource counters
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.
include/linux/res_counter.h +83 -0
@@ -0,0 +1,83 @@ +#ifndef __RES_COUNTER_H__ +#define __RES_COUNTER_H__ +/* +* resource counters +* +* Copyright 2007 OpenVZ SWsoft Inc +* +* Author: Pavel Emelianov <[email protected]> +* +*/ + +#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, +unsigned long val) +{ +unsigned long flags; + +spin_lock_irqsave (&cnt->lock, flags); +res_counter_uncharge_locked (cnt, val); +spin_unlock_irqrestore (&cnt->lock, flags); +} + +#endif
init/Kconfig +4 -0
@@ -253,6 +253,10 @@ config CPUSETS Say N if unsure. +config RESOURCE_COUNTERS + bool + select CONTAINERS + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" default y
kernel/Makefile +1 -0
@@ -51,6 +51,7 @@ obj-$(CONFIG_RELAY) += relay.o obj-$(CONFIG_UTS_NS) += utsname.o obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o +obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y) # According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is
kernel/res_counter.c +72 -0
72 lines not on this page · raw message
PATCH 2/8 Add container pointer on struct page
In reply to Pavel EmelianovEach 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.
include/linux/mm.h +6 -0
@@ -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))
include/linux/mm_types.h +3 -0
@@ -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 */
Re: [PATCH 2/8] Add container pointer on struct page
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
Re: [PATCH 2/8] Add container pointer on struct page
Quoted 5 lines · Jean-Pierre Dion
Hi Pavel,
I have been implied in the work for the
memory controller of res groups a few months ago.
Quoted 9 lines
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 ;)
Quoted 5 lines
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.
Quoted 7 lines
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).
Quoted 10 lines
jean-pierre
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
PATCH 3/8 Add container pointer on mm_struct
In reply to Pavel EmelianovNaturally 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.
include/linux/sched.h +3 -0
@@ -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 {
kernel/fork.c +10 -3
@@ -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_mm (mm); }
@@ -493,7 +499,7 @@ static struct mm_struct *dup_mm(struct t mm->token_priority =0 ; mm->last_interval =0 ; -if (!mm_init (mm)) +if (!mm_init (mm, tsk)) goto fail_nomem;if (init_new_context (tsk, mm))
@@ -520,6 +526,7 @@ fail_nocontext:* because it calls destroy_context() */ mm_free_pgd (mm); +mm_free_container (mm);free_mm (mm);return NULL ; }
PATCH 4/8 RSS container core
In reply to Pavel EmelianovThis includes
* definition of rss_container as container subsystem combined with resource counter;
* registration of RSS container in generic containers;
* routines for pages tracking.
include/linux/container_subsys.h +4 -0
@@ -9,6 +9,10 @@ SUBSYS(cpuset)#endif +#ifdef CONFIG_RSS_CONTAINER +SUBSYS(rss) +#endif +/* */ /* */
include/linux/rss_container.h +55 -0
55 lines not on this page · raw message
init/Kconfig +11 -0
@@ -257,6 +257,17 @@ config CPUSETS bool select CONTAINERS +config RSS_CONTAINER + bool "RSS accounting container" + select RESOURCE_COUNTERS + help + Provides a simple Resource Controller for monitoring and + controlling the total Resident Set Size of the tasks in a container + The reclaim logic is now container aware, when the container goes + overlimit the page reclaimer reclaims pages belonging to this + container. If we are unable to reclaim enough pages to satisfy the + request, the process is killed with an out of memory warning. + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" default y
mm/Makefile +2 -0
@@ -29,3 +29,5 @@ obj-$(CONFIG_MEMORY_HOTPLUG) += memory_h obj-$(CONFIG_FS_XIP) += filemap_xip.o obj-$(CONFIG_MIGRATION) += migrate.o obj-$(CONFIG_SMP) += allocpercpu.o + +obj-$(CONFIG_RSS_CONTAINER) += rss_container.o
mm/rss_container.c +274 -0
274 lines not on this page · raw message
PATCH 5/8 RSS accounting hooks over the code
In reply to Pavel EmelianovAs 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.
fs/exec.c +12 -3
@@ -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_strstruct 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);
include/linux/rmap.h +7 -3
@@ -69,9 +69,13 @@ void __anon_vma_link(struct vm_area_stru/* * rmap interfaces called when adding or removing pte of page */ -void page_add_anon_rmap (struct page *,struct vm_area_struct *,unsigned long ); -void page_add_new_anon_rmap (struct page *,struct vm_area_struct *,unsigned long ); -void page_add_file_rmap (struct page *); +struct page_container ; + +void page_add_anon_rmap (struct page *,struct vm_area_struct *, +unsigned long ,struct page_container *); +void page_add_new_anon_rmap (struct page *,struct vm_area_struct *, +unsigned long ,struct page_container *); +void page_add_file_rmap (struct page *,struct page_container *);void page_remove_rmap (struct page *,struct vm_area_struct *);/**
mm/fremap.c +10 -1
@@ -20,6 +20,8 @@#include <asm/cacheflush.h> #include <asm/tlbflush.h> +#include <linux/rss_container.h> +static int zap_pte (struct mm_struct *mm,struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {
@@ -57,6 +59,10 @@ int install_page(struct mm_struct *mm, spte_t *pte;pte_t pte_val;spinlock_t *ptl; +struct page_container *pcont; + +if (container_rss_prepare (page, vma, &pcont)) + goto out_release; pte =get_locked_pte (mm, addr, &ptl);if (!pte)
@@ -81,13 +87,16 @@ int install_page(struct mm_struct *mm, sflush_icache_page (vma, page); pte_val =mk_pte (page, prot);set_pte_at (mm, addr, pte, pte_val); -page_add_file_rmap (page); +page_add_file_rmap (page, pcont);update_mmu_cache (vma, addr, pte_val);lazy_mmu_prot_update (pte_val); err =0 ; unlock:pte_unmap_unlock (pte, ptl); out: +if (err !=0 ) +container_rss_release (pcont); +out_release:return err; }EXPORT_SYMBOL (install_page);
mm/memory.c +42 -10
188 lines not on this page · raw message
mm/migrate.c +12 -2
@@ -28,6 +28,7 @@#include <linux/mempolicy.h> #include <linux/vmalloc.h> #include <linux/security.h> +#include <linux/rss_container.h> #include "internal.h"
@@ -134,6 +135,7 @@ static void remove_migration_pte(structpte_t *ptep, pte;spinlock_t *ptl;unsigned long addr =page_address_in_vma (new, vma); +struct page_container *pcont;if (addr == -EFAULT )return ;
@@ -157,6 +159,11 @@ static void remove_migration_pte(structreturn ; } +if (container_rss_prepare (new, vma, &pcont)) { +pte_unmap (ptep); +return ; + } + ptl =pte_lockptr (mm, pmd);spin_lock (ptl); pte = *ptep;
@@ -175,16 +182,19 @@ static void remove_migration_pte(structset_pte_at (mm, addr, ptep, pte);if (PageAnon (new)) -page_add_anon_rmap (new, vma, addr); +page_add_anon_rmap (new, vma, addr, pcont);else -page_add_file_rmap (new); +page_add_file_rmap (new, pcont);/* No need to invalidate - it was non-present before */ update_mmu_cache (vma, addr, pte);lazy_mmu_prot_update (pte); +pte_unmap_unlock (ptep, ptl); +return ; out:pte_unmap_unlock (ptep, ptl); +container_rss_release (pcont); }/*
mm/rmap.c +26 -6
@@ -51,6 +51,8 @@#include <asm/tlbflush.h> +#include <linux/rss_container.h> +struct kmem_cache *anon_vma_cachep;static inline void validate_anon_vma (struct vm_area_struct *find_vma)
@@ -526,14 +528,19 @@ static void __page_set_anon_rmap(struct* @page: the page to add the mapping to * @vma: the vm area in which the mapping is added * @address: the user virtual address mapped +* @pcont: the page beancounter to charge page with * * The caller needs to hold the pte lock. */ void page_add_anon_rmap (struct page *page, -struct vm_area_struct *vma,unsigned long address) +struct vm_area_struct *vma,unsigned long address, +struct page_container *pcont) { -if (atomic_inc_and_test (&page->_mapcount)) +if (atomic_inc_and_test (&page->_mapcount)) { +container_rss_add (pcont);__page_set_anon_rmap (page, vma, address); + }else +container_rss_release (pcont);/* else checking page index and mapping is racy */ }
@@ -542,27 +549,35 @@ void page_add_anon_rmap(struct page *pag* @page: the page to add the mapping to * @vma: the vm area in which the mapping is added * @address: the user virtual address mapped +* @pcont: the page beancounter to charge page with * * Same as page_add_anon_rmap but must only be called on *new* pages. * This means the inc-and-test can be bypassed. */ void page_add_new_anon_rmap (struct page *page, -struct vm_area_struct *vma,unsigned long address) +struct vm_area_struct *vma,unsigned long address, +struct page_container *pcont) {atomic_set (&page->_mapcount,0 );/* elevate count by 1 (starts at -1) */ +container_rss_add (pcont);__page_set_anon_rmap (page, vma, address); }/** * page_add_file_rmap - add pte mapping to a file page -* @page: the page to add the mapping to +* @page: the page to add the mapping to +* @pcont: the page beancounter to charge page with * * The caller needs to hold the pte lock. */ -void page_add_file_rmap (struct page *page) +void page_add_file_rmap (struct page *page,struct page_container *pcont) { -if (atomic_inc_and_test (&page->_mapcount)) +if (atomic_inc_and_test (&page->_mapcount)) { +if (pcont) +container_rss_add (pcont);__inc_zone_page_state (page,NR_FILE_MAPPED ); + }else if (pcont) +container_rss_release (pcont); }/**
@@ -573,6 +588,9 @@ void page_add_file_rmap(struct page *pag*/ void page_remove_rmap (struct page *page,struct vm_area_struct *vma) { +struct page_container *pcont; + + pcont =page_container (page);if (atomic_add_negative (-1 , &page->_mapcount)) {if (unlikely (page_mapcount (page) <0 )) {printk (KERN_EMERG "Eeek! page_mapcount(page) went negative! (%d)\n" ,page_mapcount (page));
@@ -588,6 +606,8 @@ void page_remove_rmap(struct page *page,BUG (); } +if (pcont) +container_rss_del (pcont);/* * It would be tidy to reset the PageAnon mapping here, * but that might overwrite a racing page_add_anon_rmap
mm/swapfile.c +12 -3
@@ -32,6 +32,8 @@#include <asm/tlbflush.h> #include <linux/swapops.h> +#include <linux/rss_container.h> +DEFINE_SPINLOCK (swap_lock);unsigned int nr_swapfiles;long total_swap_pages;
@@ -507,13 +509,14 @@ unsigned int count_swap_pages(int type,* force COW, vm_page_prot omits write permission from any private vma. */ static void unuse_pte (struct vm_area_struct *vma,pte_t *pte, -unsigned long addr,swp_entry_t entry,struct page *page) +unsigned long addr,swp_entry_t entry,struct page *page, +struct page_container *pcont) {inc_mm_counter (vma->vm_mm, anon_rss);get_page (page);set_pte_at (vma->vm_mm, addr, pte,pte_mkold (mk_pte (page, vma->vm_page_prot))); -page_add_anon_rmap (page, vma, addr); +page_add_anon_rmap (page, vma, addr, pcont);swap_free (entry);/* * Move the page to the active list so it is not
@@ -530,6 +533,10 @@ static int unuse_pte_range(struct vm_arepte_t *pte;spinlock_t *ptl;int found =0 ; +struct page_container *pcont; + +if (container_rss_prepare (page, vma, &pcont)) +return 0 ; pte =pte_offset_map_lock (vma->vm_mm, pmd, addr, &ptl); do {
@@ -538,12 +545,14 @@ static int unuse_pte_range(struct vm_are* Test inline before going to call unuse_pte. */ if (unlikely (pte_same (*pte, swp_pte))) { -unuse_pte (vma, pte++, addr, entry, page); +unuse_pte (vma, pte++, addr, entry, page, pcont); found =1 ;break ; } }while (pte++, addr +=PAGE_SIZE , addr != end);pte_unmap_unlock (pte -1 , ptl); +if (!found) +container_rss_release (pcont);return found; }
PATCH 6/8 Per container OOM killer
In reply to Pavel EmelianovWhen 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.
include/linux/rss_container.h +1 -0
@@ -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);
mm/oom_kill.c +38 -4
81 lines not on this page · raw message
PATCH 7/8 Page scanner changes needed to implement per-container scanner
In reply to Pavel EmelianovStruct scan_control now carries:
* the RSS container to free pages in;
* pointer to an isolate_pages() function to isolate pages needed for the reclamation.
mm/vmscan.c +57 -23
@@ -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(ureturn 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_lisunsigned 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 ); zone->pages_scanned += pgscanned; zone->nr_active -= pgmoved;spin_unlock_irq (&zone->lru_lock);
@@ -1012,7 +1031,9 @@ static unsigned long shrink_zones(int pr* holds filesystem locks which prevent writeout this might not work, and the * allocation attempt will fail. */ -unsigned long try_to_free_pages (struct zone **zones,gfp_t gfp_mask) + +static unsigned long do_try_to_free_pages (struct zone **zones,gfp_t gfp_mask, +struct scan_control *sc) {int priority;int ret =0 ;
@@ -1021,13 +1042,6 @@ unsigned long try_to_free_pages(struct zstruct reclaim_state *reclaim_state = current->reclaim_state;unsigned long lru_pages =0 ;int i; -struct scan_control sc = { - .gfp_mask = gfp_mask, - .may_writepage = !laptop_mode, - .swap_cluster_max =SWAP_CLUSTER_MAX , - .may_swap =1 , - .swappiness = vm_swappiness, - };count_vm_event (ALLOCSTALL );
@@ -1041,17 +1055,18 @@ unsigned long try_to_free_pages(struct z }for (priority =DEF_PRIORITY ; priority >=0 ; priority--) { - sc.nr_scanned =0 ; + sc->nr_scanned =0 ;if (!priority)disable_swap_token (); - nr_reclaimed +=shrink_zones (priority, zones, &sc); -shrink_slab (sc.nr_scanned, gfp_mask, lru_pages); + nr_reclaimed +=shrink_zones (priority, zones, sc); +if (sc->cnt ==NULL ) +shrink_slab (sc->nr_scanned, gfp_mask, lru_pages);if (reclaim_state) { nr_reclaimed += reclaim_state->reclaimed_slab; reclaim_state->reclaimed_slab =0 ; } - total_scanned += sc.nr_scanned; -if (nr_reclaimed >= sc.swap_cluster_max) { + total_scanned += sc->nr_scanned; +if (nr_reclaimed >= sc->swap_cluster_max) { ret =1 ; goto out; }
@@ -1063,18 +1078,18 @@ unsigned long try_to_free_pages(struct z* that's undesirable in laptop mode, where we *want* lumpy * writeout. So in laptop mode, write out the whole world. */ -if (total_scanned > sc.swap_cluster_max + - sc.swap_cluster_max /2 ) { +if (total_scanned > sc->swap_cluster_max + + sc->swap_cluster_max /2 ) {wakeup_pdflush (laptop_mode ?0 : total_scanned); - sc.may_writepage =1 ; + sc->may_writepage =1 ; }/* Take a nap, wait for some writeback to complete */ -if (sc.nr_scanned && priority <DEF_PRIORITY -2 ) +if (sc->nr_scanned && priority <DEF_PRIORITY -2 )congestion_wait (WRITE ,HZ /10 ); }/* top priority shrink_caches still had more to do? don't OOM, then */ -if (!sc.all_unreclaimable) +if (!sc->all_unreclaimable) ret =1 ; out:/*
@@ -1097,6 +1112,21 @@ out:return ret; } +unsigned long try_to_free_pages (struct zone **zones,gfp_t gfp_mask) +{ +struct scan_control sc = { + .gfp_mask = gfp_mask, + .may_writepage = !laptop_mode, + .swap_cluster_max =SWAP_CLUSTER_MAX , + .may_swap =1 , + .swappiness = vm_swappiness, + .cnt =NULL , + .isolate_pages = isolate_pages_global, + }; + +return do_try_to_free_pages (zones, gfp_mask, &sc); +} +/* * For kswapd, balance_pgdat() will work across all this node's zones until * they are all at pages_high.
@@ -1131,6 +1190,8 @@ static unsigned long balance_pgdat(pg_da .may_swap = 1, .swap_cluster_max = SWAP_CLUSTER_MAX, .swappiness = vm_swappiness, + .cnt = NULL, + .isolate_pages = isolate_pages_global, };/* * temp_priority is used to remember the scanning priority at which
@@ -1436,6 +1497,8 @@ unsigned long shrink_all_memory(unsigned .swap_cluster_max = nr_pages, .may_writepage = 1, .swappiness = vm_swappiness, + .cnt = NULL, + .isolate_pages = isolate_pages_global, }; current->reclaim_state = &reclaim_state;
@@ -1619,6 +1682,8 @@ static int __zone_reclaim(struct zone *z SWAP_CLUSTER_MAX), .gfp_mask = gfp_mask, .swappiness = vm_swappiness, + .cnt = NULL, + .isolate_pages = isolate_pages_global, };unsigned long slab_reclaimable;
PATCH 8/8 Per-container pages reclamation
In reply to Pavel EmelianovImplement 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.
include/linux/swap.h +3 -0
@@ -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);
mm/vmscan.c +31 -0
@@ -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 ].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 +unsigned long try_to_free_pages (struct zone **zones,gfp_t gfp_mask) {struct scan_control sc = {
mm/swap.c +2 -0
@@ -30,6 +30,7 @@#include <linux/cpu.h> #include <linux/notifier.h> #include <linux/init.h> +#include <linux/rss_container.h> /* How many pages do we try to swap or page in/out together? */ int page_cluster;
@@ -147,6 +148,7 @@ void fastcall activate_page(struct pageSetPageActive (page);add_page_to_active_list (zone, page);__count_vm_event (PGACTIVATE ); +container_rss_move_lists (page,1 ); }spin_unlock_irq (&zone->lru_lock); }
mm/rss_container.c +72 -0
78 lines not on this page · raw message
include/linux/rss_container.h +6 -0
@@ -24,6 +24,10 @@void mm_init_container (struct mm_struct *mm,struct task_struct *tsk);void mm_free_container (struct mm_struct *mm); +void container_rss_move_lists (struct page *pg,bool active); +unsigned long isolate_pages_in_container (unsigned long nr_to_scan, +struct list_head *dst,unsigned long *scanned, +struct zone *zone,struct rss_container *,int active);#else static inline int container_rss_prepare (struct page *pg,struct vm_area_struct *vma,struct page_container **pc)
@@ -52,5 +56,7 @@ { } +#define isolate_container_pages (nr, dst, scanned, rss, act, zone) ({ BUG(); 0;}) +#define container_rss_move_lists (pg, active) do { } while (0)#endif #endif
Re: [PATCH 8/8] Per-container pages reclamation
Quoted 6 lines · Pavel Emelianov
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
Quoted 5 lines
mm/rss_container.c +5
-0+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 ;
mm/rss_container.c line 135
Quoted 10 lines
mm/rss_container.c +10
-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.
Quoted 3 lines
mm/rss_container.c +3
-0+ +list_move (&pc->list, &pc_list); +
Signature · 4 lines
Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
Re: [PATCH 8/8] Per-container pages reclamation
Quoted 22 lines · Balbir Singh
Quoted 6 lines · Pavel Emelianov
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
Quoted 9 lines
mm/rss_container.c +9
-0+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);
Quoted 8 lines
mm/rss_container.c line 135
Quoted 6 lines
mm/rss_container.c +6
-0+ +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.
Quoted 10 lines
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.
Quoted 3 lines
mm/rss_container.c +3
-0+ +list_move (&pc->list, &pc_list); +
Re: [PATCH 8/8] Per-container pages reclamation
Quoted 30 lines · Pavel Emelianov
Quoted 12 lines · Balbir Singh
Quoted 6 lines · Pavel Emelianov
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
Quoted 16 lines
Quoted 5 lines
mm/rss_container.c +5
-0+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 ;mm/rss_container.c line 135
Quoted 10 lines
mm/rss_container.c +10
-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
Quoted 3 lines
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.
Quoted 10 lines
Quoted 9 lines
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.
Quoted 3 lines
mm/rss_container.c +3
-0+ +list_move (&pc->list, &pc_list); +
Signature · 4 lines
Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
Re: [PATCH 8/8] Per-container pages reclamation
Quoted 31 lines · Balbir Singh
Quoted 30 lines · Pavel Emelianov
Quoted 28 lines · Balbir Singh
Quoted 6 lines · Pavel Emelianov
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
Quoted 5 lines
mm/rss_container.c +5
-0+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 ;mm/rss_container.c line 135
Quoted 10 lines
mm/rss_container.c +10
-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
Quoted 7 lines
Quoted 4 lines
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.
Quoted 2 lines
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.
Quoted 13 lines
Quoted 11 lines
Quoted 10 lines
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.
Quoted 3 lines
mm/rss_container.c +3
-0+ +list_move (&pc->list, &pc_list); +
Re: [PATCH 8/8] Per-container pages reclamation
In reply to Pavel EmelianovQuoted 25 lines · Pavel Emelianov
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.(quoted diff) +19
-0+#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))
Review
Quoted 10 lines
(quoted diff) +10
-0+ 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.
---
mm/vmscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/vmscan.c renamed from mm/vmscan.c~rss-fix-nodescan +1 -1
@@ -1127,7 +1127,7 @@ unsigned long try_to_free_pages_in_containt 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))
_
Signature · 4 lines
Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
Re: [PATCH 8/8] Per-container pages reclamation
In reply to Pavel EmelianovQuoted 7 lines · Pavel Emelianov
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+0x180/0x338 [<ffffffff80270474>] __handle_mm_fault+0x1f2/0xa81 [<ffffffff804c58ef>] do_page_fault+0x42b/0x7b3 [<ffffffff802484c4>] hrtimer_cancel+0xc/0x16 [<ffffffff804c2a89>] do_nanosleep+0x47/0x70 [<ffffffff802485f4>] hrtimer_nanosleep+0x58/0x119 [<ffffffff8023bc1f>] sys_sysinfo+0x15b/0x173 [<ffffffff804c3d3d>] error_exit+0x0/0x84
On analyzing the code, I found that the page is mapped (we have a page_mapped() check in container_rss_move_lists()), but the page_container is invalid. Please review the fix attached (we reset the page's container pointer to NULL when a page is completely unmapped)
Signature · 4 lines
Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
mm/rss_container.c +1 -0
@@ -212,6 +212,7 @@ void container_rss_del(struct page_contacss_put (&rss->css);kfree (pc); +init_page_container (page); } static void rss_move_task(struct container_subsys *ss,
mm/page_alloc.c +2 -0
@@ -41,6 +41,7 @@#include <linux/pfn.h> #include <linux/backing-dev.h> #include <linux/fault-inject.h> +#include <linux/rss_container.h> #include <asm/tlbflush.h> #include <asm/div64.h>
@@ -1977,6 +1978,7 @@ void __meminit memmap_init_zone(unsignedset_page_links (page, zone, nid, pfn);init_page_count (page);reset_page_mapcount (page); +init_page_container (page);SetPageReserved (page);INIT_LIST_HEAD (&page->lru);#ifdef WANT_PAGE_VIRTUAL
include/linux/rss_container.h +9 -0
@@ -28,6 +28,11 @@ void container_rss_move_lists(struct pagunsigned long isolate_pages_in_container (unsigned long nr_to_scan,struct list_head *dst,unsigned long *scanned,struct zone *zone,struct rss_container *,int active); +static inline void init_page_container (struct page *page) +{ +page_container (page) =NULL ; +} +#else static inline int container_rss_prepare (struct page *pg,struct vm_area_struct *vma,struct page_container **pc)
@@ -56,6 +61,10 @@ static inline void mm_free_container(str { } +static inline void init_page_container (struct page *page) +{ +} +#define isolate_container_pages (nr, dst, scanned, rss, act, zone) ({ BUG(); 0;})#define container_rss_move_lists (pg, active) do { } while (0)#endif
Re: [PATCH 8/8] Per-container pages reclamation
Quoted 6 lines · Balbir Singh
Quoted 5 lines · Pavel Emelianov
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.
Quoted 58 lines
Quoted 1 line
Hi, Pavel/Andrew,
I've started running some basic tests like lmbench and LTP vm stress on the RSS controller.
With the controller
rss_limitset to 256 MB, I saw the following panic on a machineUnable 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+0x180/0x338 [<ffffffff80270474>] __handle_mm_fault+0x1f2/0xa81 [<ffffffff804c58ef>] do_page_fault+0x42b/0x7b3 [<ffffffff802484c4>] hrtimer_cancel+0xc/0x16 [<ffffffff804c2a89>] do_nanosleep+0x47/0x70 [<ffffffff802485f4>] hrtimer_nanosleep+0x58/0x119 [<ffffffff8023bc1f>] sys_sysinfo+0x15b/0x173 [<ffffffff804c3d3d>] error_exit+0x0/0x84On analyzing the code, I found that the page is mapped (we have a
page_mapped()check incontainer_rss_move_lists()), but thepage_containeris invalid. Please review the fix attached (we reset the page's container pointer to NULL when a page is completely unmapped)------------------------------------------------------------------------
Index:
linux-2.6.20/mm/rss_container.c
===================================================================
mm/rss_container.c void container_rss_del(struct page_conta · line 215
Quoted 4 lines
mm/rss_container.c +1
-0@@ -212,6 +212,7 @@ void container_rss_del(struct page_contacss_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.
Quoted 40 lines
mm/rss_container.c +0
-0} static void rss_move_task(struct container_subsys *ss,Index:
linux-2.6.20/mm/page_alloc.c
===================================================================mm/page_alloc.c +2
-0@@ -41,6 +41,7 @@#include <linux/pfn.h> #include <linux/backing-dev.h> #include <linux/fault-inject.h> +#include <linux/rss_container.h> #include <asm/tlbflush.h> #include <asm/div64.h> @@ -1977,6 +1978,7 @@ void __meminit memmap_init_zone(unsignedset_page_links (page, zone, nid, pfn);init_page_count (page);reset_page_mapcount (page); +init_page_container (page);SetPageReserved (page);INIT_LIST_HEAD (&page->lru);#ifdef WANT_PAGE_VIRTUAL Index:
linux-2.6.20/include/linux/rss_container.h
===================================================================include/linux/rss_container.h +5
-0@@ -28,6 +28,11 @@ void container_rss_move_lists(struct pagunsigned long isolate_pages_in_container (unsigned long nr_to_scan,struct list_head *dst,unsigned long *scanned,struct zone *zone,struct rss_container *,int active); +static inline void init_page_container (struct page *page) +{ +page_container (page) =NULL ; +} +#else static inline int container_rss_prepare (struct page *pg,struct vm_area_struct *vma,struct page_container **pc)
include/linux/rss_container.h static inline void mm_free_container(str · line 70
Quoted 10 lines
include/linux/rss_container.h +4
-0@@ -56,6 +61,10 @@ static inline void mm_free_container(str { } +static inline void init_page_container (struct page *page) +{ +} +#define isolate_container_pages (nr, dst, scanned, rss, act, zone) ({ BUG(); 0;})#define container_rss_move_lists (pg, active) do { } while (0)#endif
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 8/8] Per-container pages reclamation
Quoted 15 lines · Pavel Emelianov
mm/rss_container.c void container_rss_del(struct page_conta · line 215
Quoted 9 lines
Index:
linux-2.6.20/mm/rss_container.c
===================================================================mm/rss_container.c +1
-0@@ -212,6 +212,7 @@ void container_rss_del(struct page_contacss_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.
Quoted 3 lines
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
Signature · 4 lines
Thanks, Balbir Singh Linux Technology Center IBM, ISTL
---
mm/page_alloc.c +3 -0
@@ -41,6 +41,7 @@#include <linux/pfn.h> #include <linux/backing-dev.h> #include <linux/fault-inject.h> +#include <linux/rss_container.h> #include <asm/tlbflush.h> #include <asm/div64.h>
@@ -791,6 +792,7 @@ static void fastcall free_hot_cold_page(if (!PageHighMem (page))debug_check_no_locks_freed (page_address (page),PAGE_SIZE ); +init_page_container (page);arch_free_page (page,0 );kernel_map_pages (page,1 ,0 );
@@ -1977,6 +1979,7 @@ void __meminit memmap_init_zone(unsignedset_page_links (page, zone, nid, pfn);init_page_count (page);reset_page_mapcount (page); +init_page_container (page);SetPageReserved (page);INIT_LIST_HEAD (&page->lru);#ifdef WANT_PAGE_VIRTUAL
include/linux/rss_container.h +9 -0
@@ -28,6 +28,11 @@ void container_rss_move_lists(struct pagunsigned long isolate_pages_in_container (unsigned long nr_to_scan,struct list_head *dst,unsigned long *scanned,struct zone *zone,struct rss_container *,int active); +static inline void init_page_container (struct page *page) +{ +page_container (page) =NULL ; +} +#else static inline int container_rss_prepare (struct page *pg,struct vm_area_struct *vma,struct page_container **pc)
@@ -56,6 +61,10 @@ static inline void mm_free_container(str { } +static inline void init_page_container (struct page *page) +{ +} +#define isolate_container_pages (nr, dst, scanned, rss, act, zone) ({ BUG(); 0;})#define container_rss_move_lists (pg, active) do { } while (0)#endif
mm/rss_container.c +0 -3
@@ -199,12 +199,9 @@ void container_rss_add(struct page_contavoid container_rss_del (struct page_container *pc) { -struct page *page;struct rss_container *rss; - page = pc->page; rss = pc->cnt; -spin_lock_irq (&rss->res.lock);list_del (&pc->list);res_counter_uncharge_locked (&rss->res,1 );
*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 fashion.
The biggest problem with this approach is that there is no per zone reclaim left, which is relied upon by the allocator to provide free pages in a given physical address range. However there has been talk to create a proper range allocator independent of zones.
Just my 0.02 euro..
Peter
Quoted 4 lines · Peter Zijlstra
*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.
Quoted 4 lines
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.
Quoted 2 lines
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.
Quoted 3 lines
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.
Quoted 11 lines
Let us call
struct reclaim_struct { struct list_head active_list; struct list_head inactive_list; unsigned long nr_active; unsigned long nr_inactive; }
Quoted 4 lines
Lets recognise three distinct page categories:
- anonymous memory,
- mapped pagecache, and
- unmapped pagecache.
We cannot split the user memory in parts. There must be some overall parameter that will allow administrator to say "Well, let us run this container in a 64Mb sandbox". With the anonymous and mapped memory separated administrator will be a bit confused.
Quoted 4 lines
We then keep anonymous pages on a per container
reclaim_struct, these pages are fully accounted to each container.
Hmm... We do have such a construction. struct rss_container has two lists and we shrink from them sequentially using an existing scanner. Don't forget that this scanner has been evolving for many years and writing a new scanner is just a waste of time.
Quoted 4 lines
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.
What happens if one container fills the RAM with mapped pages from a single file? Who will be the "owner" of this page set? Who will expend its IO bandwidth to push these pages on disk? What if this container will mlock() this set? Who will be killed?
Quoted 9 lines
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 perhapsadd_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.
Yet again. The current scanner came out from the work of many people. This is a very tricky place that is still evolving. Do you propose to throw this out and write a new scanner?
Quoted 2 lines
The slab shrinker should be called proportional to the containers size relative to the machine.
The slab shrinker must be called only if we do know what kernel objects are used by this particular container. Otherwise we break the idea of isolation. Generally speaking if some container runs out of its resources we should reclaim pages, shrink objects, kill tasks, etc from this container only.
Quoted 19 lines
Global reclaim will have to call each container reclaim in proportional fashion.
The biggest problem with this approach is that there is no per zone reclaim left, which is relied upon by the allocator to provide free pages in a given physical address range. However there has been talk to create a proper range allocator independent of zones.
Just my 0.02 euro..
Peter
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Quoted 17 lines · Pavel Emelianov
Quoted 4 lines · Peter Zijlstra
*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.
Quoted 8 lines
Quoted 4 lines
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
Quoted 8 lines
Quoted 2 lines
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]