Pavel Emelianov wrote: > Balbir Singh wrote: >> Account RSS usage of a task and the associated container. The definition >> of RSS was debated and discussed in the following thread >> >> http://lkml.org/lkml/2006/10/10/130 >> >> >> The code tracks all resident pages (including shared pages) as RSS. This >> patch >> can easily adapt to the definition of RSS that will be agreed upon. This >> implementation provides a proof of concept RSS controller. >> >> The accounting is inspired from Rohit Seth's container patches. >> >> TODO's >> >> 1. Merge file_rss and anon_rss tracking with the current rss tracking to >> maximize code reuse >> 2. Add/remove RSS tracking as the definition of RSS evolves >> >> >> Signed-off-by: Balbir Singh <[EMAIL PROTECTED]> >> --- >> > > [snip] > >> --- linux-2.6.19-rc2/kernel/res_group/memctlr.c~container-memctlr-acct >> 2006-11-09 21:46:22.000000000 +0530 >> +++ linux-2.6.19-rc2-balbir/kernel/res_group/memctlr.c 2006-11-09 >> 21:47:06.000000000 +0530 >> @@ -37,6 +37,8 @@ static struct resource_group *root_rgrou >> static const char version[] = "0.01"; >> static struct memctlr *memctlr_root; >> >> +#define MEMCTLR_MAGIC 0xdededede >> + >> struct mem_counter { >> atomic_long_t rss; >> }; >> @@ -49,6 +51,7 @@ struct memctlr { >> /* Statistics */ >> int successes; >> int failures; >> + int magic; > > What is this magic for? Is it just for debugging? >
Yes > [snip] > >> +static inline struct memctlr *get_memctlr_from_page(struct page *page) >> +{ >> + struct resource_group *rgroup; >> + struct memctlr *res; >> + >> + /* >> + * Is the resource groups infrastructure initialized? >> + */ >> + if (!memctlr_root) >> + return NULL; >> + >> + rcu_read_lock(); >> + rgroup = (struct resource_group *)rcu_dereference(current->container); >> + rcu_read_unlock(); >> + >> + res = get_memctlr(rgroup); >> + if (!res) >> + return NULL; >> + >> + BUG_ON(res->magic != MEMCTLR_MAGIC); >> + return res; >> +} > > I don't see how page passed to this function is involved into > 'struct memctlr *res' determining. Could you comment this? > Yeah, from page is a misnomer. We just use the current task task. I'll fix the naming convention > [snip] > >> --- linux-2.6.19-rc2/mm/rmap.c~container-memctlr-acct 2006-11-09 >> 21:46:22.000000000 +0530 >> +++ linux-2.6.19-rc2-balbir/mm/rmap.c 2006-11-09 21:46:22.000000000 >> +0530 >> @@ -537,6 +537,7 @@ void page_add_anon_rmap(struct page *pag >> if (atomic_inc_and_test(&page->_mapcount)) >> __page_set_anon_rmap(page, vma, address); >> /* else checking page index and mapping is racy */ >> + memctlr_inc_rss(page); >> } >> >> /* >> @@ -553,6 +554,7 @@ void page_add_new_anon_rmap(struct page >> { >> atomic_set(&page->_mapcount, 0); /* elevate count by 1 (starts at -1) */ >> __page_set_anon_rmap(page, vma, address); >> + memctlr_inc_rss(page); >> } >> >> /** >> @@ -565,6 +567,7 @@ void page_add_file_rmap(struct page *pag >> { >> if (atomic_inc_and_test(&page->_mapcount)) >> __inc_zone_page_state(page, NR_FILE_MAPPED); >> + memctlr_inc_rss(page); > > Consider a task maps one file page 100 times in different places > and touches 'all of them'. In this case I see that you'll get > 100 in rss counter while real rss will be just 1. > Hmmm... something for me to think about. Depending on how we define RSS, the code for accounting should be easy to add & modify depending on how we define RSS. But you bring up a very good point. >> } >> >> /** >> @@ -596,8 +599,9 @@ void page_remove_rmap(struct page *page) >> if (page_test_and_clear_dirty(page)) >> set_page_dirty(page); >> __dec_zone_page_state(page, >> - PageAnon(page) ? NR_ANON_PAGES : >> NR_FILE_MAPPED); >> + PageAnon(page) ? NR_ANON_PAGES : >> NR_FILE_MAPPED); > > What is this extra space after a question-mark for? This is again something I changed and looks my undo was not very good. Please ignore it, I'll remove it from the diff. > >> } >> + memctlr_dec_rss(page, mm); >> } >> >> /* >> diff -puN include/linux/rmap.h~container-memctlr-acct include/linux/rmap.h >> --- linux-2.6.19-rc2/include/linux/rmap.h~container-memctlr-acct >> 2006-11-09 21:46:22.000000000 +0530 >> +++ linux-2.6.19-rc2-balbir/include/linux/rmap.h 2006-11-09 >> 21:46:22.000000000 +0530 >> @@ -8,6 +8,7 @@ >> #include <linux/slab.h> >> #include <linux/mm.h> >> #include <linux/spinlock.h> >> +#include <linux/memctlr.h> >> >> /* >> * The anon_vma heads a list of private "related" vmas, to scan if >> @@ -84,6 +85,7 @@ void page_remove_rmap(struct page *); >> static inline void page_dup_rmap(struct page *page) >> { >> atomic_inc(&page->_mapcount); >> + memctlr_inc_rss(page); >> } > > I'm not sure this is correct. page_dup_rmap() happens in the context > of forking process and thus you'll increment rss counter on current. > But this must be incremented at new task's counter, mustn't it? This is fixed in the next patch container-memctlr-task-migration. Thanks for spotting it. -- Thanks, Balbir Singh, Linux Technology Center, IBM Software Labs ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ ckrm-tech mailing list https://lists.sourceforge.net/lists/listinfo/ckrm-tech