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?

[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?

[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.

>  }
>  
>  /**
> @@ -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?

>       }
> +     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?

-------------------------------------------------------------------------
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

Reply via email to