[PATCH 0/7] memcg numa node scan update.

[PATCH 0/7] memcg numa node scan update.

[KAMEZAWA Hiroyuki]

In the last month, I added round-robin scan of numa nodes at
hittling limit, and wrote "a better algorithm is needed."

Here is update. Because some of patches are bugfixes, I may
cut out them as independent patch.

Pathc 6-7/7 implements a selection logic.

==
Tested on 8cpu/24GB system, which has 2 nodes.
limit memory to 300MB and run httpd under it.
httpd's working set is 4096files/600MB.

Then, do 40960 access by apache-bench. and see how memory reclaim costs.
Because a thread of httpd doesn't consume cpu much, the number of
working threads are not balanced between numa nodes and file caches
will be not balanced.

[round-robin]
 [kamezawa@bluextal ~]$ cat /cgroup/memory/test/memory.scan_stat
  scanned_pages_by_limit 550740
  freed_pages_by_limit 206473
  elapsed_ns_by_limit 9485418834

[After patch]
  scanned_pages_by_limit 521520
  freed_pages_by_limit 199330
  elapsed_ns_by_limit 7904913234

I can see elapsed time is decreased.
Test on big machine is welcomed.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/7] Fix mem_cgroup_hierarchical_reclaim() to do stable hierarchy walk.

[KAMEZAWA Hiroyuki]

patch is onto mmotm-06-15.
==

Re: [PATCH 1/7] Fix mem_cgroup_hierarchical_reclaim() to do stable hierarchy walk.

[Michal Hocko]

On Thu 16-06-11 12:51:41, KAMEZAWA Hiroyuki wrote:
[...]
> @@ -1667,41 +1668,28 @@ static int mem_cgroup_hierarchical_recla
>  	if (!check_soft && root_mem->memsw_is_minimum)
>  		noswap = true;
>  
> -	while (1) {
> +again:
> +	if (!shrink) {
> +		visit = 0;
> +		for_each_mem_cgroup_tree(victim, root_mem)
> +			visit++;
> +	} else {
> +		/*
> +		 * At shrinking, we check the usage again in caller side.
> +		 * so, visit children one by one.
> +		 */
> +		visit = 1;
> +	}
> +	/*
> +	 * We are not draining per cpu cached charges during soft limit reclaim
> +	 * because global reclaim doesn't care about charges. It tries to free
> +	 * some memory and  charges will not give any.
> +	 */
> +	if (!check_soft)
> +		drain_all_stock_async(root_mem);
> +
> +	while (visit--) {

This is racy, isn't it? What prevents some groups to disapear in the
meantime? We would reclaim from those that are left more that we want.

Why cannot we simply do something like (totally untested):

Index: linus_tree/mm/memcontrol.c
===================================================================
--- linus_tree.orig/mm/memcontrol.c	2011-06-22 17:11:54.000000000 +0200
+++ linus_tree/mm/memcontrol.c	2011-06-22 17:13:05.000000000 +0200
@@ -1652,7 +1652,7 @@ static int mem_cgroup_hierarchical_recla
 						unsigned long reclaim_options,
 						unsigned long *total_scanned)
 {
-	struct mem_cgroup *victim;
+	struct mem_cgroup *victim, *first_victim = NULL;
 	int ret, total = 0;
 	int loop = 0;
 	bool noswap = reclaim_options & MEM_CGROUP_RECLAIM_NOSWAP;
@@ -1669,6 +1669,11 @@ static int mem_cgroup_hierarchical_recla
 
 	while (1) {
 		victim = mem_cgroup_select_victim(root_mem);
+		if (!first_victim)
+			first_victim = victim;
+		else if (first_victim == victim)
+			break;
+
 		if (victim == root_mem) {
 			loop++;
 			/*
-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/7] Fix mem_cgroup_hierarchical_reclaim() to do stable hierarchy walk.

[Michal Hocko]

On Wed 22-06-11 17:15:00, Michal Hocko wrote:
> On Thu 16-06-11 12:51:41, KAMEZAWA Hiroyuki wrote:
> [...]
> > @@ -1667,41 +1668,28 @@ static int mem_cgroup_hierarchical_recla
> >  	if (!check_soft && root_mem->memsw_is_minimum)
> >  		noswap = true;
> >  
> > -	while (1) {
> > +again:
> > +	if (!shrink) {
> > +		visit = 0;
> > +		for_each_mem_cgroup_tree(victim, root_mem)
> > +			visit++;
> > +	} else {
> > +		/*
> > +		 * At shrinking, we check the usage again in caller side.
> > +		 * so, visit children one by one.
> > +		 */
> > +		visit = 1;
> > +	}
> > +	/*
> > +	 * We are not draining per cpu cached charges during soft limit reclaim
> > +	 * because global reclaim doesn't care about charges. It tries to free
> > +	 * some memory and  charges will not give any.
> > +	 */
> > +	if (!check_soft)
> > +		drain_all_stock_async(root_mem);
> > +
> > +	while (visit--) {
> 
> This is racy, isn't it? What prevents some groups to disapear in the
> meantime? We would reclaim from those that are left more that we want.
> 
> Why cannot we simply do something like (totally untested):
> 
> Index: linus_tree/mm/memcontrol.c
> ===================================================================
> --- linus_tree.orig/mm/memcontrol.c	2011-06-22 17:11:54.000000000 +0200
> +++ linus_tree/mm/memcontrol.c	2011-06-22 17:13:05.000000000 +0200
> @@ -1652,7 +1652,7 @@ static int mem_cgroup_hierarchical_recla
>  						unsigned long reclaim_options,
>  						unsigned long *total_scanned)
>  {
> -	struct mem_cgroup *victim;
> +	struct mem_cgroup *victim, *first_victim = NULL;
>  	int ret, total = 0;
>  	int loop = 0;
>  	bool noswap = reclaim_options & MEM_CGROUP_RECLAIM_NOSWAP;
> @@ -1669,6 +1669,11 @@ static int mem_cgroup_hierarchical_recla
>  
>  	while (1) {
>  		victim = mem_cgroup_select_victim(root_mem);
> +		if (!first_victim)
> +			first_victim = victim;
> +		else if (first_victim == victim)
> +			break;

this will obviously need css_get and css_put to make sure that the group
doesn't disappear in the meantime.

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/7] Fix mem_cgroup_hierarchical_reclaim() to do stable hierarchy walk.

[KAMEZAWA Hiroyuki]

On Wed, 22 Jun 2011 20:33:31 +0200
Michal Hocko <mhocko@suse.cz> wrote:

> On Wed 22-06-11 17:15:00, Michal Hocko wrote:
> > On Thu 16-06-11 12:51:41, KAMEZAWA Hiroyuki wrote:
> > [...]
> > > @@ -1667,41 +1668,28 @@ static int mem_cgroup_hierarchical_recla
> > >  	if (!check_soft && root_mem->memsw_is_minimum)
> > >  		noswap = true;
> > >  
> > > -	while (1) {
> > > +again:
> > > +	if (!shrink) {
> > > +		visit = 0;
> > > +		for_each_mem_cgroup_tree(victim, root_mem)
> > > +			visit++;
> > > +	} else {
> > > +		/*
> > > +		 * At shrinking, we check the usage again in caller side.
> > > +		 * so, visit children one by one.
> > > +		 */
> > > +		visit = 1;
> > > +	}
> > > +	/*
> > > +	 * We are not draining per cpu cached charges during soft limit reclaim
> > > +	 * because global reclaim doesn't care about charges. It tries to free
> > > +	 * some memory and  charges will not give any.
> > > +	 */
> > > +	if (!check_soft)
> > > +		drain_all_stock_async(root_mem);
> > > +
> > > +	while (visit--) {
> > 
> > This is racy, isn't it? What prevents some groups to disapear in the
> > meantime? We would reclaim from those that are left more that we want.
> > 
> > Why cannot we simply do something like (totally untested):
> > 
> > Index: linus_tree/mm/memcontrol.c
> > ===================================================================
> > --- linus_tree.orig/mm/memcontrol.c	2011-06-22 17:11:54.000000000 +0200
> > +++ linus_tree/mm/memcontrol.c	2011-06-22 17:13:05.000000000 +0200
> > @@ -1652,7 +1652,7 @@ static int mem_cgroup_hierarchical_recla
> >  						unsigned long reclaim_options,
> >  						unsigned long *total_scanned)
> >  {
> > -	struct mem_cgroup *victim;
> > +	struct mem_cgroup *victim, *first_victim = NULL;
> >  	int ret, total = 0;
> >  	int loop = 0;
> >  	bool noswap = reclaim_options & MEM_CGROUP_RECLAIM_NOSWAP;
> > @@ -1669,6 +1669,11 @@ static int mem_cgroup_hierarchical_recla
> >  
> >  	while (1) {
> >  		victim = mem_cgroup_select_victim(root_mem);
> > +		if (!first_victim)
> > +			first_victim = victim;
> > +		else if (first_victim == victim)
> > +			break;
> 
> this will obviously need css_get and css_put to make sure that the group
> doesn't disappear in the meantime.
> 

I forgot why we didn't this. Hmm, ok, I'll use this style.

Thanks,
-Kame




--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/7] export memory cgroup's swappines by mem_cgroup_swappiness()

[KAMEZAWA Hiroyuki]

Re: [PATCH 2/7] export memory cgroup's swappines by mem_cgroup_swappiness()

[Michal Hocko]

On Thu 16-06-11 12:52:22, KAMEZAWA Hiroyuki wrote:
> From 6f9c40172947fb92ab0ea6f7d73d577473879636 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Wed, 15 Jun 2011 12:06:31 +0900
> Subject: [PATCH 2/7] export memory cgroup's swappines by mem_cgroup_swappiness()
> 
> Each memory cgroup has 'swappiness' value and it can be accessed by
> get_swappiness(memcg). The major user is try_to_free_mem_cgroup_pages()
> and swappiness is passed by argument.
> 
> It's now static function but some planned updates will need to
> get swappiness from files other than memcontrol.c
> This patch exports get_swappiness() as mem_cgroup_swappiness().
> By this, we can remove the argument of swapiness from try_to_fre...
> 
> I think this makes sense because passed swapiness is always from memory
> cgroup passed as an argument and this duplication of argument is
> not very good.

Yes makes sense and it makes it more looking like a global reclaim.

> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Reviewed-by: Michal Hocko <mhocko@suse.cz>

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/7] export memory cgroup's swappines by mem_cgroup_swappiness()

[Ying Han]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

On Wed, Jun 22, 2011 at 8:22 AM, Michal Hocko <mhocko@suse.cz> wrote:
>
> On Thu 16-06-11 12:52:22, KAMEZAWA Hiroyuki wrote:
> > From 6f9c40172947fb92ab0ea6f7d73d577473879636 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Wed, 15 Jun 2011 12:06:31 +0900
> > Subject: [PATCH 2/7] export memory cgroup's swappines by
mem_cgroup_swappiness()
> >
> > Each memory cgroup has 'swappiness' value and it can be accessed by
> > get_swappiness(memcg). The major user is try_to_free_mem_cgroup_pages()
> > and swappiness is passed by argument.
> >
> > It's now static function but some planned updates will need to
> > get swappiness from files other than memcontrol.c
> > This patch exports get_swappiness() as mem_cgroup_swappiness().
> > By this, we can remove the argument of swapiness from try_to_fre...
> >
> > I think this makes sense because passed swapiness is always from memory
> > cgroup passed as an argument and this duplication of argument is
> > not very good.
>
> Yes makes sense and it makes it more looking like a global reclaim.
>
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> Reviewed-by: Michal Hocko <mhocko@suse.cz>
>
 Acked-by: Ying Han <yinghan@google.com>

--Ying
>
> --
> Michal Hocko
> SUSE Labs
> SUSE LINUX s.r.o.
> Lihovarska 1060/12
> 190 00 Praha 9
> Czech Republic

[-- Attachment #2: Type: text/html, Size: 1957 bytes --]

[PATCH 3/7] memcg: add memory.scan_stat

[KAMEZAWA Hiroyuki]

Re: [PATCH 3/7] memcg: add memory.scan_stat

[Ying Han]

On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Wed, 15 Jun 2011 14:11:01 +0900
> Subject: [PATCH 3/7] memcg: add memory.scan_stat
>
> commit log of commit 0ae5e89 " memcg: count the soft_limit reclaim in..."
> says it adds scanning stats to memory.stat file. But it doesn't because
> we considered we needed to make a concensus for such new APIs.
>
> This patch is a trial to add memory.scan_stat. This shows
>  - the number of scanned pages
>  - the number of recleimed pages
>  - the number of elaplsed time (including sleep/pause time)
>  for both of direct/soft reclaim and shrinking caused by changing limit
>  or force_empty.
>
> The biggest difference with oringinal Ying's one is that this file
> can be reset by some write, as
>
>  # echo 0 ...../memory.scan_stat
>
> [kamezawa@bluextal ~]$ cat /cgroup/memory/A/memory.scan_stat
> scanned_pages_by_limit 358470
> freed_pages_by_limit 180795
> elapsed_ns_by_limit 21629927
> scanned_pages_by_system 0
> freed_pages_by_system 0
> elapsed_ns_by_system 0
> scanned_pages_by_shrink 76646
> freed_pages_by_shrink 38355
> elappsed_ns_by_shrink 31990670
> total_scanned_pages_by_limit 358470
> total_freed_pages_by_limit 180795
> total_elapsed_ns_by_hierarchical 216299275
> total_scanned_pages_by_system 0
> total_freed_pages_by_system 0
> total_elapsed_ns_by_system 0
> total_scanned_pages_by_shrink 76646
> total_freed_pages_by_shrink 38355
> total_elapsed_ns_by_shrink 31990670
>
> total_xxxx is for hierarchy management.
>
> This will be useful for further memcg developments and need to be
> developped before we do some complicated rework on LRU/softlimit
> management.

Agreed. Actually we are also looking into adding a per-memcg API for
adding visibility of
page reclaim path. It would be helpful for us to settle w/ the API first.

I am not a fan of names, but how about
"/dev/cgroup/memory/memory.reclaim_stat" ?

>
> Now, scan/free/elapsed_by_system is incomplete but future works of
> Johannes at el. will fill remaining information and then, we can
> look into problems of isolation between memcgs.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  Documentation/cgroups/memory.txt |   33 +++++++++
>  include/linux/memcontrol.h       |   16 ++++
>  include/linux/swap.h             |    6 -
>  mm/memcontrol.c                  |  135 +++++++++++++++++++++++++++++++++++++--
>  mm/vmscan.c                      |   27 ++++++-
>  5 files changed, 199 insertions(+), 18 deletions(-)
>
> Index: mmotm-0615/Documentation/cgroups/memory.txt
> ===================================================================
> --- mmotm-0615.orig/Documentation/cgroups/memory.txt
> +++ mmotm-0615/Documentation/cgroups/memory.txt
> @@ -380,7 +380,7 @@ will be charged as a new owner of it.
>
>  5.2 stat file
>
> -memory.stat file includes following statistics
> +5.2.1 memory.stat file includes following statistics
>
>  # per-memory cgroup local status
>  cache          - # of bytes of page cache memory.
> @@ -438,6 +438,37 @@ Note:
>         file_mapped is accounted only when the memory cgroup is owner of page
>         cache.)
>
> +5.2.2 memory.scan_stat
> +
> +memory.scan_stat includes statistics information for memory scanning and
> +freeing, reclaiming. The statistics shows memory scanning information since
> +memory cgroup creation and can be reset to 0 by writing 0 as
> +
> + #echo 0 > ../memory.scan_stat
> +
> +This file contains following statistics.
> +
> +scanned_pages_by_limit - # of scanned pages at hitting limit.
> +freed_pages_by_limit   - # of freed pages at hitting limit.

How those stats different from Johannes's patch? I feel we should keep
them into this API instead of memory.stat
"pgscan_direct_limit"
"pgreclaim_direct_limit"

> +elapsed_ns_by_limit    - nano sec of elappsed time at LRU scan at
> +                                  hitting limit.(this includes sleep time.)


> +
> +scanned_pages_by_system        - # of scanned pages by the kernel.
> +                         (Now, this value means global memory reclaim
> +                           caused by system memory shortage with a hint
> +                          of softlimit. "No soft limit" case will be
> +                          supported in future.)
> +freed_pages_by_system  - # of freed pages by the kernel.

The same for the following which I assume the same meaning with:
"pgscan_direct_hierarchy"
"pgreclaim_direct_hierarchy"

> +elapsed_ns_by_system   - nano sec of elappsed time by kernel.
> +
> +scanned_pages_by_shrink        - # of scanned pages by shrinking.
> +                                 (i.e. changes of limit, force_empty, etc.)
> +freed_pages_by_shrink  - # of freed pages by shirkining.

So those stats are not included in the ones above?

--Ying

> +elappsed_ns_by_shrink  - nano sec of elappsed time at shrinking.
> +
> +total_xxx includes the statistics of children scanning caused by the cgroup.
> +
> +
>  5.3 swappiness
>
>  Similar to /proc/sys/vm/swappiness, but affecting a hierarchy of groups only.
> Index: mmotm-0615/include/linux/memcontrol.h
> ===================================================================
> --- mmotm-0615.orig/include/linux/memcontrol.h
> +++ mmotm-0615/include/linux/memcontrol.h
> @@ -120,6 +120,22 @@ struct zone_reclaim_stat*
>  mem_cgroup_get_reclaim_stat_from_page(struct page *page);
>  extern void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
>                                        struct task_struct *p);
> +struct memcg_scanrecord {
> +       struct mem_cgroup *mem; /* scanend memory cgroup */
> +       struct mem_cgroup *root; /* scan target hierarchy root */
> +       int context;            /* scanning context (see memcontrol.c) */
> +       unsigned long nr_scanned; /* the number of scanned pages */
> +       unsigned long nr_freed; /* the number of freed pages */
> +       unsigned long elappsed; /* nsec of time elapsed while scanning */
> +};
> +
> +extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
> +                                                 gfp_t gfp_mask, bool noswap,
> +                                                 struct memcg_scanrecord *rec);
> +extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> +                                               gfp_t gfp_mask, bool noswap,
> +                                               struct zone *zone,
> +                                               struct memcg_scanrecord *rec);
>
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>  extern int do_swap_account;
> Index: mmotm-0615/include/linux/swap.h
> ===================================================================
> --- mmotm-0615.orig/include/linux/swap.h
> +++ mmotm-0615/include/linux/swap.h
> @@ -253,12 +253,6 @@ static inline void lru_cache_add_file(st
>  /* linux/mm/vmscan.c */
>  extern unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
>                                        gfp_t gfp_mask, nodemask_t *mask);
> -extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
> -                                                 gfp_t gfp_mask, bool noswap);
> -extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> -                                               gfp_t gfp_mask, bool noswap,
> -                                               struct zone *zone,
> -                                               unsigned long *nr_scanned);
>  extern int __isolate_lru_page(struct page *page, int mode, int file);
>  extern unsigned long shrink_all_memory(unsigned long nr_pages);
>  extern int vm_swappiness;
> Index: mmotm-0615/mm/memcontrol.c
> ===================================================================
> --- mmotm-0615.orig/mm/memcontrol.c
> +++ mmotm-0615/mm/memcontrol.c
> @@ -203,6 +203,57 @@ struct mem_cgroup_eventfd_list {
>  static void mem_cgroup_threshold(struct mem_cgroup *mem);
>  static void mem_cgroup_oom_notify(struct mem_cgroup *mem);
>
> +enum {
> +       SCAN_BY_LIMIT,
> +       FREED_BY_LIMIT,
> +       ELAPSED_BY_LIMIT,
> +
> +       SCAN_BY_SYSTEM,
> +       FREED_BY_SYSTEM,
> +       ELAPSED_BY_SYSTEM,
> +
> +       SCAN_BY_SHRINK,
> +       FREED_BY_SHRINK,
> +       ELAPSED_BY_SHRINK,
> +       NR_SCANSTATS,
> +};
> +#define __FREED                (1)
> +#define        __ELAPSED       (2)
> +
> +struct scanstat {
> +       spinlock_t      lock;
> +       unsigned long   stats[NR_SCANSTATS];    /* local statistics */
> +       unsigned long   totalstats[NR_SCANSTATS];   /* hierarchical */
> +};
> +
> +const char *scanstat_string[NR_SCANSTATS] = {
> +       "scanned_pages_by_limit",
> +       "freed_pages_by_limit",
> +       "elapsed_ns_by_limit",
> +
> +       "scanned_pages_by_system",
> +       "freed_pages_by_system",
> +       "elapsed_ns_by_system",
> +
> +       "scanned_pages_by_shrink",
> +       "freed_pages_by_shrink",
> +       "elappsed_ns_by_shrink",
> +};
> +
> +const char *total_scanstat_string[NR_SCANSTATS] = {
> +       "total_scanned_pages_by_limit",
> +       "total_freed_pages_by_limit",
> +       "total_elapsed_ns_by_hierarchical",
> +
> +       "total_scanned_pages_by_system",
> +       "total_freed_pages_by_system",
> +       "total_elapsed_ns_by_system",
> +
> +       "total_scanned_pages_by_shrink",
> +       "total_freed_pages_by_shrink",
> +       "total_elapsed_ns_by_shrink",
> +};
> +
>  /*
>  * The memory controller data structure. The memory controller controls both
>  * page cache and RSS per cgroup. We would eventually like to provide
> @@ -264,7 +315,8 @@ struct mem_cgroup {
>
>        /* For oom notifier event fd */
>        struct list_head oom_notify;
> -
> +       /* For recording LRU-scan statistics */
> +       struct scanstat scanstat;
>        /*
>         * Should we move charges of a task when a task is moved into this
>         * mem_cgroup ? And what type of charges should we move ?
> @@ -1634,6 +1686,28 @@ int mem_cgroup_select_victim_node(struct
>  }
>  #endif
>
> +
> +
> +static void mem_cgroup_record_scanstat(struct memcg_scanrecord *rec)
> +{
> +       struct mem_cgroup *mem;
> +       int context = rec->context;
> +
> +       mem = rec->mem;
> +       spin_lock(&mem->scanstat.lock);
> +       mem->scanstat.stats[context] += rec->nr_scanned;
> +       mem->scanstat.stats[context + __FREED] += rec->nr_freed;
> +       mem->scanstat.stats[context + __ELAPSED] += rec->elappsed;
> +       spin_unlock(&mem->scanstat.lock);
> +
> +       mem = rec->root;
> +       spin_lock(&mem->scanstat.lock);
> +       mem->scanstat.totalstats[context] += rec->nr_scanned;
> +       mem->scanstat.totalstats[context + __FREED] += rec->nr_freed;
> +       mem->scanstat.totalstats[context + __ELAPSED] += rec->elappsed;
> +       spin_unlock(&mem->scanstat.lock);
> +}
> +
>  /*
>  * Scan the hierarchy if needed to reclaim memory. We remember the last child
>  * we reclaimed from, so that we don't end up penalizing one child extensively
> @@ -1659,8 +1733,8 @@ static int mem_cgroup_hierarchical_recla
>        bool shrink = reclaim_options & MEM_CGROUP_RECLAIM_SHRINK;
>        bool check_soft = reclaim_options & MEM_CGROUP_RECLAIM_SOFT;
>        unsigned long excess;
> -       unsigned long nr_scanned;
>        int visit;
> +       struct memcg_scanrecord rec;
>
>        excess = res_counter_soft_limit_excess(&root_mem->res) >> PAGE_SHIFT;
>
> @@ -1668,6 +1742,15 @@ static int mem_cgroup_hierarchical_recla
>        if (!check_soft && root_mem->memsw_is_minimum)
>                noswap = true;
>
> +       if (shrink)
> +               rec.context = SCAN_BY_SHRINK;
> +       else if (check_soft)
> +               rec.context = SCAN_BY_SYSTEM;
> +       else
> +               rec.context = SCAN_BY_LIMIT;
> +
> +       rec.root = root_mem;
> +
>  again:
>        if (!shrink) {
>                visit = 0;
> @@ -1695,14 +1778,19 @@ again:
>                        css_put(&victim->css);
>                        continue;
>                }
> +               rec.mem = victim;
> +               rec.nr_scanned = 0;
> +               rec.nr_freed = 0;
> +               rec.elappsed = 0;
>                /* we use swappiness of local cgroup */
>                if (check_soft) {
>                        ret = mem_cgroup_shrink_node_zone(victim, gfp_mask,
> -                               noswap, zone, &nr_scanned);
> -                       *total_scanned += nr_scanned;
> +                               noswap, zone, &rec);
> +                       *total_scanned += rec.nr_scanned;
>                } else
>                        ret = try_to_free_mem_cgroup_pages(victim, gfp_mask,
> -                                               noswap);
> +                                               noswap, &rec);
> +               mem_cgroup_record_scanstat(&rec);
>                css_put(&victim->css);
>
>                total += ret;
> @@ -3757,7 +3845,8 @@ try_to_free:
>                        ret = -EINTR;
>                        goto out;
>                }
> -               progress = try_to_free_mem_cgroup_pages(mem, GFP_KERNEL, false);
> +               progress = try_to_free_mem_cgroup_pages(mem,
> +                               GFP_KERNEL, false, NULL);
>                if (!progress) {
>                        nr_retries--;
>                        /* maybe some writeback is necessary */
> @@ -4599,6 +4688,34 @@ static int mem_control_numa_stat_open(st
>  }
>  #endif /* CONFIG_NUMA */
>
> +static int mem_cgroup_scan_stat_read(struct cgroup *cgrp,
> +                               struct cftype *cft,
> +                               struct cgroup_map_cb *cb)
> +{
> +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
> +       int i;
> +
> +       for (i = 0; i < NR_SCANSTATS; i++)
> +               cb->fill(cb, scanstat_string[i], mem->scanstat.stats[i]);
> +       for (i = 0; i < NR_SCANSTATS; i++)
> +               cb->fill(cb, total_scanstat_string[i],
> +                       mem->scanstat.totalstats[i]);
> +       return 0;
> +}
> +
> +static int mem_cgroup_reset_scan_stat(struct cgroup *cgrp,
> +                               unsigned int event)
> +{
> +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
> +
> +       spin_lock(&mem->scanstat.lock);
> +       memset(&mem->scanstat.stats, 0, sizeof(mem->scanstat.stats));
> +       memset(&mem->scanstat.totalstats, 0, sizeof(mem->scanstat.totalstats));
> +       spin_unlock(&mem->scanstat.lock);
> +       return 0;
> +}
> +
> +
>  static struct cftype mem_cgroup_files[] = {
>        {
>                .name = "usage_in_bytes",
> @@ -4669,6 +4786,11 @@ static struct cftype mem_cgroup_files[]
>                .mode = S_IRUGO,
>        },
>  #endif
> +       {
> +               .name = "scan_stat",
> +               .read_map = mem_cgroup_scan_stat_read,
> +               .trigger = mem_cgroup_reset_scan_stat,
> +       },
>  };
>
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> @@ -4932,6 +5054,7 @@ mem_cgroup_create(struct cgroup_subsys *
>        atomic_set(&mem->refcnt, 1);
>        mem->move_charge_at_immigrate = 0;
>        mutex_init(&mem->thresholds_lock);
> +       spin_lock_init(&mem->scanstat.lock);
>        return &mem->css;
>  free_out:
>        __mem_cgroup_free(mem);
> Index: mmotm-0615/mm/vmscan.c
> ===================================================================
> --- mmotm-0615.orig/mm/vmscan.c
> +++ mmotm-0615/mm/vmscan.c
> @@ -2199,9 +2199,9 @@ unsigned long try_to_free_pages(struct z
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR
>
>  unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> -                                               gfp_t gfp_mask, bool noswap,
> -                                               struct zone *zone,
> -                                               unsigned long *nr_scanned)
> +                                       gfp_t gfp_mask, bool noswap,
> +                                       struct zone *zone,
> +                                       struct memcg_scanrecord *rec)
>  {
>        struct scan_control sc = {
>                .nr_scanned = 0,
> @@ -2213,6 +2213,7 @@ unsigned long mem_cgroup_shrink_node_zon
>                .order = 0,
>                .mem_cgroup = mem,
>        };
> +       unsigned long start, end;
>
>        sc.gfp_mask = (gfp_mask & GFP_RECLAIM_MASK) |
>                        (GFP_HIGHUSER_MOVABLE & ~GFP_RECLAIM_MASK);
> @@ -2221,6 +2222,7 @@ unsigned long mem_cgroup_shrink_node_zon
>                                                      sc.may_writepage,
>                                                      sc.gfp_mask);
>
> +       start = sched_clock();
>        /*
>         * NOTE: Although we can get the priority field, using it
>         * here is not a good idea, since it limits the pages we can scan.
> @@ -2229,19 +2231,27 @@ unsigned long mem_cgroup_shrink_node_zon
>         * the priority and make it zero.
>         */
>        shrink_zone(0, zone, &sc);
> +       end = sched_clock();
> +
> +       if (rec) {
> +               rec->nr_scanned += sc.nr_scanned;
> +               rec->nr_freed += sc.nr_reclaimed;
> +               rec->elappsed += end - start;
> +       }
>
>        trace_mm_vmscan_memcg_softlimit_reclaim_end(sc.nr_reclaimed);
>
> -       *nr_scanned = sc.nr_scanned;
>        return sc.nr_reclaimed;
>  }
>
>  unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem_cont,
>                                           gfp_t gfp_mask,
> -                                          bool noswap)
> +                                          bool noswap,
> +                                          struct memcg_scanrecord *rec)
>  {
>        struct zonelist *zonelist;
>        unsigned long nr_reclaimed;
> +       unsigned long start, end;
>        int nid;
>        struct scan_control sc = {
>                .may_writepage = !laptop_mode,
> @@ -2259,6 +2269,7 @@ unsigned long try_to_free_mem_cgroup_pag
>                .gfp_mask = sc.gfp_mask,
>        };
>
> +       start = sched_clock();
>        /*
>         * Unlike direct reclaim via alloc_pages(), memcg's reclaim doesn't
>         * take care of from where we get pages. So the node where we start the
> @@ -2273,6 +2284,12 @@ unsigned long try_to_free_mem_cgroup_pag
>                                            sc.gfp_mask);
>
>        nr_reclaimed = do_try_to_free_pages(zonelist, &sc, &shrink);
> +       end = sched_clock();
> +       if (rec) {
> +               rec->nr_scanned += sc.nr_scanned;
> +               rec->nr_freed += sc.nr_reclaimed;
> +               rec->elappsed += end - start;
> +       }
>
>        trace_mm_vmscan_memcg_reclaim_end(nr_reclaimed);
>
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/7] memcg: add memory.scan_stat

[KAMEZAWA Hiroyuki]

On Fri, 17 Jun 2011 15:04:18 -0700
Ying Han <yinghan@google.com> wrote:

> On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Wed, 15 Jun 2011 14:11:01 +0900
> > Subject: [PATCH 3/7] memcg: add memory.scan_stat
> >
> > commit log of commit 0ae5e89 " memcg: count the soft_limit reclaim in..."
> > says it adds scanning stats to memory.stat file. But it doesn't because
> > we considered we needed to make a concensus for such new APIs.
> >
> > This patch is a trial to add memory.scan_stat. This shows
> > A - the number of scanned pages
> > A - the number of recleimed pages
> > A - the number of elaplsed time (including sleep/pause time)
> > A for both of direct/soft reclaim and shrinking caused by changing limit
> > A or force_empty.
> >
> > The biggest difference with oringinal Ying's one is that this file
> > can be reset by some write, as
> >
> > A # echo 0 ...../memory.scan_stat
> >
> > [kamezawa@bluextal ~]$ cat /cgroup/memory/A/memory.scan_stat
> > scanned_pages_by_limit 358470
> > freed_pages_by_limit 180795
> > elapsed_ns_by_limit 21629927
> > scanned_pages_by_system 0
> > freed_pages_by_system 0
> > elapsed_ns_by_system 0
> > scanned_pages_by_shrink 76646
> > freed_pages_by_shrink 38355
> > elappsed_ns_by_shrink 31990670
> > total_scanned_pages_by_limit 358470
> > total_freed_pages_by_limit 180795
> > total_elapsed_ns_by_hierarchical 216299275
> > total_scanned_pages_by_system 0
> > total_freed_pages_by_system 0
> > total_elapsed_ns_by_system 0
> > total_scanned_pages_by_shrink 76646
> > total_freed_pages_by_shrink 38355
> > total_elapsed_ns_by_shrink 31990670
> >
> > total_xxxx is for hierarchy management.
> >
> > This will be useful for further memcg developments and need to be
> > developped before we do some complicated rework on LRU/softlimit
> > management.
> 
> Agreed. Actually we are also looking into adding a per-memcg API for
> adding visibility of
> page reclaim path. It would be helpful for us to settle w/ the API first.
> 
> I am not a fan of names, but how about
> "/dev/cgroup/memory/memory.reclaim_stat" ?
> 

Hm, ok, I have no favorite. 


> >
> > Now, scan/free/elapsed_by_system is incomplete but future works of
> > Johannes at el. will fill remaining information and then, we can
> > look into problems of isolation between memcgs.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> > A Documentation/cgroups/memory.txt | A  33 +++++++++
> > A include/linux/memcontrol.h A  A  A  | A  16 ++++
> > A include/linux/swap.h A  A  A  A  A  A  | A  A 6 -
> > A mm/memcontrol.c A  A  A  A  A  A  A  A  A | A 135 +++++++++++++++++++++++++++++++++++++--
> > A mm/vmscan.c A  A  A  A  A  A  A  A  A  A  A | A  27 ++++++-
> > A 5 files changed, 199 insertions(+), 18 deletions(-)
> >
> > Index: mmotm-0615/Documentation/cgroups/memory.txt
> > ===================================================================
> > --- mmotm-0615.orig/Documentation/cgroups/memory.txt
> > +++ mmotm-0615/Documentation/cgroups/memory.txt
> > @@ -380,7 +380,7 @@ will be charged as a new owner of it.
> >
> > A 5.2 stat file
> >
> > -memory.stat file includes following statistics
> > +5.2.1 memory.stat file includes following statistics
> >
> > A # per-memory cgroup local status
> > A cache A  A  A  A  A - # of bytes of page cache memory.
> > @@ -438,6 +438,37 @@ Note:
> > A  A  A  A  file_mapped is accounted only when the memory cgroup is owner of page
> > A  A  A  A  cache.)
> >
> > +5.2.2 memory.scan_stat
> > +
> > +memory.scan_stat includes statistics information for memory scanning and
> > +freeing, reclaiming. The statistics shows memory scanning information since
> > +memory cgroup creation and can be reset to 0 by writing 0 as
> > +
> > + #echo 0 > ../memory.scan_stat
> > +
> > +This file contains following statistics.
> > +
> > +scanned_pages_by_limit - # of scanned pages at hitting limit.
> > +freed_pages_by_limit A  - # of freed pages at hitting limit.
> 
> How those stats different from Johannes's patch? I feel we should keep
> them into this API instead of memory.stat
> "pgscan_direct_limit"
> "pgreclaim_direct_limit"
> 

It's unclear the unit of number from that name.
And, I can't find what it means "direct_limit". Only "limit" is meaningful.



> > +elapsed_ns_by_limit A  A - nano sec of elappsed time at LRU scan at
> > + A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A hitting limit.(this includes sleep time.)
> 
> 
> > +
> > +scanned_pages_by_system A  A  A  A - # of scanned pages by the kernel.
> > + A  A  A  A  A  A  A  A  A  A  A  A  (Now, this value means global memory reclaim
> > + A  A  A  A  A  A  A  A  A  A  A  A  A  caused by system memory shortage with a hint
> > + A  A  A  A  A  A  A  A  A  A  A  A  A of softlimit. "No soft limit" case will be
> > + A  A  A  A  A  A  A  A  A  A  A  A  A supported in future.)
> > +freed_pages_by_system A - # of freed pages by the kernel.
> 
> The same for the following which I assume the same meaning with:
> "pgscan_direct_hierarchy"
> "pgreclaim_direct_hierarchy"
> 

Doesn't make sense. What hierarchy means, here?

Above 2 is for showing "amount of scanned/reclaimed memory by system's memory
pressure". 
(But now, it just shows "softlimit" information until Johannes' work comes.)

For hierarchy information, I have "total_xxxx_by_xxxx" parameter. 

> > +elapsed_ns_by_system A  - nano sec of elappsed time by kernel.
> > +
> > +scanned_pages_by_shrink A  A  A  A - # of scanned pages by shrinking.
> > + A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  (i.e. changes of limit, force_empty, etc.)
> > +freed_pages_by_shrink A - # of freed pages by shirkining.
> 
> So those stats are not included in the ones above?
> 

Yes.

Thanks,
-Kame



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/7] memcg: add memory.scan_stat

[KAMEZAWA Hiroyuki]

On Mon, 20 Jun 2011 08:41:23 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Fri, 17 Jun 2011 15:04:18 -0700
> Ying Han <yinghan@google.com> wrote:
> 
> > On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki
> > <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
> > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > > Date: Wed, 15 Jun 2011 14:11:01 +0900
> > > Subject: [PATCH 3/7] memcg: add memory.scan_stat
> > >
> > > commit log of commit 0ae5e89 " memcg: count the soft_limit reclaim in..."
> > > says it adds scanning stats to memory.stat file. But it doesn't because
> > > we considered we needed to make a concensus for such new APIs.
> > >
> > > This patch is a trial to add memory.scan_stat. This shows
> > > A - the number of scanned pages
> > > A - the number of recleimed pages
> > > A - the number of elaplsed time (including sleep/pause time)
> > > A for both of direct/soft reclaim and shrinking caused by changing limit
> > > A or force_empty.
> > >
> > > The biggest difference with oringinal Ying's one is that this file
> > > can be reset by some write, as
> > >
> > > A # echo 0 ...../memory.scan_stat
> > >
> > > [kamezawa@bluextal ~]$ cat /cgroup/memory/A/memory.scan_stat
> > > scanned_pages_by_limit 358470
> > > freed_pages_by_limit 180795
> > > elapsed_ns_by_limit 21629927
> > > scanned_pages_by_system 0
> > > freed_pages_by_system 0
> > > elapsed_ns_by_system 0
> > > scanned_pages_by_shrink 76646
> > > freed_pages_by_shrink 38355
> > > elappsed_ns_by_shrink 31990670
> > > total_scanned_pages_by_limit 358470
> > > total_freed_pages_by_limit 180795
> > > total_elapsed_ns_by_hierarchical 216299275
> > > total_scanned_pages_by_system 0
> > > total_freed_pages_by_system 0
> > > total_elapsed_ns_by_system 0
> > > total_scanned_pages_by_shrink 76646
> > > total_freed_pages_by_shrink 38355
> > > total_elapsed_ns_by_shrink 31990670
> > >
> > > total_xxxx is for hierarchy management.
> > >
> > > This will be useful for further memcg developments and need to be
> > > developped before we do some complicated rework on LRU/softlimit
> > > management.
> > 
> > Agreed. Actually we are also looking into adding a per-memcg API for
> > adding visibility of
> > page reclaim path. It would be helpful for us to settle w/ the API first.
> > 
> > I am not a fan of names, but how about
> > "/dev/cgroup/memory/memory.reclaim_stat" ?
> > 
> 
> Hm, ok, I have no favorite. 
> 
> 

If I rename, I'll just rename file name as "reclaim_stat" but doesn't
rename internal structures because there is already "struct reclaim_stat".

Hm, to be honest, I don't like the name "reclaim_stat".
(Because in most case, the pages are not freed for reclaim, but for
 hitting limit.)

memory.vmscan_info ?

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/7] memcg: add memory.scan_stat

[Ying Han]

On Sunday, June 19, 2011, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Mon, 20 Jun 2011 08:41:23 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
>> On Fri, 17 Jun 2011 15:04:18 -0700
>> Ying Han <yinghan@google.com> wrote:
>>
>> > On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki
>> > <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> > > From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
>> > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> > > Date: Wed, 15 Jun 2011 14:11:01 +0900
>> > > Subject: [PATCH 3/7] memcg: add memory.scan_stat
>> > >
>> > > commit log of commit 0ae5e89 " memcg: count the soft_limit reclaim in..."
>> > > says it adds scanning stats to memory.stat file. But it doesn't because
>> > > we considered we needed to make a concensus for such new APIs.
>> > >
>> > > This patch is a trial to add memory.scan_stat. This shows
>> > >  - the number of scanned pages
>> > >  - the number of recleimed pages
>> > >  - the number of elaplsed time (including sleep/pause time)
>> > >  for both of direct/soft reclaim and shrinking caused by changing limit
>> > >  or force_empty.
>> > >
>> > > The biggest difference with oringinal Ying's one is that this file
>> > > can be reset by some write, as
>> > >
>> > >  # echo 0 ...../memory.scan_stat
>> > >
>> > > [kamezawa@bluextal ~]$ cat /cgroup/memory/A/memory.scan_stat
>> > > scanned_pages_by_limit 358470
>> > > freed_pages_by_limit 180795
>> > > elapsed_ns_by_limit 21629927
>> > > scanned_pages_by_system 0
>> > > freed_pages_by_system 0
>> > > elapsed_ns_by_system 0
>> > > scanned_pages_by_shrink 76646
>> > > freed_pages_by_shrink 38355
>> > > elappsed_ns_by_shrink 31990670
>> > > total_scanned_pages_by_limit 358470
>> > > total_freed_pages_by_limit 180795
>> > > total_elapsed_ns_by_hierarchical 216299275
>> > > total_scanned_pages_by_system 0
>> > > total_freed_pages_by_system 0
>> > > total_elapsed_ns_by_system 0
>> > > total_scanned_pages_by_shrink 76646
>> > > total_freed_pages_by_shrink 38355
>> > > total_elapsed_ns_by_shrink 31990670
>> > >
>> > > total_xxxx is for hierarchy management.
>> > >
>> > > This will be useful for further memcg developments and need to be
>> > > developped before we do some complicated rework on LRU/softlimit
>> > > management.
>> >
>> > Agreed. Actually we are also looking into adding a per-memcg API for
>> > adding visibility of
>> > page reclaim path. It would be helpful for us to settle w/ the API first.
>> >
>> > I am not a fan of names, but how about
>> > "/dev/cgroup/memory/memory.reclaim_stat" ?
>> >
>>
>> Hm, ok, I have no favorite.
>>
>>
>
> If I rename, I'll just rename file name as "reclaim_stat" but doesn't
> rename internal structures because there is already "struct reclaim_stat".
>
> Hm, to be honest, I don't like the name "reclaim_stat".
> (Because in most case, the pages are not freed for reclaim, but for
>  hitting limit.)
>
> memory.vmscan_info ?

No objection on the name. I will look into the other part of the patch

Thanks

--ying
>
> Thanks,
> -Kame
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/7] memcg: add memory.scan_stat

[Ying Han]

[-- Attachment #1: Type: text/plain, Size: 19256 bytes --]

On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki <
kamezawa.hiroyu@jp.fujitsu.com> wrote:

> From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Wed, 15 Jun 2011 14:11:01 +0900
> Subject: [PATCH 3/7] memcg: add memory.scan_stat
>
> commit log of commit 0ae5e89 " memcg: count the soft_limit reclaim in..."
> says it adds scanning stats to memory.stat file. But it doesn't because
> we considered we needed to make a concensus for such new APIs.
>
> This patch is a trial to add memory.scan_stat. This shows
>  - the number of scanned pages
>  - the number of recleimed pages
>  - the number of elaplsed time (including sleep/pause time)
>  for both of direct/soft reclaim and shrinking caused by changing limit
>  or force_empty.
>
> The biggest difference with oringinal Ying's one is that this file
> can be reset by some write, as
>
>  # echo 0 ...../memory.scan_stat
>
> [kamezawa@bluextal ~]$ cat /cgroup/memory/A/memory.scan_stat
> scanned_pages_by_limit 358470
> freed_pages_by_limit 180795
> elapsed_ns_by_limit 21629927
> scanned_pages_by_system 0
> freed_pages_by_system 0
> elapsed_ns_by_system 0
> scanned_pages_by_shrink 76646
> freed_pages_by_shrink 38355
> elappsed_ns_by_shrink 31990670
>

elapsed?


> total_scanned_pages_by_limit 358470
> total_freed_pages_by_limit 180795
> total_elapsed_ns_by_hierarchical 216299275
> total_scanned_pages_by_system 0
> total_freed_pages_by_system 0
> total_elapsed_ns_by_system 0
> total_scanned_pages_by_shrink 76646
> total_freed_pages_by_shrink 38355
> total_elapsed_ns_by_shrink 31990670
>
> total_xxxx is for hierarchy management.
>

For some reason, i feel the opposite where the local stat (like
"scanned_pages_by_limit") are reclaimed under hierarchical reclaim. The
total_xxx stats are only incremented for root_mem which is the cgroup
triggers the hierarchical reclaim. So:

total_scanned_pages_by_limit: number of pages being scanned while the memcg
hits its limit
scanned_pages_by_limit: number of pages being scanned while one of the
memcg's ancestor hits its limit

am i missing something?


>
> This will be useful for further memcg developments and need to be
> developped before we do some complicated rework on LRU/softlimit
> management.
>
> Now, scan/free/elapsed_by_system is incomplete but future works of
> Johannes at el. will fill remaining information and then, we can
> look into problems of isolation between memcgs.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  Documentation/cgroups/memory.txt |   33 +++++++++
>  include/linux/memcontrol.h       |   16 ++++
>  include/linux/swap.h             |    6 -
>  mm/memcontrol.c                  |  135
> +++++++++++++++++++++++++++++++++++++--
>  mm/vmscan.c                      |   27 ++++++-
>  5 files changed, 199 insertions(+), 18 deletions(-)
>
> Index: mmotm-0615/Documentation/cgroups/memory.txt
> ===================================================================
> --- mmotm-0615.orig/Documentation/cgroups/memory.txt
> +++ mmotm-0615/Documentation/cgroups/memory.txt
> @@ -380,7 +380,7 @@ will be charged as a new owner of it.
>
>  5.2 stat file
>
> -memory.stat file includes following statistics
> +5.2.1 memory.stat file includes following statistics
>
>  # per-memory cgroup local status
>  cache          - # of bytes of page cache memory.
> @@ -438,6 +438,37 @@ Note:
>         file_mapped is accounted only when the memory cgroup is owner of
> page
>         cache.)
>
> +5.2.2 memory.scan_stat
> +
> +memory.scan_stat includes statistics information for memory scanning and
> +freeing, reclaiming. The statistics shows memory scanning information
> since
> +memory cgroup creation and can be reset to 0 by writing 0 as
> +
> + #echo 0 > ../memory.scan_stat
> +
> +This file contains following statistics.
> +
> +scanned_pages_by_limit - # of scanned pages at hitting limit.
> +freed_pages_by_limit   - # of freed pages at hitting limit.
> +elapsed_ns_by_limit    - nano sec of elappsed time at LRU scan at
> +                                  hitting limit.(this includes sleep
> time.)
>
elapsed?

> +
> +scanned_pages_by_system        - # of scanned pages by the kernel.
> +                         (Now, this value means global memory reclaim
> +                           caused by system memory shortage with a hint
> +                          of softlimit. "No soft limit" case will be
> +                          supported in future.)
> +freed_pages_by_system  - # of freed pages by the kernel.
> +elapsed_ns_by_system   - nano sec of elappsed time by kernel.
> +
> +scanned_pages_by_shrink        - # of scanned pages by shrinking.
> +                                 (i.e. changes of limit, force_empty,
> etc.)
> +freed_pages_by_shrink  - # of freed pages by shirkining.
> +elappsed_ns_by_shrink  - nano sec of elappsed time at shrinking.
>
elapsed?

> +
> +total_xxx includes the statistics of children scanning caused by the
> cgroup.
>

based on the code inspection, the total_xxx also includes the cgroup's scan
stat as well.

+
> +
>  5.3 swappiness
>
>  Similar to /proc/sys/vm/swappiness, but affecting a hierarchy of groups
> only.
> Index: mmotm-0615/include/linux/memcontrol.h
> ===================================================================
> --- mmotm-0615.orig/include/linux/memcontrol.h
> +++ mmotm-0615/include/linux/memcontrol.h
> @@ -120,6 +120,22 @@ struct zone_reclaim_stat*
>  mem_cgroup_get_reclaim_stat_from_page(struct page *page);
>  extern void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
>                                        struct task_struct *p);
> +struct memcg_scanrecord {
> +       struct mem_cgroup *mem; /* scanend memory cgroup */
> +       struct mem_cgroup *root; /* scan target hierarchy root */
> +       int context;            /* scanning context (see memcontrol.c) */
> +       unsigned long nr_scanned; /* the number of scanned pages */
> +       unsigned long nr_freed; /* the number of freed pages */
> +       unsigned long elappsed; /* nsec of time elapsed while scanning */
>
elapsed?

> +};
> +
> +extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
> +                                                 gfp_t gfp_mask, bool
> noswap,
> +                                                 struct memcg_scanrecord
> *rec);
> +extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> +                                               gfp_t gfp_mask, bool
> noswap,
> +                                               struct zone *zone,
> +                                               struct memcg_scanrecord
> *rec);
>
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>  extern int do_swap_account;
> Index: mmotm-0615/include/linux/swap.h
> ===================================================================
> --- mmotm-0615.orig/include/linux/swap.h
> +++ mmotm-0615/include/linux/swap.h
> @@ -253,12 +253,6 @@ static inline void lru_cache_add_file(st
>  /* linux/mm/vmscan.c */
>  extern unsigned long try_to_free_pages(struct zonelist *zonelist, int
> order,
>                                        gfp_t gfp_mask, nodemask_t *mask);
> -extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
> -                                                 gfp_t gfp_mask, bool
> noswap);
> -extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> -                                               gfp_t gfp_mask, bool
> noswap,
> -                                               struct zone *zone,
> -                                               unsigned long *nr_scanned);
>  extern int __isolate_lru_page(struct page *page, int mode, int file);
>  extern unsigned long shrink_all_memory(unsigned long nr_pages);
>  extern int vm_swappiness;
> Index: mmotm-0615/mm/memcontrol.c
> ===================================================================
> --- mmotm-0615.orig/mm/memcontrol.c
> +++ mmotm-0615/mm/memcontrol.c
> @@ -203,6 +203,57 @@ struct mem_cgroup_eventfd_list {
>  static void mem_cgroup_threshold(struct mem_cgroup *mem);
>  static void mem_cgroup_oom_notify(struct mem_cgroup *mem);
>
> +enum {
> +       SCAN_BY_LIMIT,
> +       FREED_BY_LIMIT,
> +       ELAPSED_BY_LIMIT,
> +
> +       SCAN_BY_SYSTEM,
> +       FREED_BY_SYSTEM,
> +       ELAPSED_BY_SYSTEM,
> +
> +       SCAN_BY_SHRINK,
> +       FREED_BY_SHRINK,
> +       ELAPSED_BY_SHRINK,
> +       NR_SCANSTATS,
> +};
> +#define __FREED                (1)
> +#define        __ELAPSED       (2)
>

/tab/space/


> +
> +struct scanstat {
> +       spinlock_t      lock;
> +       unsigned long   stats[NR_SCANSTATS];    /* local statistics */
> +       unsigned long   totalstats[NR_SCANSTATS];   /* hierarchical */
> +};
> +
> +const char *scanstat_string[NR_SCANSTATS] = {
> +       "scanned_pages_by_limit",
> +       "freed_pages_by_limit",
> +       "elapsed_ns_by_limit",
> +
> +       "scanned_pages_by_system",
> +       "freed_pages_by_system",
> +       "elapsed_ns_by_system",
> +
> +       "scanned_pages_by_shrink",
> +       "freed_pages_by_shrink",
> +       "elappsed_ns_by_shrink",
>
elapsed?

> +};
> +
> +const char *total_scanstat_string[NR_SCANSTATS] = {
> +       "total_scanned_pages_by_limit",
> +       "total_freed_pages_by_limit",
> +       "total_elapsed_ns_by_hierarchical",
>

typo?


> +
> +       "total_scanned_pages_by_system",
> +       "total_freed_pages_by_system",
> +       "total_elapsed_ns_by_system",
> +
> +       "total_scanned_pages_by_shrink",
> +       "total_freed_pages_by_shrink",
> +       "total_elapsed_ns_by_shrink",
> +};
> +
>  /*
>  * The memory controller data structure. The memory controller controls
> both
>  * page cache and RSS per cgroup. We would eventually like to provide
> @@ -264,7 +315,8 @@ struct mem_cgroup {
>
>        /* For oom notifier event fd */
>        struct list_head oom_notify;
> -
> +       /* For recording LRU-scan statistics */
> +       struct scanstat scanstat;
>        /*
>         * Should we move charges of a task when a task is moved into this
>         * mem_cgroup ? And what type of charges should we move ?
> @@ -1634,6 +1686,28 @@ int mem_cgroup_select_victim_node(struct
>  }
>  #endif
>
> +
> +
> +static void mem_cgroup_record_scanstat(struct memcg_scanrecord *rec)
> +{
> +       struct mem_cgroup *mem;
> +       int context = rec->context;
> +
> +       mem = rec->mem;
> +       spin_lock(&mem->scanstat.lock);
> +       mem->scanstat.stats[context] += rec->nr_scanned;
> +       mem->scanstat.stats[context + __FREED] += rec->nr_freed;
> +       mem->scanstat.stats[context + __ELAPSED] += rec->elappsed;
>
elapsed?


+       spin_unlock(&mem->scanstat.lock);
> +
> +       mem = rec->root;
> +       spin_lock(&mem->scanstat.lock);
> +       mem->scanstat.totalstats[context] += rec->nr_scanned;
> +       mem->scanstat.totalstats[context + __FREED] += rec->nr_freed;
> +       mem->scanstat.totalstats[context + __ELAPSED] += rec->elappsed;
>

elapsed?

> +       spin_unlock(&mem->scanstat.lock);
> +}
> +
>  /*
>  * Scan the hierarchy if needed to reclaim memory. We remember the last
> child
>  * we reclaimed from, so that we don't end up penalizing one child
> extensively
> @@ -1659,8 +1733,8 @@ static int mem_cgroup_hierarchical_recla
>        bool shrink = reclaim_options & MEM_CGROUP_RECLAIM_SHRINK;
>        bool check_soft = reclaim_options & MEM_CGROUP_RECLAIM_SOFT;
>        unsigned long excess;
> -       unsigned long nr_scanned;
>        int visit;
> +       struct memcg_scanrecord rec;
>
>        excess = res_counter_soft_limit_excess(&root_mem->res) >>
> PAGE_SHIFT;
>
> @@ -1668,6 +1742,15 @@ static int mem_cgroup_hierarchical_recla
>        if (!check_soft && root_mem->memsw_is_minimum)
>                noswap = true;
>
> +       if (shrink)
> +               rec.context = SCAN_BY_SHRINK;
> +       else if (check_soft)
> +               rec.context = SCAN_BY_SYSTEM;
> +       else
> +               rec.context = SCAN_BY_LIMIT;
> +
> +       rec.root = root_mem;
>



> +
>  again:
>        if (!shrink) {
>                visit = 0;
> @@ -1695,14 +1778,19 @@ again:
>                        css_put(&victim->css);
>                        continue;
>                }
> +               rec.mem = victim;
> +               rec.nr_scanned = 0;
> +               rec.nr_freed = 0;
> +               rec.elappsed = 0;
>
elapsed?

>                /* we use swappiness of local cgroup */
>                if (check_soft) {
>                        ret = mem_cgroup_shrink_node_zone(victim, gfp_mask,
> -                               noswap, zone, &nr_scanned);
> -                       *total_scanned += nr_scanned;
> +                               noswap, zone, &rec);
> +                       *total_scanned += rec.nr_scanned;
>                } else
>                        ret = try_to_free_mem_cgroup_pages(victim, gfp_mask,
> -                                               noswap);
> +                                               noswap, &rec);
> +               mem_cgroup_record_scanstat(&rec);
>                css_put(&victim->css);
>
>                total += ret;
> @@ -3757,7 +3845,8 @@ try_to_free:
>                        ret = -EINTR;
>                        goto out;
>                }
> -               progress = try_to_free_mem_cgroup_pages(mem, GFP_KERNEL,
> false);
> +               progress = try_to_free_mem_cgroup_pages(mem,
> +                               GFP_KERNEL, false, NULL);
>

So we don't record the stat for force_empty case?


>                if (!progress) {
>                        nr_retries--;
>                        /* maybe some writeback is necessary */
> @@ -4599,6 +4688,34 @@ static int mem_control_numa_stat_open(st
>  }
>  #endif /* CONFIG_NUMA */
>
> +static int mem_cgroup_scan_stat_read(struct cgroup *cgrp,
> +                               struct cftype *cft,
> +                               struct cgroup_map_cb *cb)
> +{
> +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
> +       int i;
> +
> +       for (i = 0; i < NR_SCANSTATS; i++)
> +               cb->fill(cb, scanstat_string[i], mem->scanstat.stats[i]);
> +       for (i = 0; i < NR_SCANSTATS; i++)
> +               cb->fill(cb, total_scanstat_string[i],
> +                       mem->scanstat.totalstats[i]);
> +       return 0;
> +}
> +
> +static int mem_cgroup_reset_scan_stat(struct cgroup *cgrp,
> +                               unsigned int event)
> +{
> +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
> +
> +       spin_lock(&mem->scanstat.lock);
> +       memset(&mem->scanstat.stats, 0, sizeof(mem->scanstat.stats));
> +       memset(&mem->scanstat.totalstats, 0,
> sizeof(mem->scanstat.totalstats));
> +       spin_unlock(&mem->scanstat.lock);
> +       return 0;
> +}
> +
> +
>  static struct cftype mem_cgroup_files[] = {
>        {
>                .name = "usage_in_bytes",
> @@ -4669,6 +4786,11 @@ static struct cftype mem_cgroup_files[]
>                .mode = S_IRUGO,
>        },
>  #endif
> +       {
> +               .name = "scan_stat",
> +               .read_map = mem_cgroup_scan_stat_read,
> +               .trigger = mem_cgroup_reset_scan_stat,
> +       },
>  };
>
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> @@ -4932,6 +5054,7 @@ mem_cgroup_create(struct cgroup_subsys *
>        atomic_set(&mem->refcnt, 1);
>        mem->move_charge_at_immigrate = 0;
>        mutex_init(&mem->thresholds_lock);
> +       spin_lock_init(&mem->scanstat.lock);
>        return &mem->css;
>  free_out:
>        __mem_cgroup_free(mem);
> Index: mmotm-0615/mm/vmscan.c
> ===================================================================
> --- mmotm-0615.orig/mm/vmscan.c
> +++ mmotm-0615/mm/vmscan.c
> @@ -2199,9 +2199,9 @@ unsigned long try_to_free_pages(struct z
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR
>
>  unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> -                                               gfp_t gfp_mask, bool
> noswap,
> -                                               struct zone *zone,
> -                                               unsigned long *nr_scanned)
> +                                       gfp_t gfp_mask, bool noswap,
> +                                       struct zone *zone,
> +                                       struct memcg_scanrecord *rec)
>  {
>        struct scan_control sc = {
>                .nr_scanned = 0,
> @@ -2213,6 +2213,7 @@ unsigned long mem_cgroup_shrink_node_zon
>                .order = 0,
>                .mem_cgroup = mem,
>        };
> +       unsigned long start, end;
>
>        sc.gfp_mask = (gfp_mask & GFP_RECLAIM_MASK) |
>                        (GFP_HIGHUSER_MOVABLE & ~GFP_RECLAIM_MASK);
> @@ -2221,6 +2222,7 @@ unsigned long mem_cgroup_shrink_node_zon
>                                                      sc.may_writepage,
>                                                      sc.gfp_mask);
>
> +       start = sched_clock();
>        /*
>         * NOTE: Although we can get the priority field, using it
>         * here is not a good idea, since it limits the pages we can scan.
> @@ -2229,19 +2231,27 @@ unsigned long mem_cgroup_shrink_node_zon
>         * the priority and make it zero.
>         */
>        shrink_zone(0, zone, &sc);
> +       end = sched_clock();
> +
> +       if (rec) {
> +               rec->nr_scanned += sc.nr_scanned;
>



> +               rec->nr_freed += sc.nr_reclaimed;
> +               rec->elappsed += end - start;
>
elapsed?

> +       }
>
>        trace_mm_vmscan_memcg_softlimit_reclaim_end(sc.nr_reclaimed);
>
> -       *nr_scanned = sc.nr_scanned;
>        return sc.nr_reclaimed;
>  }
>
>  unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem_cont,
>                                           gfp_t gfp_mask,
> -                                          bool noswap)
> +                                          bool noswap,
> +                                          struct memcg_scanrecord *rec)
>  {
>        struct zonelist *zonelist;
>        unsigned long nr_reclaimed;
> +       unsigned long start, end;
>        int nid;
>        struct scan_control sc = {
>                .may_writepage = !laptop_mode,
> @@ -2259,6 +2269,7 @@ unsigned long try_to_free_mem_cgroup_pag
>                .gfp_mask = sc.gfp_mask,
>        };
>
> +       start = sched_clock();
>        /*
>         * Unlike direct reclaim via alloc_pages(), memcg's reclaim doesn't
>         * take care of from where we get pages. So the node where we start
> the
> @@ -2273,6 +2284,12 @@ unsigned long try_to_free_mem_cgroup_pag
>                                            sc.gfp_mask);
>
>        nr_reclaimed = do_try_to_free_pages(zonelist, &sc, &shrink);
> +       end = sched_clock();
> +       if (rec) {
> +               rec->nr_scanned += sc.nr_scanned;
>

sc.nr_scanned only contains the nr_scanned of last
priority do_try_to_free_pages(). we need to reset it to total_scanned before
return. I am looking at v3.0-rc3 .


> +               rec->nr_freed += sc.nr_reclaimed;
> +               rec->elappsed += end - start;
>
elapsed?

> +       }
>
>        trace_mm_vmscan_memcg_reclaim_end(nr_reclaimed);
>
>
> --Ying

[-- Attachment #2: Type: text/html, Size: 24408 bytes --]

Re: [PATCH 3/7] memcg: add memory.scan_stat

[Ying Han]

On Mon, Jun 20, 2011 at 11:49 PM, Ying Han <yinghan@google.com> wrote:
>
>
> On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>
>> From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> Date: Wed, 15 Jun 2011 14:11:01 +0900
>> Subject: [PATCH 3/7] memcg: add memory.scan_stat
>>
>> commit log of commit 0ae5e89 " memcg: count the soft_limit reclaim in..."
>> says it adds scanning stats to memory.stat file. But it doesn't because
>> we considered we needed to make a concensus for such new APIs.
>>
>> This patch is a trial to add memory.scan_stat. This shows
>>  - the number of scanned pages
>>  - the number of recleimed pages
>>  - the number of elaplsed time (including sleep/pause time)
>>  for both of direct/soft reclaim and shrinking caused by changing limit
>>  or force_empty.
>>
>> The biggest difference with oringinal Ying's one is that this file
>> can be reset by some write, as
>>
>>  # echo 0 ...../memory.scan_stat
>>
>> [kamezawa@bluextal ~]$ cat /cgroup/memory/A/memory.scan_stat
>> scanned_pages_by_limit 358470
>> freed_pages_by_limit 180795
>> elapsed_ns_by_limit 21629927
>> scanned_pages_by_system 0
>> freed_pages_by_system 0
>> elapsed_ns_by_system 0
>> scanned_pages_by_shrink 76646
>> freed_pages_by_shrink 38355
>> elappsed_ns_by_shrink 31990670
>
> elapsed?
>
>>
>> total_scanned_pages_by_limit 358470
>> total_freed_pages_by_limit 180795
>> total_elapsed_ns_by_hierarchical 216299275
>> total_scanned_pages_by_system 0
>> total_freed_pages_by_system 0
>> total_elapsed_ns_by_system 0
>> total_scanned_pages_by_shrink 76646
>> total_freed_pages_by_shrink 38355
>> total_elapsed_ns_by_shrink 31990670
>>
>> total_xxxx is for hierarchy management.
>
> For some reason, i feel the opposite where the local stat (like "scanned_pages_by_limit") are reclaimed under hierarchical reclaim. The total_xxx stats are only incremented for root_mem which is the cgroup triggers the hierarchical reclaim. So:
> total_scanned_pages_by_limit: number of pages being scanned while the memcg hits its limit
> scanned_pages_by_limit: number of pages being scanned while one of the memcg's ancestor hits its limit
> am i missing something?
>
>>
>> This will be useful for further memcg developments and need to be
>> developped before we do some complicated rework on LRU/softlimit
>> management.
>>
>> Now, scan/free/elapsed_by_system is incomplete but future works of
>> Johannes at el. will fill remaining information and then, we can
>> look into problems of isolation between memcgs.
>>
>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> ---
>>  Documentation/cgroups/memory.txt |   33 +++++++++
>>  include/linux/memcontrol.h       |   16 ++++
>>  include/linux/swap.h             |    6 -
>>  mm/memcontrol.c                  |  135 +++++++++++++++++++++++++++++++++++++--
>>  mm/vmscan.c                      |   27 ++++++-
>>  5 files changed, 199 insertions(+), 18 deletions(-)
>>
>> Index: mmotm-0615/Documentation/cgroups/memory.txt
>> ===================================================================
>> --- mmotm-0615.orig/Documentation/cgroups/memory.txt
>> +++ mmotm-0615/Documentation/cgroups/memory.txt
>> @@ -380,7 +380,7 @@ will be charged as a new owner of it.
>>
>>  5.2 stat file
>>
>> -memory.stat file includes following statistics
>> +5.2.1 memory.stat file includes following statistics
>>
>>  # per-memory cgroup local status
>>  cache          - # of bytes of page cache memory.
>> @@ -438,6 +438,37 @@ Note:
>>         file_mapped is accounted only when the memory cgroup is owner of page
>>         cache.)
>>
>> +5.2.2 memory.scan_stat
>> +
>> +memory.scan_stat includes statistics information for memory scanning and
>> +freeing, reclaiming. The statistics shows memory scanning information since
>> +memory cgroup creation and can be reset to 0 by writing 0 as
>> +
>> + #echo 0 > ../memory.scan_stat
>> +
>> +This file contains following statistics.
>> +
>> +scanned_pages_by_limit - # of scanned pages at hitting limit.
>> +freed_pages_by_limit   - # of freed pages at hitting limit.
>> +elapsed_ns_by_limit    - nano sec of elappsed time at LRU scan at
>> +                                  hitting limit.(this includes sleep time.)
>
> elapsed?
>>
>> +
>> +scanned_pages_by_system        - # of scanned pages by the kernel.
>> +                         (Now, this value means global memory reclaim
>> +                           caused by system memory shortage with a hint
>> +                          of softlimit. "No soft limit" case will be
>> +                          supported in future.)
>> +freed_pages_by_system  - # of freed pages by the kernel.
>> +elapsed_ns_by_system   - nano sec of elappsed time by kernel.
>> +
>> +scanned_pages_by_shrink        - # of scanned pages by shrinking.
>> +                                 (i.e. changes of limit, force_empty, etc.)
>> +freed_pages_by_shrink  - # of freed pages by shirkining.
>> +elappsed_ns_by_shrink  - nano sec of elappsed time at shrinking.
>
> elapsed?
>>
>> +
>> +total_xxx includes the statistics of children scanning caused by the cgroup.
>
> based on the code inspection, the total_xxx also includes the cgroup's scan stat as well.
>>
>> +
>> +
>>  5.3 swappiness
>>
>>  Similar to /proc/sys/vm/swappiness, but affecting a hierarchy of groups only.
>> Index: mmotm-0615/include/linux/memcontrol.h
>> ===================================================================
>> --- mmotm-0615.orig/include/linux/memcontrol.h
>> +++ mmotm-0615/include/linux/memcontrol.h
>> @@ -120,6 +120,22 @@ struct zone_reclaim_stat*
>>  mem_cgroup_get_reclaim_stat_from_page(struct page *page);
>>  extern void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
>>                                        struct task_struct *p);
>> +struct memcg_scanrecord {
>> +       struct mem_cgroup *mem; /* scanend memory cgroup */
>> +       struct mem_cgroup *root; /* scan target hierarchy root */
>> +       int context;            /* scanning context (see memcontrol.c) */
>> +       unsigned long nr_scanned; /* the number of scanned pages */
>> +       unsigned long nr_freed; /* the number of freed pages */
>> +       unsigned long elappsed; /* nsec of time elapsed while scanning */
>
> elapsed?
>>
>> +};
>> +
>> +extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
>> +                                                 gfp_t gfp_mask, bool noswap,
>> +                                                 struct memcg_scanrecord *rec);
>> +extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
>> +                                               gfp_t gfp_mask, bool noswap,
>> +                                               struct zone *zone,
>> +                                               struct memcg_scanrecord *rec);
>>
>>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>>  extern int do_swap_account;
>> Index: mmotm-0615/include/linux/swap.h
>> ===================================================================
>> --- mmotm-0615.orig/include/linux/swap.h
>> +++ mmotm-0615/include/linux/swap.h
>> @@ -253,12 +253,6 @@ static inline void lru_cache_add_file(st
>>  /* linux/mm/vmscan.c */
>>  extern unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
>>                                        gfp_t gfp_mask, nodemask_t *mask);
>> -extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
>> -                                                 gfp_t gfp_mask, bool noswap);
>> -extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
>> -                                               gfp_t gfp_mask, bool noswap,
>> -                                               struct zone *zone,
>> -                                               unsigned long *nr_scanned);
>>  extern int __isolate_lru_page(struct page *page, int mode, int file);
>>  extern unsigned long shrink_all_memory(unsigned long nr_pages);
>>  extern int vm_swappiness;
>> Index: mmotm-0615/mm/memcontrol.c
>> ===================================================================
>> --- mmotm-0615.orig/mm/memcontrol.c
>> +++ mmotm-0615/mm/memcontrol.c
>> @@ -203,6 +203,57 @@ struct mem_cgroup_eventfd_list {
>>  static void mem_cgroup_threshold(struct mem_cgroup *mem);
>>  static void mem_cgroup_oom_notify(struct mem_cgroup *mem);
>>
>> +enum {
>> +       SCAN_BY_LIMIT,
>> +       FREED_BY_LIMIT,
>> +       ELAPSED_BY_LIMIT,
>> +
>> +       SCAN_BY_SYSTEM,
>> +       FREED_BY_SYSTEM,
>> +       ELAPSED_BY_SYSTEM,
>> +
>> +       SCAN_BY_SHRINK,
>> +       FREED_BY_SHRINK,
>> +       ELAPSED_BY_SHRINK,
>> +       NR_SCANSTATS,
>> +};
>> +#define __FREED                (1)
>> +#define        __ELAPSED       (2)
>
> /tab/space/
>
>>
>> +
>> +struct scanstat {
>> +       spinlock_t      lock;
>> +       unsigned long   stats[NR_SCANSTATS];    /* local statistics */
>> +       unsigned long   totalstats[NR_SCANSTATS];   /* hierarchical */
>> +};
>> +
>> +const char *scanstat_string[NR_SCANSTATS] = {
>> +       "scanned_pages_by_limit",
>> +       "freed_pages_by_limit",
>> +       "elapsed_ns_by_limit",
>> +
>> +       "scanned_pages_by_system",
>> +       "freed_pages_by_system",
>> +       "elapsed_ns_by_system",
>> +
>> +       "scanned_pages_by_shrink",
>> +       "freed_pages_by_shrink",
>> +       "elappsed_ns_by_shrink",
>
> elapsed?
>>
>> +};
>> +
>> +const char *total_scanstat_string[NR_SCANSTATS] = {
>> +       "total_scanned_pages_by_limit",
>> +       "total_freed_pages_by_limit",
>> +       "total_elapsed_ns_by_hierarchical",
>
> typo?
>
>>
>> +
>> +       "total_scanned_pages_by_system",
>> +       "total_freed_pages_by_system",
>> +       "total_elapsed_ns_by_system",
>> +
>> +       "total_scanned_pages_by_shrink",
>> +       "total_freed_pages_by_shrink",
>> +       "total_elapsed_ns_by_shrink",
>> +};
>> +
>>  /*
>>  * The memory controller data structure. The memory controller controls both
>>  * page cache and RSS per cgroup. We would eventually like to provide
>> @@ -264,7 +315,8 @@ struct mem_cgroup {
>>
>>        /* For oom notifier event fd */
>>        struct list_head oom_notify;
>> -
>> +       /* For recording LRU-scan statistics */
>> +       struct scanstat scanstat;
>>        /*
>>         * Should we move charges of a task when a task is moved into this
>>         * mem_cgroup ? And what type of charges should we move ?
>> @@ -1634,6 +1686,28 @@ int mem_cgroup_select_victim_node(struct
>>  }
>>  #endif
>>
>> +
>> +
>> +static void mem_cgroup_record_scanstat(struct memcg_scanrecord *rec)
>> +{
>> +       struct mem_cgroup *mem;
>> +       int context = rec->context;
>> +
>> +       mem = rec->mem;
>> +       spin_lock(&mem->scanstat.lock);
>> +       mem->scanstat.stats[context] += rec->nr_scanned;
>> +       mem->scanstat.stats[context + __FREED] += rec->nr_freed;
>> +       mem->scanstat.stats[context + __ELAPSED] += rec->elappsed;
>
> elapsed?
>
>> +       spin_unlock(&mem->scanstat.lock);
>> +
>> +       mem = rec->root;
>> +       spin_lock(&mem->scanstat.lock);
>> +       mem->scanstat.totalstats[context] += rec->nr_scanned;
>> +       mem->scanstat.totalstats[context + __FREED] += rec->nr_freed;
>> +       mem->scanstat.totalstats[context + __ELAPSED] += rec->elappsed;
>
> elapsed?
>>
>> +       spin_unlock(&mem->scanstat.lock);
>> +}
>> +
>>  /*
>>  * Scan the hierarchy if needed to reclaim memory. We remember the last child
>>  * we reclaimed from, so that we don't end up penalizing one child extensively
>> @@ -1659,8 +1733,8 @@ static int mem_cgroup_hierarchical_recla
>>        bool shrink = reclaim_options & MEM_CGROUP_RECLAIM_SHRINK;
>>        bool check_soft = reclaim_options & MEM_CGROUP_RECLAIM_SOFT;
>>        unsigned long excess;
>> -       unsigned long nr_scanned;
>>        int visit;
>> +       struct memcg_scanrecord rec;
>>
>>        excess = res_counter_soft_limit_excess(&root_mem->res) >> PAGE_SHIFT;
>>
>> @@ -1668,6 +1742,15 @@ static int mem_cgroup_hierarchical_recla
>>        if (!check_soft && root_mem->memsw_is_minimum)
>>                noswap = true;
>>
>> +       if (shrink)
>> +               rec.context = SCAN_BY_SHRINK;
>> +       else if (check_soft)
>> +               rec.context = SCAN_BY_SYSTEM;
>> +       else
>> +               rec.context = SCAN_BY_LIMIT;
>> +
>> +       rec.root = root_mem;
>
>
>
>>
>> +
>>  again:
>>        if (!shrink) {
>>                visit = 0;
>> @@ -1695,14 +1778,19 @@ again:
>>                        css_put(&victim->css);
>>                        continue;
>>                }
>> +               rec.mem = victim;
>> +               rec.nr_scanned = 0;
>> +               rec.nr_freed = 0;
>> +               rec.elappsed = 0;
>
> elapsed?
>>
>>                /* we use swappiness of local cgroup */
>>                if (check_soft) {
>>                        ret = mem_cgroup_shrink_node_zone(victim, gfp_mask,
>> -                               noswap, zone, &nr_scanned);
>> -                       *total_scanned += nr_scanned;
>> +                               noswap, zone, &rec);
>> +                       *total_scanned += rec.nr_scanned;
>>                } else
>>                        ret = try_to_free_mem_cgroup_pages(victim, gfp_mask,
>> -                                               noswap);
>> +                                               noswap, &rec);
>> +               mem_cgroup_record_scanstat(&rec);
>>                css_put(&victim->css);
>>
>>                total += ret;
>> @@ -3757,7 +3845,8 @@ try_to_free:
>>                        ret = -EINTR;
>>                        goto out;
>>                }
>> -               progress = try_to_free_mem_cgroup_pages(mem, GFP_KERNEL, false);
>> +               progress = try_to_free_mem_cgroup_pages(mem,
>> +                               GFP_KERNEL, false, NULL);
>
> So we don't record the stat for force_empty case?
>
>>
>>                if (!progress) {
>>                        nr_retries--;
>>                        /* maybe some writeback is necessary */
>> @@ -4599,6 +4688,34 @@ static int mem_control_numa_stat_open(st
>>  }
>>  #endif /* CONFIG_NUMA */
>>
>> +static int mem_cgroup_scan_stat_read(struct cgroup *cgrp,
>> +                               struct cftype *cft,
>> +                               struct cgroup_map_cb *cb)
>> +{
>> +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
>> +       int i;
>> +
>> +       for (i = 0; i < NR_SCANSTATS; i++)
>> +               cb->fill(cb, scanstat_string[i], mem->scanstat.stats[i]);
>> +       for (i = 0; i < NR_SCANSTATS; i++)
>> +               cb->fill(cb, total_scanstat_string[i],
>> +                       mem->scanstat.totalstats[i]);
>> +       return 0;
>> +}
>> +
>> +static int mem_cgroup_reset_scan_stat(struct cgroup *cgrp,
>> +                               unsigned int event)
>> +{
>> +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
>> +
>> +       spin_lock(&mem->scanstat.lock);
>> +       memset(&mem->scanstat.stats, 0, sizeof(mem->scanstat.stats));
>> +       memset(&mem->scanstat.totalstats, 0, sizeof(mem->scanstat.totalstats));
>> +       spin_unlock(&mem->scanstat.lock);
>> +       return 0;
>> +}
>> +
>> +
>>  static struct cftype mem_cgroup_files[] = {
>>        {
>>                .name = "usage_in_bytes",
>> @@ -4669,6 +4786,11 @@ static struct cftype mem_cgroup_files[]
>>                .mode = S_IRUGO,
>>        },
>>  #endif
>> +       {
>> +               .name = "scan_stat",
>> +               .read_map = mem_cgroup_scan_stat_read,
>> +               .trigger = mem_cgroup_reset_scan_stat,
>> +       },
>>  };
>>
>>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>> @@ -4932,6 +5054,7 @@ mem_cgroup_create(struct cgroup_subsys *
>>        atomic_set(&mem->refcnt, 1);
>>        mem->move_charge_at_immigrate = 0;
>>        mutex_init(&mem->thresholds_lock);
>> +       spin_lock_init(&mem->scanstat.lock);
>>        return &mem->css;
>>  free_out:
>>        __mem_cgroup_free(mem);
>> Index: mmotm-0615/mm/vmscan.c
>> ===================================================================
>> --- mmotm-0615.orig/mm/vmscan.c
>> +++ mmotm-0615/mm/vmscan.c
>> @@ -2199,9 +2199,9 @@ unsigned long try_to_free_pages(struct z
>>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR
>>
>>  unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
>> -                                               gfp_t gfp_mask, bool noswap,
>> -                                               struct zone *zone,
>> -                                               unsigned long *nr_scanned)
>> +                                       gfp_t gfp_mask, bool noswap,
>> +                                       struct zone *zone,
>> +                                       struct memcg_scanrecord *rec)
>>  {
>>        struct scan_control sc = {
>>                .nr_scanned = 0,
>> @@ -2213,6 +2213,7 @@ unsigned long mem_cgroup_shrink_node_zon
>>                .order = 0,
>>                .mem_cgroup = mem,
>>        };
>> +       unsigned long start, end;
>>
>>        sc.gfp_mask = (gfp_mask & GFP_RECLAIM_MASK) |
>>                        (GFP_HIGHUSER_MOVABLE & ~GFP_RECLAIM_MASK);
>> @@ -2221,6 +2222,7 @@ unsigned long mem_cgroup_shrink_node_zon
>>                                                      sc.may_writepage,
>>                                                      sc.gfp_mask);
>>
>> +       start = sched_clock();
>>        /*
>>         * NOTE: Although we can get the priority field, using it
>>         * here is not a good idea, since it limits the pages we can scan.
>> @@ -2229,19 +2231,27 @@ unsigned long mem_cgroup_shrink_node_zon
>>         * the priority and make it zero.
>>         */
>>        shrink_zone(0, zone, &sc);
>> +       end = sched_clock();
>> +
>> +       if (rec) {
>> +               rec->nr_scanned += sc.nr_scanned;
>
>
>>
>> +               rec->nr_freed += sc.nr_reclaimed;
>> +               rec->elappsed += end - start;
>
> elapsed?
>>
>> +       }
>>
>>        trace_mm_vmscan_memcg_softlimit_reclaim_end(sc.nr_reclaimed);
>>
>> -       *nr_scanned = sc.nr_scanned;
>>        return sc.nr_reclaimed;
>>  }
>>
>>  unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem_cont,
>>                                           gfp_t gfp_mask,
>> -                                          bool noswap)
>> +                                          bool noswap,
>> +                                          struct memcg_scanrecord *rec)
>>  {
>>        struct zonelist *zonelist;
>>        unsigned long nr_reclaimed;
>> +       unsigned long start, end;
>>        int nid;
>>        struct scan_control sc = {
>>                .may_writepage = !laptop_mode,
>> @@ -2259,6 +2269,7 @@ unsigned long try_to_free_mem_cgroup_pag
>>                .gfp_mask = sc.gfp_mask,
>>        };
>>
>> +       start = sched_clock();
>>        /*
>>         * Unlike direct reclaim via alloc_pages(), memcg's reclaim doesn't
>>         * take care of from where we get pages. So the node where we start the
>> @@ -2273,6 +2284,12 @@ unsigned long try_to_free_mem_cgroup_pag
>>                                            sc.gfp_mask);
>>
>>        nr_reclaimed = do_try_to_free_pages(zonelist, &sc, &shrink);
>> +       end = sched_clock();
>> +       if (rec) {
>> +               rec->nr_scanned += sc.nr_scanned;
>
> sc.nr_scanned only contains the nr_scanned of last priority do_try_to_free_pages(). we need to reset it to total_scanned before return. I am looking at v3.0-rc3 .
>
>>
>> +               rec->nr_freed += sc.nr_reclaimed;
>> +               rec->elappsed += end - start;
>
> elapsed?
>>
>> +       }
>>
>>        trace_mm_vmscan_memcg_reclaim_end(nr_reclaimed);
>>
>>
> --Ying

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/7] memcg: add memory.scan_stat

[KAMEZAWA Hiroyuki]

On Mon, 20 Jun 2011 23:49:54 -0700
Ying Han <yinghan@google.com> wrote:

> On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki <
> kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Wed, 15 Jun 2011 14:11:01 +0900
> > Subject: [PATCH 3/7] memcg: add memory.scan_stat
> >
> > commit log of commit 0ae5e89 " memcg: count the soft_limit reclaim in..."
> > says it adds scanning stats to memory.stat file. But it doesn't because
> > we considered we needed to make a concensus for such new APIs.
> >
> > This patch is a trial to add memory.scan_stat. This shows
> >  - the number of scanned pages
> >  - the number of recleimed pages
> >  - the number of elaplsed time (including sleep/pause time)
> >  for both of direct/soft reclaim and shrinking caused by changing limit
> >  or force_empty.
> >
> > The biggest difference with oringinal Ying's one is that this file
> > can be reset by some write, as
> >
> >  # echo 0 ...../memory.scan_stat
> >
> > [kamezawa@bluextal ~]$ cat /cgroup/memory/A/memory.scan_stat
> > scanned_pages_by_limit 358470
> > freed_pages_by_limit 180795
> > elapsed_ns_by_limit 21629927
> > scanned_pages_by_system 0
> > freed_pages_by_system 0
> > elapsed_ns_by_system 0
> > scanned_pages_by_shrink 76646
> > freed_pages_by_shrink 38355
> > elappsed_ns_by_shrink 31990670
> >
> 
> elapsed?
> 

you'r right.

> 
> > total_scanned_pages_by_limit 358470
> > total_freed_pages_by_limit 180795
> > total_elapsed_ns_by_hierarchical 216299275
> > total_scanned_pages_by_system 0
> > total_freed_pages_by_system 0
> > total_elapsed_ns_by_system 0
> > total_scanned_pages_by_shrink 76646
> > total_freed_pages_by_shrink 38355
> > total_elapsed_ns_by_shrink 31990670
> >
> > total_xxxx is for hierarchy management.
> >
> 
> For some reason, i feel the opposite where the local stat (like
> "scanned_pages_by_limit") are reclaimed under hierarchical reclaim. The
> total_xxx stats are only incremented for root_mem which is the cgroup
> triggers the hierarchical reclaim. So:
> 
> total_scanned_pages_by_limit: number of pages being scanned while the memcg
> hits its limit
> scanned_pages_by_limit: number of pages being scanned while one of the
> memcg's ancestor hits its limit
> 
> am i missing something?
> 

scanned_pages_by_limit: one of ancestors and itself's limit.




> 
> >
> > This will be useful for further memcg developments and need to be
> > developped before we do some complicated rework on LRU/softlimit
> > management.
> >
> > Now, scan/free/elapsed_by_system is incomplete but future works of
> > Johannes at el. will fill remaining information and then, we can
> > look into problems of isolation between memcgs.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> >  Documentation/cgroups/memory.txt |   33 +++++++++
> >  include/linux/memcontrol.h       |   16 ++++
> >  include/linux/swap.h             |    6 -
> >  mm/memcontrol.c                  |  135
> > +++++++++++++++++++++++++++++++++++++--
> >  mm/vmscan.c                      |   27 ++++++-
> >  5 files changed, 199 insertions(+), 18 deletions(-)
> >
> > Index: mmotm-0615/Documentation/cgroups/memory.txt
> > ===================================================================
> > --- mmotm-0615.orig/Documentation/cgroups/memory.txt
> > +++ mmotm-0615/Documentation/cgroups/memory.txt
> > @@ -380,7 +380,7 @@ will be charged as a new owner of it.
> >
> >  5.2 stat file
> >
> > -memory.stat file includes following statistics
> > +5.2.1 memory.stat file includes following statistics
> >
> >  # per-memory cgroup local status
> >  cache          - # of bytes of page cache memory.
> > @@ -438,6 +438,37 @@ Note:
> >         file_mapped is accounted only when the memory cgroup is owner of
> > page
> >         cache.)
> >
> > +5.2.2 memory.scan_stat
> > +
> > +memory.scan_stat includes statistics information for memory scanning and
> > +freeing, reclaiming. The statistics shows memory scanning information
> > since
> > +memory cgroup creation and can be reset to 0 by writing 0 as
> > +
> > + #echo 0 > ../memory.scan_stat
> > +
> > +This file contains following statistics.
> > +
> > +scanned_pages_by_limit - # of scanned pages at hitting limit.
> > +freed_pages_by_limit   - # of freed pages at hitting limit.
> > +elapsed_ns_by_limit    - nano sec of elappsed time at LRU scan at
> > +                                  hitting limit.(this includes sleep
> > time.)
> >
> elapsed?
> 
> > +
> > +scanned_pages_by_system        - # of scanned pages by the kernel.
> > +                         (Now, this value means global memory reclaim
> > +                           caused by system memory shortage with a hint
> > +                          of softlimit. "No soft limit" case will be
> > +                          supported in future.)
> > +freed_pages_by_system  - # of freed pages by the kernel.
> > +elapsed_ns_by_system   - nano sec of elappsed time by kernel.
> > +
> > +scanned_pages_by_shrink        - # of scanned pages by shrinking.
> > +                                 (i.e. changes of limit, force_empty,
> > etc.)
> > +freed_pages_by_shrink  - # of freed pages by shirkining.
> > +elappsed_ns_by_shrink  - nano sec of elappsed time at shrinking.
> >
> elapsed?
> 
> > +
> > +total_xxx includes the statistics of children scanning caused by the
> > cgroup.
> >
> 
> based on the code inspection, the total_xxx also includes the cgroup's scan
> stat as well.
> 

yes.


> +
> > +
> >  5.3 swappiness
> >
> >  Similar to /proc/sys/vm/swappiness, but affecting a hierarchy of groups
> > only.
> > Index: mmotm-0615/include/linux/memcontrol.h
> > ===================================================================
> > --- mmotm-0615.orig/include/linux/memcontrol.h
> > +++ mmotm-0615/include/linux/memcontrol.h
> > @@ -120,6 +120,22 @@ struct zone_reclaim_stat*
> >  mem_cgroup_get_reclaim_stat_from_page(struct page *page);
> >  extern void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
> >                                        struct task_struct *p);
> > +struct memcg_scanrecord {
> > +       struct mem_cgroup *mem; /* scanend memory cgroup */
> > +       struct mem_cgroup *root; /* scan target hierarchy root */
> > +       int context;            /* scanning context (see memcontrol.c) */
> > +       unsigned long nr_scanned; /* the number of scanned pages */
> > +       unsigned long nr_freed; /* the number of freed pages */
> > +       unsigned long elappsed; /* nsec of time elapsed while scanning */
> >
> elapsed?
> 
> > +};
> > +
> > +extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
> > +                                                 gfp_t gfp_mask, bool
> > noswap,
> > +                                                 struct memcg_scanrecord
> > *rec);
> > +extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> > +                                               gfp_t gfp_mask, bool
> > noswap,
> > +                                               struct zone *zone,
> > +                                               struct memcg_scanrecord
> > *rec);
> >
> >  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> >  extern int do_swap_account;
> > Index: mmotm-0615/include/linux/swap.h
> > ===================================================================
> > --- mmotm-0615.orig/include/linux/swap.h
> > +++ mmotm-0615/include/linux/swap.h
> > @@ -253,12 +253,6 @@ static inline void lru_cache_add_file(st
> >  /* linux/mm/vmscan.c */
> >  extern unsigned long try_to_free_pages(struct zonelist *zonelist, int
> > order,
> >                                        gfp_t gfp_mask, nodemask_t *mask);
> > -extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
> > -                                                 gfp_t gfp_mask, bool
> > noswap);
> > -extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> > -                                               gfp_t gfp_mask, bool
> > noswap,
> > -                                               struct zone *zone,
> > -                                               unsigned long *nr_scanned);
> >  extern int __isolate_lru_page(struct page *page, int mode, int file);
> >  extern unsigned long shrink_all_memory(unsigned long nr_pages);
> >  extern int vm_swappiness;
> > Index: mmotm-0615/mm/memcontrol.c
> > ===================================================================
> > --- mmotm-0615.orig/mm/memcontrol.c
> > +++ mmotm-0615/mm/memcontrol.c
> > @@ -203,6 +203,57 @@ struct mem_cgroup_eventfd_list {
> >  static void mem_cgroup_threshold(struct mem_cgroup *mem);
> >  static void mem_cgroup_oom_notify(struct mem_cgroup *mem);
> >
> > +enum {
> > +       SCAN_BY_LIMIT,
> > +       FREED_BY_LIMIT,
> > +       ELAPSED_BY_LIMIT,
> > +
> > +       SCAN_BY_SYSTEM,
> > +       FREED_BY_SYSTEM,
> > +       ELAPSED_BY_SYSTEM,
> > +
> > +       SCAN_BY_SHRINK,
> > +       FREED_BY_SHRINK,
> > +       ELAPSED_BY_SHRINK,
> > +       NR_SCANSTATS,
> > +};
> > +#define __FREED                (1)
> > +#define        __ELAPSED       (2)
> >
> 
> /tab/space/
> 
> 
> > +
> > +struct scanstat {
> > +       spinlock_t      lock;
> > +       unsigned long   stats[NR_SCANSTATS];    /* local statistics */
> > +       unsigned long   totalstats[NR_SCANSTATS];   /* hierarchical */
> > +};
> > +
> > +const char *scanstat_string[NR_SCANSTATS] = {
> > +       "scanned_pages_by_limit",
> > +       "freed_pages_by_limit",
> > +       "elapsed_ns_by_limit",
> > +
> > +       "scanned_pages_by_system",
> > +       "freed_pages_by_system",
> > +       "elapsed_ns_by_system",
> > +
> > +       "scanned_pages_by_shrink",
> > +       "freed_pages_by_shrink",
> > +       "elappsed_ns_by_shrink",
> >
> elapsed?
> 
> > +};
> > +
> > +const char *total_scanstat_string[NR_SCANSTATS] = {
> > +       "total_scanned_pages_by_limit",
> > +       "total_freed_pages_by_limit",
> > +       "total_elapsed_ns_by_hierarchical",
> >
> 
> typo?
> 
> 
> > +
> > +       "total_scanned_pages_by_system",
> > +       "total_freed_pages_by_system",
> > +       "total_elapsed_ns_by_system",
> > +
> > +       "total_scanned_pages_by_shrink",
> > +       "total_freed_pages_by_shrink",
> > +       "total_elapsed_ns_by_shrink",
> > +};
> > +
> >  /*
> >  * The memory controller data structure. The memory controller controls
> > both
> >  * page cache and RSS per cgroup. We would eventually like to provide
> > @@ -264,7 +315,8 @@ struct mem_cgroup {
> >
> >        /* For oom notifier event fd */
> >        struct list_head oom_notify;
> > -
> > +       /* For recording LRU-scan statistics */
> > +       struct scanstat scanstat;
> >        /*
> >         * Should we move charges of a task when a task is moved into this
> >         * mem_cgroup ? And what type of charges should we move ?
> > @@ -1634,6 +1686,28 @@ int mem_cgroup_select_victim_node(struct
> >  }
> >  #endif
> >
> > +
> > +
> > +static void mem_cgroup_record_scanstat(struct memcg_scanrecord *rec)
> > +{
> > +       struct mem_cgroup *mem;
> > +       int context = rec->context;
> > +
> > +       mem = rec->mem;
> > +       spin_lock(&mem->scanstat.lock);
> > +       mem->scanstat.stats[context] += rec->nr_scanned;
> > +       mem->scanstat.stats[context + __FREED] += rec->nr_freed;
> > +       mem->scanstat.stats[context + __ELAPSED] += rec->elappsed;
> >
> elapsed?
> 
> 
> +       spin_unlock(&mem->scanstat.lock);
> > +
> > +       mem = rec->root;
> > +       spin_lock(&mem->scanstat.lock);
> > +       mem->scanstat.totalstats[context] += rec->nr_scanned;
> > +       mem->scanstat.totalstats[context + __FREED] += rec->nr_freed;
> > +       mem->scanstat.totalstats[context + __ELAPSED] += rec->elappsed;
> >
> 
> elapsed?
> 
> > +       spin_unlock(&mem->scanstat.lock);
> > +}
> > +
> >  /*
> >  * Scan the hierarchy if needed to reclaim memory. We remember the last
> > child
> >  * we reclaimed from, so that we don't end up penalizing one child
> > extensively
> > @@ -1659,8 +1733,8 @@ static int mem_cgroup_hierarchical_recla
> >        bool shrink = reclaim_options & MEM_CGROUP_RECLAIM_SHRINK;
> >        bool check_soft = reclaim_options & MEM_CGROUP_RECLAIM_SOFT;
> >        unsigned long excess;
> > -       unsigned long nr_scanned;
> >        int visit;
> > +       struct memcg_scanrecord rec;
> >
> >        excess = res_counter_soft_limit_excess(&root_mem->res) >>
> > PAGE_SHIFT;
> >
> > @@ -1668,6 +1742,15 @@ static int mem_cgroup_hierarchical_recla
> >        if (!check_soft && root_mem->memsw_is_minimum)
> >                noswap = true;
> >
> > +       if (shrink)
> > +               rec.context = SCAN_BY_SHRINK;
> > +       else if (check_soft)
> > +               rec.context = SCAN_BY_SYSTEM;
> > +       else
> > +               rec.context = SCAN_BY_LIMIT;
> > +
> > +       rec.root = root_mem;
> >
> 
> 
> 
> > +
> >  again:
> >        if (!shrink) {
> >                visit = 0;
> > @@ -1695,14 +1778,19 @@ again:
> >                        css_put(&victim->css);
> >                        continue;
> >                }
> > +               rec.mem = victim;
> > +               rec.nr_scanned = 0;
> > +               rec.nr_freed = 0;
> > +               rec.elappsed = 0;
> >
> elapsed?
> 
> >                /* we use swappiness of local cgroup */
> >                if (check_soft) {
> >                        ret = mem_cgroup_shrink_node_zone(victim, gfp_mask,
> > -                               noswap, zone, &nr_scanned);
> > -                       *total_scanned += nr_scanned;
> > +                               noswap, zone, &rec);
> > +                       *total_scanned += rec.nr_scanned;
> >                } else
> >                        ret = try_to_free_mem_cgroup_pages(victim, gfp_mask,
> > -                                               noswap);
> > +                                               noswap, &rec);
> > +               mem_cgroup_record_scanstat(&rec);
> >                css_put(&victim->css);
> >
> >                total += ret;
> > @@ -3757,7 +3845,8 @@ try_to_free:
> >                        ret = -EINTR;
> >                        goto out;
> >                }
> > -               progress = try_to_free_mem_cgroup_pages(mem, GFP_KERNEL,
> > false);
> > +               progress = try_to_free_mem_cgroup_pages(mem,
> > +                               GFP_KERNEL, false, NULL);
> >
> 
> So we don't record the stat for force_empty case?
> 

yes, now. force_empty is used only for rmdir(). I don't think log is
necessary for cgroup disappearing.


> 
> >                if (!progress) {
> >                        nr_retries--;
> >                        /* maybe some writeback is necessary */
> > @@ -4599,6 +4688,34 @@ static int mem_control_numa_stat_open(st
> >  }
> >  #endif /* CONFIG_NUMA */
> >
> > +static int mem_cgroup_scan_stat_read(struct cgroup *cgrp,
> > +                               struct cftype *cft,
> > +                               struct cgroup_map_cb *cb)
> > +{
> > +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
> > +       int i;
> > +
> > +       for (i = 0; i < NR_SCANSTATS; i++)
> > +               cb->fill(cb, scanstat_string[i], mem->scanstat.stats[i]);
> > +       for (i = 0; i < NR_SCANSTATS; i++)
> > +               cb->fill(cb, total_scanstat_string[i],
> > +                       mem->scanstat.totalstats[i]);
> > +       return 0;
> > +}
> > +
> > +static int mem_cgroup_reset_scan_stat(struct cgroup *cgrp,
> > +                               unsigned int event)
> > +{
> > +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
> > +
> > +       spin_lock(&mem->scanstat.lock);
> > +       memset(&mem->scanstat.stats, 0, sizeof(mem->scanstat.stats));
> > +       memset(&mem->scanstat.totalstats, 0,
> > sizeof(mem->scanstat.totalstats));
> > +       spin_unlock(&mem->scanstat.lock);
> > +       return 0;
> > +}
> > +
> > +
> >  static struct cftype mem_cgroup_files[] = {
> >        {
> >                .name = "usage_in_bytes",
> > @@ -4669,6 +4786,11 @@ static struct cftype mem_cgroup_files[]
> >                .mode = S_IRUGO,
> >        },
> >  #endif
> > +       {
> > +               .name = "scan_stat",
> > +               .read_map = mem_cgroup_scan_stat_read,
> > +               .trigger = mem_cgroup_reset_scan_stat,
> > +       },
> >  };
> >
> >  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> > @@ -4932,6 +5054,7 @@ mem_cgroup_create(struct cgroup_subsys *
> >        atomic_set(&mem->refcnt, 1);
> >        mem->move_charge_at_immigrate = 0;
> >        mutex_init(&mem->thresholds_lock);
> > +       spin_lock_init(&mem->scanstat.lock);
> >        return &mem->css;
> >  free_out:
> >        __mem_cgroup_free(mem);
> > Index: mmotm-0615/mm/vmscan.c
> > ===================================================================
> > --- mmotm-0615.orig/mm/vmscan.c
> > +++ mmotm-0615/mm/vmscan.c
> > @@ -2199,9 +2199,9 @@ unsigned long try_to_free_pages(struct z
> >  #ifdef CONFIG_CGROUP_MEM_RES_CTLR
> >
> >  unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
> > -                                               gfp_t gfp_mask, bool
> > noswap,
> > -                                               struct zone *zone,
> > -                                               unsigned long *nr_scanned)
> > +                                       gfp_t gfp_mask, bool noswap,
> > +                                       struct zone *zone,
> > +                                       struct memcg_scanrecord *rec)
> >  {
> >        struct scan_control sc = {
> >                .nr_scanned = 0,
> > @@ -2213,6 +2213,7 @@ unsigned long mem_cgroup_shrink_node_zon
> >                .order = 0,
> >                .mem_cgroup = mem,
> >        };
> > +       unsigned long start, end;
> >
> >        sc.gfp_mask = (gfp_mask & GFP_RECLAIM_MASK) |
> >                        (GFP_HIGHUSER_MOVABLE & ~GFP_RECLAIM_MASK);
> > @@ -2221,6 +2222,7 @@ unsigned long mem_cgroup_shrink_node_zon
> >                                                      sc.may_writepage,
> >                                                      sc.gfp_mask);
> >
> > +       start = sched_clock();
> >        /*
> >         * NOTE: Although we can get the priority field, using it
> >         * here is not a good idea, since it limits the pages we can scan.
> > @@ -2229,19 +2231,27 @@ unsigned long mem_cgroup_shrink_node_zon
> >         * the priority and make it zero.
> >         */
> >        shrink_zone(0, zone, &sc);
> > +       end = sched_clock();
> > +
> > +       if (rec) {
> > +               rec->nr_scanned += sc.nr_scanned;
> >
> 
> 
> 
> > +               rec->nr_freed += sc.nr_reclaimed;
> > +               rec->elappsed += end - start;
> >
> elapsed?
> 
> > +       }
> >
> >        trace_mm_vmscan_memcg_softlimit_reclaim_end(sc.nr_reclaimed);
> >
> > -       *nr_scanned = sc.nr_scanned;
> >        return sc.nr_reclaimed;
> >  }
> >
> >  unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem_cont,
> >                                           gfp_t gfp_mask,
> > -                                          bool noswap)
> > +                                          bool noswap,
> > +                                          struct memcg_scanrecord *rec)
> >  {
> >        struct zonelist *zonelist;
> >        unsigned long nr_reclaimed;
> > +       unsigned long start, end;
> >        int nid;
> >        struct scan_control sc = {
> >                .may_writepage = !laptop_mode,
> > @@ -2259,6 +2269,7 @@ unsigned long try_to_free_mem_cgroup_pag
> >                .gfp_mask = sc.gfp_mask,
> >        };
> >
> > +       start = sched_clock();
> >        /*
> >         * Unlike direct reclaim via alloc_pages(), memcg's reclaim doesn't
> >         * take care of from where we get pages. So the node where we start
> > the
> > @@ -2273,6 +2284,12 @@ unsigned long try_to_free_mem_cgroup_pag
> >                                            sc.gfp_mask);
> >
> >        nr_reclaimed = do_try_to_free_pages(zonelist, &sc, &shrink);
> > +       end = sched_clock();
> > +       if (rec) {
> > +               rec->nr_scanned += sc.nr_scanned;
> >
> 
> sc.nr_scanned only contains the nr_scanned of last
> priority do_try_to_free_pages(). we need to reset it to total_scanned before
> return. I am looking at v3.0-rc3 .
> 

Hm. ok, then, total_reclaimed in softlimit is buggy, too.
I'll check.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/7] memcg: add memory.scan_stat

[Ying Han]

On Tue, Jun 21, 2011 at 5:20 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Mon, 20 Jun 2011 23:49:54 -0700
> Ying Han <yinghan@google.com> wrote:
>
>> On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki <
>> kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>
>> > From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
>> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> > Date: Wed, 15 Jun 2011 14:11:01 +0900
>> > Subject: [PATCH 3/7] memcg: add memory.scan_stat
>> >
>> > commit log of commit 0ae5e89 " memcg: count the soft_limit reclaim in..."
>> > says it adds scanning stats to memory.stat file. But it doesn't because
>> > we considered we needed to make a concensus for such new APIs.
>> >
>> > This patch is a trial to add memory.scan_stat. This shows
>> >  - the number of scanned pages
>> >  - the number of recleimed pages
>> >  - the number of elaplsed time (including sleep/pause time)
>> >  for both of direct/soft reclaim and shrinking caused by changing limit
>> >  or force_empty.
>> >
>> > The biggest difference with oringinal Ying's one is that this file
>> > can be reset by some write, as
>> >
>> >  # echo 0 ...../memory.scan_stat
>> >
>> > [kamezawa@bluextal ~]$ cat /cgroup/memory/A/memory.scan_stat
>> > scanned_pages_by_limit 358470
>> > freed_pages_by_limit 180795
>> > elapsed_ns_by_limit 21629927
>> > scanned_pages_by_system 0
>> > freed_pages_by_system 0
>> > elapsed_ns_by_system 0
>> > scanned_pages_by_shrink 76646
>> > freed_pages_by_shrink 38355
>> > elappsed_ns_by_shrink 31990670
>> >
>>
>> elapsed?
>>
>
> you'r right.
>
>>
>> > total_scanned_pages_by_limit 358470
>> > total_freed_pages_by_limit 180795
>> > total_elapsed_ns_by_hierarchical 216299275
>> > total_scanned_pages_by_system 0
>> > total_freed_pages_by_system 0
>> > total_elapsed_ns_by_system 0
>> > total_scanned_pages_by_shrink 76646
>> > total_freed_pages_by_shrink 38355
>> > total_elapsed_ns_by_shrink 31990670
>> >
>> > total_xxxx is for hierarchy management.
>> >
>>
>> For some reason, i feel the opposite where the local stat (like
>> "scanned_pages_by_limit") are reclaimed under hierarchical reclaim. The
>> total_xxx stats are only incremented for root_mem which is the cgroup
>> triggers the hierarchical reclaim. So:
>>
>> total_scanned_pages_by_limit: number of pages being scanned while the memcg
>> hits its limit
>> scanned_pages_by_limit: number of pages being scanned while one of the
>> memcg's ancestor hits its limit
>>
>> am i missing something?
>>
>
> scanned_pages_by_limit: one of ancestors and itself's limit.

yes, and that is what I understood.
>
>
>
>
>>
>> >
>> > This will be useful for further memcg developments and need to be
>> > developped before we do some complicated rework on LRU/softlimit
>> > management.
>> >
>> > Now, scan/free/elapsed_by_system is incomplete but future works of
>> > Johannes at el. will fill remaining information and then, we can
>> > look into problems of isolation between memcgs.
>> >
>> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> > ---
>> >  Documentation/cgroups/memory.txt |   33 +++++++++
>> >  include/linux/memcontrol.h       |   16 ++++
>> >  include/linux/swap.h             |    6 -
>> >  mm/memcontrol.c                  |  135
>> > +++++++++++++++++++++++++++++++++++++--
>> >  mm/vmscan.c                      |   27 ++++++-
>> >  5 files changed, 199 insertions(+), 18 deletions(-)
>> >
>> > Index: mmotm-0615/Documentation/cgroups/memory.txt
>> > ===================================================================
>> > --- mmotm-0615.orig/Documentation/cgroups/memory.txt
>> > +++ mmotm-0615/Documentation/cgroups/memory.txt
>> > @@ -380,7 +380,7 @@ will be charged as a new owner of it.
>> >
>> >  5.2 stat file
>> >
>> > -memory.stat file includes following statistics
>> > +5.2.1 memory.stat file includes following statistics
>> >
>> >  # per-memory cgroup local status
>> >  cache          - # of bytes of page cache memory.
>> > @@ -438,6 +438,37 @@ Note:
>> >         file_mapped is accounted only when the memory cgroup is owner of
>> > page
>> >         cache.)
>> >
>> > +5.2.2 memory.scan_stat
>> > +
>> > +memory.scan_stat includes statistics information for memory scanning and
>> > +freeing, reclaiming. The statistics shows memory scanning information
>> > since
>> > +memory cgroup creation and can be reset to 0 by writing 0 as
>> > +
>> > + #echo 0 > ../memory.scan_stat
>> > +
>> > +This file contains following statistics.
>> > +
>> > +scanned_pages_by_limit - # of scanned pages at hitting limit.
>> > +freed_pages_by_limit   - # of freed pages at hitting limit.
>> > +elapsed_ns_by_limit    - nano sec of elappsed time at LRU scan at
>> > +                                  hitting limit.(this includes sleep
>> > time.)
>> >
>> elapsed?
>>
>> > +
>> > +scanned_pages_by_system        - # of scanned pages by the kernel.
>> > +                         (Now, this value means global memory reclaim
>> > +                           caused by system memory shortage with a hint
>> > +                          of softlimit. "No soft limit" case will be
>> > +                          supported in future.)
>> > +freed_pages_by_system  - # of freed pages by the kernel.
>> > +elapsed_ns_by_system   - nano sec of elappsed time by kernel.
>> > +
>> > +scanned_pages_by_shrink        - # of scanned pages by shrinking.
>> > +                                 (i.e. changes of limit, force_empty,
>> > etc.)
>> > +freed_pages_by_shrink  - # of freed pages by shirkining.
>> > +elappsed_ns_by_shrink  - nano sec of elappsed time at shrinking.
>> >
>> elapsed?
>>
>> > +
>> > +total_xxx includes the statistics of children scanning caused by the
>> > cgroup.
>> >
>>
>> based on the code inspection, the total_xxx also includes the cgroup's scan
>> stat as well.
>>
>
> yes.
>
>
>> +
>> > +
>> >  5.3 swappiness
>> >
>> >  Similar to /proc/sys/vm/swappiness, but affecting a hierarchy of groups
>> > only.
>> > Index: mmotm-0615/include/linux/memcontrol.h
>> > ===================================================================
>> > --- mmotm-0615.orig/include/linux/memcontrol.h
>> > +++ mmotm-0615/include/linux/memcontrol.h
>> > @@ -120,6 +120,22 @@ struct zone_reclaim_stat*
>> >  mem_cgroup_get_reclaim_stat_from_page(struct page *page);
>> >  extern void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
>> >                                        struct task_struct *p);
>> > +struct memcg_scanrecord {
>> > +       struct mem_cgroup *mem; /* scanend memory cgroup */
>> > +       struct mem_cgroup *root; /* scan target hierarchy root */
>> > +       int context;            /* scanning context (see memcontrol.c) */
>> > +       unsigned long nr_scanned; /* the number of scanned pages */
>> > +       unsigned long nr_freed; /* the number of freed pages */
>> > +       unsigned long elappsed; /* nsec of time elapsed while scanning */
>> >
>> elapsed?
>>
>> > +};
>> > +
>> > +extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
>> > +                                                 gfp_t gfp_mask, bool
>> > noswap,
>> > +                                                 struct memcg_scanrecord
>> > *rec);
>> > +extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
>> > +                                               gfp_t gfp_mask, bool
>> > noswap,
>> > +                                               struct zone *zone,
>> > +                                               struct memcg_scanrecord
>> > *rec);
>> >
>> >  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>> >  extern int do_swap_account;
>> > Index: mmotm-0615/include/linux/swap.h
>> > ===================================================================
>> > --- mmotm-0615.orig/include/linux/swap.h
>> > +++ mmotm-0615/include/linux/swap.h
>> > @@ -253,12 +253,6 @@ static inline void lru_cache_add_file(st
>> >  /* linux/mm/vmscan.c */
>> >  extern unsigned long try_to_free_pages(struct zonelist *zonelist, int
>> > order,
>> >                                        gfp_t gfp_mask, nodemask_t *mask);
>> > -extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
>> > -                                                 gfp_t gfp_mask, bool
>> > noswap);
>> > -extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
>> > -                                               gfp_t gfp_mask, bool
>> > noswap,
>> > -                                               struct zone *zone,
>> > -                                               unsigned long *nr_scanned);
>> >  extern int __isolate_lru_page(struct page *page, int mode, int file);
>> >  extern unsigned long shrink_all_memory(unsigned long nr_pages);
>> >  extern int vm_swappiness;
>> > Index: mmotm-0615/mm/memcontrol.c
>> > ===================================================================
>> > --- mmotm-0615.orig/mm/memcontrol.c
>> > +++ mmotm-0615/mm/memcontrol.c
>> > @@ -203,6 +203,57 @@ struct mem_cgroup_eventfd_list {
>> >  static void mem_cgroup_threshold(struct mem_cgroup *mem);
>> >  static void mem_cgroup_oom_notify(struct mem_cgroup *mem);
>> >
>> > +enum {
>> > +       SCAN_BY_LIMIT,
>> > +       FREED_BY_LIMIT,
>> > +       ELAPSED_BY_LIMIT,
>> > +
>> > +       SCAN_BY_SYSTEM,
>> > +       FREED_BY_SYSTEM,
>> > +       ELAPSED_BY_SYSTEM,
>> > +
>> > +       SCAN_BY_SHRINK,
>> > +       FREED_BY_SHRINK,
>> > +       ELAPSED_BY_SHRINK,
>> > +       NR_SCANSTATS,
>> > +};
>> > +#define __FREED                (1)
>> > +#define        __ELAPSED       (2)
>> >
>>
>> /tab/space/
>>
>>
>> > +
>> > +struct scanstat {
>> > +       spinlock_t      lock;
>> > +       unsigned long   stats[NR_SCANSTATS];    /* local statistics */
>> > +       unsigned long   totalstats[NR_SCANSTATS];   /* hierarchical */
>> > +};

I wonder why not extending the mem_cgroup_stat_cpu struct, and then we
can use the per-cpu counters like others.

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b7d2d79..5b8bbe9 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -138,6 +138,7 @@ struct mem_cgroup_stat_cpu {
        long count[MEM_CGROUP_STAT_NSTATS];
        unsigned long events[MEM_CGROUP_EVENTS_NSTATS];
        unsigned long targets[MEM_CGROUP_NTARGETS];
+       unsigned long reclaim_stats[MEMCG_RECLAIM_NSTATS];
 };

 /*
@@ -557,6 +558,101 @@ static void __mem_cgroup_target_update(struct
mem_cgroup *mem, int target)
        this_cpu_write(mem->stat->targets[target], next);
 }

+void mem_cgroup_update_reclaim_stat(struct mem_cgroup *mem,
+                                   enum mem_cgroup_reclaim_stat_index idx,
+                                   unsigned long num)
+{
+       this_cpu_add(mem->stat->reclaim_stats[idx], num);
+}


>> > +
>> > +const char *scanstat_string[NR_SCANSTATS] = {
>> > +       "scanned_pages_by_limit",
>> > +       "freed_pages_by_limit",
>> > +       "elapsed_ns_by_limit",
>> > +
>> > +       "scanned_pages_by_system",
>> > +       "freed_pages_by_system",
>> > +       "elapsed_ns_by_system",
>> > +
>> > +       "scanned_pages_by_shrink",
>> > +       "freed_pages_by_shrink",
>> > +       "elappsed_ns_by_shrink",
>> >
>> elapsed?
>>
>> > +};
>> > +
>> > +const char *total_scanstat_string[NR_SCANSTATS] = {
>> > +       "total_scanned_pages_by_limit",
>> > +       "total_freed_pages_by_limit",
>> > +       "total_elapsed_ns_by_hierarchical",
>> >
>>
>> typo?
>>
>>
>> > +
>> > +       "total_scanned_pages_by_system",
>> > +       "total_freed_pages_by_system",
>> > +       "total_elapsed_ns_by_system",
>> > +
>> > +       "total_scanned_pages_by_shrink",
>> > +       "total_freed_pages_by_shrink",
>> > +       "total_elapsed_ns_by_shrink",
>> > +};
>> > +
>> >  /*
>> >  * The memory controller data structure. The memory controller controls
>> > both
>> >  * page cache and RSS per cgroup. We would eventually like to provide
>> > @@ -264,7 +315,8 @@ struct mem_cgroup {
>> >
>> >        /* For oom notifier event fd */
>> >        struct list_head oom_notify;
>> > -
>> > +       /* For recording LRU-scan statistics */
>> > +       struct scanstat scanstat;

Can we add this to mem_cgroup_stat_cpu ?

>> >        /*
>> >         * Should we move charges of a task when a task is moved into this
>> >         * mem_cgroup ? And what type of charges should we move ?
>> > @@ -1634,6 +1686,28 @@ int mem_cgroup_select_victim_node(struct
>> >  }
>> >  #endif
>> >
>> > +
>> > +
>> > +static void mem_cgroup_record_scanstat(struct memcg_scanrecord *rec)
>> > +{
>> > +       struct mem_cgroup *mem;
>> > +       int context = rec->context;
>> > +
>> > +       mem = rec->mem;
>> > +       spin_lock(&mem->scanstat.lock);
>> > +       mem->scanstat.stats[context] += rec->nr_scanned;
>> > +       mem->scanstat.stats[context + __FREED] += rec->nr_freed;
>> > +       mem->scanstat.stats[context + __ELAPSED] += rec->elappsed;
>> >
>> elapsed?
>>
>>
>> +       spin_unlock(&mem->scanstat.lock);
>> > +
>> > +       mem = rec->root;
>> > +       spin_lock(&mem->scanstat.lock);
>> > +       mem->scanstat.totalstats[context] += rec->nr_scanned;
>> > +       mem->scanstat.totalstats[context + __FREED] += rec->nr_freed;
>> > +       mem->scanstat.totalstats[context + __ELAPSED] += rec->elappsed;
>> >
>>
>> elapsed?
>>
>> > +       spin_unlock(&mem->scanstat.lock);
>> > +}
>> > +
>> >  /*
>> >  * Scan the hierarchy if needed to reclaim memory. We remember the last
>> > child
>> >  * we reclaimed from, so that we don't end up penalizing one child
>> > extensively
>> > @@ -1659,8 +1733,8 @@ static int mem_cgroup_hierarchical_recla
>> >        bool shrink = reclaim_options & MEM_CGROUP_RECLAIM_SHRINK;
>> >        bool check_soft = reclaim_options & MEM_CGROUP_RECLAIM_SOFT;
>> >        unsigned long excess;
>> > -       unsigned long nr_scanned;
>> >        int visit;
>> > +       struct memcg_scanrecord rec;
>> >
>> >        excess = res_counter_soft_limit_excess(&root_mem->res) >>
>> > PAGE_SHIFT;
>> >
>> > @@ -1668,6 +1742,15 @@ static int mem_cgroup_hierarchical_recla
>> >        if (!check_soft && root_mem->memsw_is_minimum)
>> >                noswap = true;
>> >
>> > +       if (shrink)
>> > +               rec.context = SCAN_BY_SHRINK;
>> > +       else if (check_soft)
>> > +               rec.context = SCAN_BY_SYSTEM;
>> > +       else
>> > +               rec.context = SCAN_BY_LIMIT;
>> > +
>> > +       rec.root = root_mem;
>> >
>>
>>
>>
>> > +
>> >  again:
>> >        if (!shrink) {
>> >                visit = 0;
>> > @@ -1695,14 +1778,19 @@ again:
>> >                        css_put(&victim->css);
>> >                        continue;
>> >                }
>> > +               rec.mem = victim;
>> > +               rec.nr_scanned = 0;
>> > +               rec.nr_freed = 0;
>> > +               rec.elappsed = 0;
>> >
>> elapsed?
>>
>> >                /* we use swappiness of local cgroup */
>> >                if (check_soft) {
>> >                        ret = mem_cgroup_shrink_node_zone(victim, gfp_mask,
>> > -                               noswap, zone, &nr_scanned);
>> > -                       *total_scanned += nr_scanned;
>> > +                               noswap, zone, &rec);
>> > +                       *total_scanned += rec.nr_scanned;
>> >                } else
>> >                        ret = try_to_free_mem_cgroup_pages(victim, gfp_mask,
>> > -                                               noswap);
>> > +                                               noswap, &rec);
>> > +               mem_cgroup_record_scanstat(&rec);
>> >                css_put(&victim->css);
>> >
>> >                total += ret;
>> > @@ -3757,7 +3845,8 @@ try_to_free:
>> >                        ret = -EINTR;
>> >                        goto out;
>> >                }
>> > -               progress = try_to_free_mem_cgroup_pages(mem, GFP_KERNEL,
>> > false);
>> > +               progress = try_to_free_mem_cgroup_pages(mem,
>> > +                               GFP_KERNEL, false, NULL);
>> >
>>
>> So we don't record the stat for force_empty case?
>>
>
> yes, now. force_empty is used only for rmdir(). I don't think log is
> necessary for cgroup disappearing.

we might need to add that case later, since force_empty can also being
used as adding external pressure to the memcg.
>
>
>>
>> >                if (!progress) {
>> >                        nr_retries--;
>> >                        /* maybe some writeback is necessary */
>> > @@ -4599,6 +4688,34 @@ static int mem_control_numa_stat_open(st
>> >  }
>> >  #endif /* CONFIG_NUMA */
>> >
>> > +static int mem_cgroup_scan_stat_read(struct cgroup *cgrp,
>> > +                               struct cftype *cft,
>> > +                               struct cgroup_map_cb *cb)
>> > +{
>> > +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
>> > +       int i;
>> > +
>> > +       for (i = 0; i < NR_SCANSTATS; i++)
>> > +               cb->fill(cb, scanstat_string[i], mem->scanstat.stats[i]);
>> > +       for (i = 0; i < NR_SCANSTATS; i++)
>> > +               cb->fill(cb, total_scanstat_string[i],
>> > +                       mem->scanstat.totalstats[i]);
>> > +       return 0;
>> > +}
>> > +
>> > +static int mem_cgroup_reset_scan_stat(struct cgroup *cgrp,
>> > +                               unsigned int event)
>> > +{
>> > +       struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
>> > +
>> > +       spin_lock(&mem->scanstat.lock);
>> > +       memset(&mem->scanstat.stats, 0, sizeof(mem->scanstat.stats));
>> > +       memset(&mem->scanstat.totalstats, 0,
>> > sizeof(mem->scanstat.totalstats));
>> > +       spin_unlock(&mem->scanstat.lock);
>> > +       return 0;
>> > +}
>> > +
>> > +
>> >  static struct cftype mem_cgroup_files[] = {
>> >        {
>> >                .name = "usage_in_bytes",
>> > @@ -4669,6 +4786,11 @@ static struct cftype mem_cgroup_files[]
>> >                .mode = S_IRUGO,
>> >        },
>> >  #endif
>> > +       {
>> > +               .name = "scan_stat",
>> > +               .read_map = mem_cgroup_scan_stat_read,
>> > +               .trigger = mem_cgroup_reset_scan_stat,
>> > +       },
>> >  };
>> >
>> >  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>> > @@ -4932,6 +5054,7 @@ mem_cgroup_create(struct cgroup_subsys *
>> >        atomic_set(&mem->refcnt, 1);
>> >        mem->move_charge_at_immigrate = 0;
>> >        mutex_init(&mem->thresholds_lock);
>> > +       spin_lock_init(&mem->scanstat.lock);
>> >        return &mem->css;
>> >  free_out:
>> >        __mem_cgroup_free(mem);
>> > Index: mmotm-0615/mm/vmscan.c
>> > ===================================================================
>> > --- mmotm-0615.orig/mm/vmscan.c
>> > +++ mmotm-0615/mm/vmscan.c
>> > @@ -2199,9 +2199,9 @@ unsigned long try_to_free_pages(struct z
>> >  #ifdef CONFIG_CGROUP_MEM_RES_CTLR
>> >
>> >  unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
>> > -                                               gfp_t gfp_mask, bool
>> > noswap,
>> > -                                               struct zone *zone,
>> > -                                               unsigned long *nr_scanned)
>> > +                                       gfp_t gfp_mask, bool noswap,
>> > +                                       struct zone *zone,
>> > +                                       struct memcg_scanrecord *rec)
>> >  {
>> >        struct scan_control sc = {
>> >                .nr_scanned = 0,
>> > @@ -2213,6 +2213,7 @@ unsigned long mem_cgroup_shrink_node_zon
>> >                .order = 0,
>> >                .mem_cgroup = mem,
>> >        };
>> > +       unsigned long start, end;
>> >
>> >        sc.gfp_mask = (gfp_mask & GFP_RECLAIM_MASK) |
>> >                        (GFP_HIGHUSER_MOVABLE & ~GFP_RECLAIM_MASK);
>> > @@ -2221,6 +2222,7 @@ unsigned long mem_cgroup_shrink_node_zon
>> >                                                      sc.may_writepage,
>> >                                                      sc.gfp_mask);
>> >
>> > +       start = sched_clock();
>> >        /*
>> >         * NOTE: Although we can get the priority field, using it
>> >         * here is not a good idea, since it limits the pages we can scan.
>> > @@ -2229,19 +2231,27 @@ unsigned long mem_cgroup_shrink_node_zon
>> >         * the priority and make it zero.
>> >         */
>> >        shrink_zone(0, zone, &sc);
>> > +       end = sched_clock();
>> > +
>> > +       if (rec) {
>> > +               rec->nr_scanned += sc.nr_scanned;
>> >
>>
>>
>>
>> > +               rec->nr_freed += sc.nr_reclaimed;
>> > +               rec->elappsed += end - start;
>> >
>> elapsed?
>>
>> > +       }
>> >
>> >        trace_mm_vmscan_memcg_softlimit_reclaim_end(sc.nr_reclaimed);
>> >
>> > -       *nr_scanned = sc.nr_scanned;
>> >        return sc.nr_reclaimed;
>> >  }
>> >
>> >  unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem_cont,
>> >                                           gfp_t gfp_mask,
>> > -                                          bool noswap)
>> > +                                          bool noswap,
>> > +                                          struct memcg_scanrecord *rec)
>> >  {
>> >        struct zonelist *zonelist;
>> >        unsigned long nr_reclaimed;
>> > +       unsigned long start, end;
>> >        int nid;
>> >        struct scan_control sc = {
>> >                .may_writepage = !laptop_mode,
>> > @@ -2259,6 +2269,7 @@ unsigned long try_to_free_mem_cgroup_pag
>> >                .gfp_mask = sc.gfp_mask,
>> >        };
>> >
>> > +       start = sched_clock();
>> >        /*
>> >         * Unlike direct reclaim via alloc_pages(), memcg's reclaim doesn't
>> >         * take care of from where we get pages. So the node where we start
>> > the
>> > @@ -2273,6 +2284,12 @@ unsigned long try_to_free_mem_cgroup_pag
>> >                                            sc.gfp_mask);
>> >
>> >        nr_reclaimed = do_try_to_free_pages(zonelist, &sc, &shrink);
>> > +       end = sched_clock();
>> > +       if (rec) {
>> > +               rec->nr_scanned += sc.nr_scanned;
>> >
>>
>> sc.nr_scanned only contains the nr_scanned of last
>> priority do_try_to_free_pages(). we need to reset it to total_scanned before
>> return. I am looking at v3.0-rc3 .
>>
>
> Hm. ok, then, total_reclaimed in softlimit is buggy, too.
> I'll check.

Thanks

--Ying
>
> Thanks,
> -Kame
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/7] memcg: add memory.scan_stat

[KAMEZAWA Hiroyuki]

On Fri, 24 Jun 2011 14:40:42 -0700
Ying Han <yinghan@google.com> wrote:

> On Tue, Jun 21, 2011 at 5:20 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > On Mon, 20 Jun 2011 23:49:54 -0700
> > Ying Han <yinghan@google.com> wrote:
> >
> >> On Wed, Jun 15, 2011 at 8:53 PM, KAMEZAWA Hiroyuki <
> >> kamezawa.hiroyu@jp.fujitsu.com> wrote:
> >>
> >> > From e08990dd9ada13cf236bec1ef44b207436434b8e Mon Sep 17 00:00:00 2001
> >> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> >> > Date: Wed, 15 Jun 2011 14:11:01 +0900
> >> > Subject: [PATCH 3/7] memcg: add memory.scan_stat

> >> > +
> >> > +struct scanstat {
> >> > + A  A  A  spinlock_t A  A  A lock;
> >> > + A  A  A  unsigned long A  stats[NR_SCANSTATS]; A  A /* local statistics */
> >> > + A  A  A  unsigned long A  totalstats[NR_SCANSTATS]; A  /* hierarchical */
> >> > +};
> 
> I wonder why not extending the mem_cgroup_stat_cpu struct, and then we
> can use the per-cpu counters like others.
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index b7d2d79..5b8bbe9 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -138,6 +138,7 @@ struct mem_cgroup_stat_cpu {
>         long count[MEM_CGROUP_STAT_NSTATS];
>         unsigned long events[MEM_CGROUP_EVENTS_NSTATS];
>         unsigned long targets[MEM_CGROUP_NTARGETS];
> +       unsigned long reclaim_stats[MEMCG_RECLAIM_NSTATS];
>  };
> 

Hmm, do we have enough benefit to consume 72 bytes per cpu and make
read-side slow for this rarely updated counter ?

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 4/7] memcg: update numa information based on event counter

[KAMEZAWA Hiroyuki]

Re: [PATCH 4/7] memcg: update numa information based on event counter

[Michal Hocko]

On Thu 16-06-11 12:54:00, KAMEZAWA Hiroyuki wrote:
> From 88090fe10e225ad8769ba0ea01692b7314e8b973 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Wed, 15 Jun 2011 16:19:46 +0900
> Subject: [PATCH 4/7] memcg: update numa information based on event counter
> 
> commit 889976 adds an numa node round-robin for memcg. But the information
> is updated once per 10sec.
> 
> This patch changes the update trigger from jiffies to memcg's event count.
> After this patch, numa scan information will be updated when
> 
>   - the number of pagein/out events is larger than 3% of limit
>   or
>   - the number of pagein/out events is larger than 16k
>     (==64MB pagein/pageout if pagesize==4k.)
> 
> The counter of mem->numascan_update the sum of percpu events counter.
> When a task hits limit, it checks mem->numascan_update. If it's over
> min(3% of limit, 16k), numa information will be updated.

Yes, I like the event based approach more than the origin (time) based
one.

> 
> This patch also adds mutex for updating information. This will allow us
> to avoid unnecessary scan.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/memcontrol.c |   51 +++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 45 insertions(+), 6 deletions(-)
> 
> Index: mmotm-0615/mm/memcontrol.c
> ===================================================================
> --- mmotm-0615.orig/mm/memcontrol.c
> +++ mmotm-0615/mm/memcontrol.c
> @@ -108,10 +108,12 @@ enum mem_cgroup_events_index {
>  enum mem_cgroup_events_target {
>  	MEM_CGROUP_TARGET_THRESH,
>  	MEM_CGROUP_TARGET_SOFTLIMIT,
> +	MEM_CGROUP_TARGET_NUMASCAN,

Shouldn't it be defined only for MAX_NUMNODES > 1

>  	MEM_CGROUP_NTARGETS,
>  };
>  #define THRESHOLDS_EVENTS_TARGET (128)
>  #define SOFTLIMIT_EVENTS_TARGET (1024)
> +#define NUMASCAN_EVENTS_TARGET  (1024)
>  
>  struct mem_cgroup_stat_cpu {
>  	long count[MEM_CGROUP_STAT_NSTATS];
> @@ -288,8 +290,9 @@ struct mem_cgroup {
>  	int last_scanned_node;
>  #if MAX_NUMNODES > 1
>  	nodemask_t	scan_nodes;
> -	unsigned long   next_scan_node_update;
> +	struct mutex	numascan_mutex;
>  #endif
> +	atomic_t	numascan_update;

Why it is out of ifdef?

>  	/*
>  	 * Should the accounting and control be hierarchical, per subtree?
>  	 */
> @@ -741,6 +744,9 @@ static void __mem_cgroup_target_update(s
>  	case MEM_CGROUP_TARGET_SOFTLIMIT:
>  		next = val + SOFTLIMIT_EVENTS_TARGET;
>  		break;
> +	case MEM_CGROUP_TARGET_NUMASCAN:
> +		next = val + NUMASCAN_EVENTS_TARGET;
> +		break;

MAX_NUMNODES > 1

>  	default:
>  		return;
>  	}
> @@ -764,6 +770,13 @@ static void memcg_check_events(struct me
>  			__mem_cgroup_target_update(mem,
>  				MEM_CGROUP_TARGET_SOFTLIMIT);
>  		}
> +		if (unlikely(__memcg_event_check(mem,
> +			MEM_CGROUP_TARGET_NUMASCAN))) {
> +			atomic_add(MEM_CGROUP_TARGET_NUMASCAN,
> +				&mem->numascan_update);
> +			__mem_cgroup_target_update(mem,
> +				MEM_CGROUP_TARGET_NUMASCAN);
> +		}
>  	}

again MAX_NUMNODES > 1

>  }
>  
> @@ -1616,17 +1629,32 @@ mem_cgroup_select_victim(struct mem_cgro
>  /*
>   * Always updating the nodemask is not very good - even if we have an empty
>   * list or the wrong list here, we can start from some node and traverse all
> - * nodes based on the zonelist. So update the list loosely once per 10 secs.
> + * nodes based on the zonelist.
>   *
> + * The counter of mem->numascan_update is updated once per
> + * NUMASCAN_EVENTS_TARGET. We update the numa information when we see
> + * the number of event is larger than 3% of limit or  64MB pagein/pageout.
>   */
> +#define NUMASCAN_UPDATE_RATIO	(3)
> +#define NUMASCAN_UPDATE_THRESH	(16384UL) /* 16k events of pagein/pageout */
>  static void mem_cgroup_may_update_nodemask(struct mem_cgroup *mem)
>  {
>  	int nid;
> -
> -	if (time_after(mem->next_scan_node_update, jiffies))
> +	unsigned long long limit;
> +	/* if no limit, we never reach here */
> +	limit = res_counter_read_u64(&mem->res, RES_LIMIT);
> +	limit /= PAGE_SIZE;
> +	/* 3% of limit */
> +	limit = (limit * NUMASCAN_UPDATE_RATIO/100UL);
> +	limit = min_t(unsigned long long, limit, NUMASCAN_UPDATE_THRESH);
> +	/*
> +	 * If the number of pagein/out event is larger than 3% of limit or
> +	 * 64MB pagein/out, refresh numa information.
> +	 */
> +	if (atomic_read(&mem->numascan_update) < limit ||
> +	    !mutex_trylock(&mem->numascan_mutex))
>  		return;

I am not sure whether a mutex is not overkill here. What about using an
atomic operation instead?

> -
> -	mem->next_scan_node_update = jiffies + 10*HZ;
> +	atomic_set(&mem->numascan_update, 0);
>  	/* make a nodemask where this memcg uses memory from */
>  	mem->scan_nodes = node_states[N_HIGH_MEMORY];
>  
> @@ -1642,6 +1670,7 @@ static void mem_cgroup_may_update_nodema
>  			continue;
>  		node_clear(nid, mem->scan_nodes);
>  	}
> +	mutex_unlock(&mem->numascan_mutex);
>  }

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/7] memcg: update numa information based on event counter

[KAMEZAWA Hiroyuki]

On Wed, 22 Jun 2011 17:53:09 +0200
Michal Hocko <mhocko@suse.cz> wrote:

> On Thu 16-06-11 12:54:00, KAMEZAWA Hiroyuki wrote:
> > From 88090fe10e225ad8769ba0ea01692b7314e8b973 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Wed, 15 Jun 2011 16:19:46 +0900
> > Subject: [PATCH 4/7] memcg: update numa information based on event counter
> > 
> > commit 889976 adds an numa node round-robin for memcg. But the information
> > is updated once per 10sec.
> > 
> > This patch changes the update trigger from jiffies to memcg's event count.
> > After this patch, numa scan information will be updated when
> > 
> >   - the number of pagein/out events is larger than 3% of limit
> >   or
> >   - the number of pagein/out events is larger than 16k
> >     (==64MB pagein/pageout if pagesize==4k.)
> > 
> > The counter of mem->numascan_update the sum of percpu events counter.
> > When a task hits limit, it checks mem->numascan_update. If it's over
> > min(3% of limit, 16k), numa information will be updated.
> 
> Yes, I like the event based approach more than the origin (time) based
> one.
> 
> > 
> > This patch also adds mutex for updating information. This will allow us
> > to avoid unnecessary scan.
> > 
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> >  mm/memcontrol.c |   51 +++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 45 insertions(+), 6 deletions(-)
> > 
> > Index: mmotm-0615/mm/memcontrol.c
> > ===================================================================
> > --- mmotm-0615.orig/mm/memcontrol.c
> > +++ mmotm-0615/mm/memcontrol.c
> > @@ -108,10 +108,12 @@ enum mem_cgroup_events_index {
> >  enum mem_cgroup_events_target {
> >  	MEM_CGROUP_TARGET_THRESH,
> >  	MEM_CGROUP_TARGET_SOFTLIMIT,
> > +	MEM_CGROUP_TARGET_NUMASCAN,
> 
> Shouldn't it be defined only for MAX_NUMNODES > 1
> 

Hmm, yes. But I want to reduce #ifdefs..


> >  	MEM_CGROUP_NTARGETS,
> >  };
> >  #define THRESHOLDS_EVENTS_TARGET (128)
> >  #define SOFTLIMIT_EVENTS_TARGET (1024)
> > +#define NUMASCAN_EVENTS_TARGET  (1024)
> >  
> >  struct mem_cgroup_stat_cpu {
> >  	long count[MEM_CGROUP_STAT_NSTATS];
> > @@ -288,8 +290,9 @@ struct mem_cgroup {
> >  	int last_scanned_node;
> >  #if MAX_NUMNODES > 1
> >  	nodemask_t	scan_nodes;
> > -	unsigned long   next_scan_node_update;
> > +	struct mutex	numascan_mutex;
> >  #endif
> > +	atomic_t	numascan_update;
> 
> Why it is out of ifdef?
> 

This was for avoiding #ifdef in mem_cgroup_create()...but it's not used now.
I'll fix this.



> >  	/*
> >  	 * Should the accounting and control be hierarchical, per subtree?
> >  	 */
> > @@ -741,6 +744,9 @@ static void __mem_cgroup_target_update(s
> >  	case MEM_CGROUP_TARGET_SOFTLIMIT:
> >  		next = val + SOFTLIMIT_EVENTS_TARGET;
> >  		break;
> > +	case MEM_CGROUP_TARGET_NUMASCAN:
> > +		next = val + NUMASCAN_EVENTS_TARGET;
> > +		break;
> 
> MAX_NUMNODES > 1
> 
> >  	default:
> >  		return;
> >  	}
> > @@ -764,6 +770,13 @@ static void memcg_check_events(struct me
> >  			__mem_cgroup_target_update(mem,
> >  				MEM_CGROUP_TARGET_SOFTLIMIT);
> >  		}
> > +		if (unlikely(__memcg_event_check(mem,
> > +			MEM_CGROUP_TARGET_NUMASCAN))) {
> > +			atomic_add(MEM_CGROUP_TARGET_NUMASCAN,
> > +				&mem->numascan_update);
> > +			__mem_cgroup_target_update(mem,
> > +				MEM_CGROUP_TARGET_NUMASCAN);
> > +		}
> >  	}
> 
> again MAX_NUMNODES > 1
> 

Hmm, ok, I will add #ifdef only here.



> >  }
> >  
> > @@ -1616,17 +1629,32 @@ mem_cgroup_select_victim(struct mem_cgro
> >  /*
> >   * Always updating the nodemask is not very good - even if we have an empty
> >   * list or the wrong list here, we can start from some node and traverse all
> > - * nodes based on the zonelist. So update the list loosely once per 10 secs.
> > + * nodes based on the zonelist.
> >   *
> > + * The counter of mem->numascan_update is updated once per
> > + * NUMASCAN_EVENTS_TARGET. We update the numa information when we see
> > + * the number of event is larger than 3% of limit or  64MB pagein/pageout.
> >   */
> > +#define NUMASCAN_UPDATE_RATIO	(3)
> > +#define NUMASCAN_UPDATE_THRESH	(16384UL) /* 16k events of pagein/pageout */
> >  static void mem_cgroup_may_update_nodemask(struct mem_cgroup *mem)
> >  {
> >  	int nid;
> > -
> > -	if (time_after(mem->next_scan_node_update, jiffies))
> > +	unsigned long long limit;
> > +	/* if no limit, we never reach here */
> > +	limit = res_counter_read_u64(&mem->res, RES_LIMIT);
> > +	limit /= PAGE_SIZE;
> > +	/* 3% of limit */
> > +	limit = (limit * NUMASCAN_UPDATE_RATIO/100UL);
> > +	limit = min_t(unsigned long long, limit, NUMASCAN_UPDATE_THRESH);
> > +	/*
> > +	 * If the number of pagein/out event is larger than 3% of limit or
> > +	 * 64MB pagein/out, refresh numa information.
> > +	 */
> > +	if (atomic_read(&mem->numascan_update) < limit ||
> > +	    !mutex_trylock(&mem->numascan_mutex))
> >  		return;
> 
> I am not sure whether a mutex is not overkill here. What about using an
> atomic operation instead?
> 

I think mutex is informative than atomic counter for code readers.
If influence of overhead is not big, I'd like to use mutex.

Thanks,
-Kame


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/7] memcg: update numa information based on event counter

[Michal Hocko]

On Thu 23-06-11 15:27:34, KAMEZAWA Hiroyuki wrote:
> On Wed, 22 Jun 2011 17:53:09 +0200
> Michal Hocko <mhocko@suse.cz> wrote:
> 
> > On Thu 16-06-11 12:54:00, KAMEZAWA Hiroyuki wrote:
[...]
> > > @@ -1616,17 +1629,32 @@ mem_cgroup_select_victim(struct mem_cgro
> > >  /*
> > >   * Always updating the nodemask is not very good - even if we have an empty
> > >   * list or the wrong list here, we can start from some node and traverse all
> > > - * nodes based on the zonelist. So update the list loosely once per 10 secs.
> > > + * nodes based on the zonelist.
> > >   *
> > > + * The counter of mem->numascan_update is updated once per
> > > + * NUMASCAN_EVENTS_TARGET. We update the numa information when we see
> > > + * the number of event is larger than 3% of limit or  64MB pagein/pageout.
> > >   */
> > > +#define NUMASCAN_UPDATE_RATIO	(3)
> > > +#define NUMASCAN_UPDATE_THRESH	(16384UL) /* 16k events of pagein/pageout */
> > >  static void mem_cgroup_may_update_nodemask(struct mem_cgroup *mem)
> > >  {
> > >  	int nid;
> > > -
> > > -	if (time_after(mem->next_scan_node_update, jiffies))
> > > +	unsigned long long limit;
> > > +	/* if no limit, we never reach here */
> > > +	limit = res_counter_read_u64(&mem->res, RES_LIMIT);
> > > +	limit /= PAGE_SIZE;
> > > +	/* 3% of limit */
> > > +	limit = (limit * NUMASCAN_UPDATE_RATIO/100UL);
> > > +	limit = min_t(unsigned long long, limit, NUMASCAN_UPDATE_THRESH);
> > > +	/*
> > > +	 * If the number of pagein/out event is larger than 3% of limit or
> > > +	 * 64MB pagein/out, refresh numa information.
> > > +	 */
> > > +	if (atomic_read(&mem->numascan_update) < limit ||
> > > +	    !mutex_trylock(&mem->numascan_mutex))
> > >  		return;
> > 
> > I am not sure whether a mutex is not overkill here. What about using an
> > atomic operation instead?
> > 
> 
> I think mutex is informative than atomic counter for code readers.
> If influence of overhead is not big, I'd like to use mutex.

I do not have a strong opinion on that. mem_cgroup is not that
widespread structure to think about every single byte. On the other hand
atomic test&set would do the same thing. We are already using atomic
operations to manipulate numascan_update so doing it whole atomic based
sounds natural.

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 5/7] Fix not good check of mem_cgroup_local_usage()

[KAMEZAWA Hiroyuki]

Re: [PATCH 5/7] Fix not good check of mem_cgroup_local_usage()

[Ying Han]

On Wed, Jun 15, 2011 at 8:54 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> From fcfc6ee9847b0b2571cd6e9847572d7c70e1e2b2 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Thu, 16 Jun 2011 09:23:54 +0900
> Subject: [PATCH 5/7] Fix not good check of mem_cgroup_local_usage()
>
> Now, mem_cgroup_local_usage(memcg) is used as hint for scanning memory
> cgroup hierarchy. If it returns true, the memcg has some reclaimable memory.
>
> But this function doesn't take care of
>  - unevictable pages
>  - anon pages on swapless system.
>
> This patch fixes the function to use LRU information.
> For NUMA, for avoid scanning, numa scan bitmap is used. If it's
> empty, some more precise check will be done.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/memcontrol.c |   43 +++++++++++++++++++++++++++++++++----------
>  1 files changed, 33 insertions(+), 10 deletions(-)
>
> Index: mmotm-0615/mm/memcontrol.c
> ===================================================================
> --- mmotm-0615.orig/mm/memcontrol.c
> +++ mmotm-0615/mm/memcontrol.c
> @@ -632,15 +632,6 @@ static long mem_cgroup_read_stat(struct
>        return val;
>  }
>
> -static long mem_cgroup_local_usage(struct mem_cgroup *mem)
> -{
> -       long ret;
> -
> -       ret = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_RSS);
> -       ret += mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_CACHE);
> -       return ret;
> -}
> -
>  static void mem_cgroup_swap_statistics(struct mem_cgroup *mem,
>                                         bool charge)
>  {
> @@ -1713,6 +1704,23 @@ static void mem_cgroup_numascan_init(str
>        mutex_init(&mem->numascan_mutex);
>  }
>
> +static bool mem_cgroup_reclaimable(struct mem_cgroup *mem, bool noswap)
> +{
> +       if (!nodes_empty(mem->scan_nodes))
> +               return true;
> +       /* slow path */
> +       if (mem_cgroup_get_local_zonestat(mem, LRU_INACTIVE_FILE))
> +               return true;
> +       if (mem_cgroup_get_local_zonestat(mem, LRU_ACTIVE_FILE))
> +               return true;

Wondering if we can simplify this like:

if (mem_cgroup_nr_file_lru_pages(mem))
   return true;


> +       if (noswap || !total_swap_pages)
> +               return false;
> +       if (mem_cgroup_get_local_zonestat(mem, LRU_INACTIVE_ANON))
> +               return true;
> +       if (mem_cgroup_get_local_zonestat(mem, LRU_ACTIVE_ANON))
> +               return true;

the same:
if (mem_cgroup_nr_anon_lru_pages(mem))
   return true;

> +       return false;
> +}

The two functions above are part of memory.numa_stat patch which is in
mmotm i believe. Just feel the functionality a bit duplicate except
the noswap parameter and scan_nodes.

--Ying

>  #else
>  int mem_cgroup_select_victim_node(struct mem_cgroup *mem)
>  {
> @@ -1722,6 +1730,21 @@ static void mem_cgroup_numascan_init(str
>  {
>        return 0;
>  }
> +
> +static bool mem_cgroup_reclaimable(struct mem_cgroup *mem, bool noswap)
> +{
> +       if (mem_cgroup_get_zonestat_node(mem, 0, LRU_INACTIVE_FILE))
> +               return true;
> +       if (mem_cgroup_get_zonestat_node(mem, 0, LRU_ACTIVE_FILE))
> +               return true;
> +       if (noswap || !total_swap_pages)
> +               return false;
> +       if (mem_cgroup_get_zonestat_node(mem, 0, LRU_INACTIVE_ANON))
> +               return true;
> +       if (mem_cgroup_get_zonestat_node(mem, 0, LRU_ACTIVE_ANON))
> +               return true;
> +       return false;
> +}
>  #endif
>
>
> @@ -1811,7 +1834,7 @@ again:
>
>        while (visit--) {
>                victim = mem_cgroup_select_victim(root_mem);
> -               if (!mem_cgroup_local_usage(victim)) {
> +               if (!mem_cgroup_reclaimable(victim, noswap)) {
>                        /* this cgroup's local usage == 0 */
>                        css_put(&victim->css);
>                        continue;
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/7] Fix not good check of mem_cgroup_local_usage()

[KAMEZAWA Hiroyuki]

On Fri, 17 Jun 2011 15:27:36 -0700
Ying Han <yinghan@google.com> wrote:

> On Wed, Jun 15, 2011 at 8:54 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > From fcfc6ee9847b0b2571cd6e9847572d7c70e1e2b2 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Thu, 16 Jun 2011 09:23:54 +0900
> > Subject: [PATCH 5/7] Fix not good check of mem_cgroup_local_usage()
> >
> > Now, mem_cgroup_local_usage(memcg) is used as hint for scanning memory
> > cgroup hierarchy. If it returns true, the memcg has some reclaimable memory.
> >
> > But this function doesn't take care of
> > A - unevictable pages
> > A - anon pages on swapless system.
> >
> > This patch fixes the function to use LRU information.
> > For NUMA, for avoid scanning, numa scan bitmap is used. If it's
> > empty, some more precise check will be done.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> > A mm/memcontrol.c | A  43 +++++++++++++++++++++++++++++++++----------
> > A 1 files changed, 33 insertions(+), 10 deletions(-)
> >
> > Index: mmotm-0615/mm/memcontrol.c
> > ===================================================================
> > --- mmotm-0615.orig/mm/memcontrol.c
> > +++ mmotm-0615/mm/memcontrol.c
> > @@ -632,15 +632,6 @@ static long mem_cgroup_read_stat(struct
> > A  A  A  A return val;
> > A }
> >
> > -static long mem_cgroup_local_usage(struct mem_cgroup *mem)
> > -{
> > - A  A  A  long ret;
> > -
> > - A  A  A  ret = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_RSS);
> > - A  A  A  ret += mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_CACHE);
> > - A  A  A  return ret;
> > -}
> > -
> > A static void mem_cgroup_swap_statistics(struct mem_cgroup *mem,
> > A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  bool charge)
> > A {
> > @@ -1713,6 +1704,23 @@ static void mem_cgroup_numascan_init(str
> > A  A  A  A mutex_init(&mem->numascan_mutex);
> > A }
> >
> > +static bool mem_cgroup_reclaimable(struct mem_cgroup *mem, bool noswap)
> > +{
> > + A  A  A  if (!nodes_empty(mem->scan_nodes))
> > + A  A  A  A  A  A  A  return true;
> > + A  A  A  /* slow path */
> > + A  A  A  if (mem_cgroup_get_local_zonestat(mem, LRU_INACTIVE_FILE))
> > + A  A  A  A  A  A  A  return true;
> > + A  A  A  if (mem_cgroup_get_local_zonestat(mem, LRU_ACTIVE_FILE))
> > + A  A  A  A  A  A  A  return true;
> 
> Wondering if we can simplify this like:
> 
> if (mem_cgroup_nr_file_lru_pages(mem))
>    return true;
> 
> 
> > + A  A  A  if (noswap || !total_swap_pages)
> > + A  A  A  A  A  A  A  return false;
> > + A  A  A  if (mem_cgroup_get_local_zonestat(mem, LRU_INACTIVE_ANON))
> > + A  A  A  A  A  A  A  return true;
> > + A  A  A  if (mem_cgroup_get_local_zonestat(mem, LRU_ACTIVE_ANON))
> > + A  A  A  A  A  A  A  return true;
> 
> the same:
> if (mem_cgroup_nr_anon_lru_pages(mem))
>    return true;
> 
> > + A  A  A  return false;
> > +}
> 
> The two functions above are part of memory.numa_stat patch which is in
> mmotm i believe. Just feel the functionality a bit duplicate except
> the noswap parameter and scan_nodes.
> 

Ah, I didn't noticed such function.


Hm, considering more, I think we don't have to scann all nodes and
make sum of number because what we check is whether pages == 0 or
pages != 0.

I'll make an update.

Thanks,
-Kame




--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/7] Fix not good check of mem_cgroup_local_usage()

[Michal Hocko]

On Thu 16-06-11 12:54:43, KAMEZAWA Hiroyuki wrote:
> From fcfc6ee9847b0b2571cd6e9847572d7c70e1e2b2 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Thu, 16 Jun 2011 09:23:54 +0900
> Subject: [PATCH 5/7] Fix not good check of mem_cgroup_local_usage()
> 
> Now, mem_cgroup_local_usage(memcg) is used as hint for scanning memory
> cgroup hierarchy. If it returns true, the memcg has some reclaimable memory.
> 
> But this function doesn't take care of
>   - unevictable pages
>   - anon pages on swapless system.
> 
> This patch fixes the function to use LRU information.
> For NUMA, for avoid scanning, numa scan bitmap is used. If it's
> empty, some more precise check will be done.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/memcontrol.c |   43 +++++++++++++++++++++++++++++++++----------
>  1 files changed, 33 insertions(+), 10 deletions(-)
> 
> Index: mmotm-0615/mm/memcontrol.c
> ===================================================================
> --- mmotm-0615.orig/mm/memcontrol.c
> +++ mmotm-0615/mm/memcontrol.c
> @@ -632,15 +632,6 @@ static long mem_cgroup_read_stat(struct 
>  	return val;
>  }
>  
> -static long mem_cgroup_local_usage(struct mem_cgroup *mem)
> -{
> -	long ret;
> -
> -	ret = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_RSS);
> -	ret += mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_CACHE);
> -	return ret;
> -}
> -
>  static void mem_cgroup_swap_statistics(struct mem_cgroup *mem,
>  					 bool charge)
>  {
> @@ -1713,6 +1704,23 @@ static void mem_cgroup_numascan_init(str
>  	mutex_init(&mem->numascan_mutex);
>  }
>  
> +static bool mem_cgroup_reclaimable(struct mem_cgroup *mem, bool noswap)
> +{
> +	if (!nodes_empty(mem->scan_nodes))
> +		return true;

How the non empty node mask guarantees that there is some reclaimable memory?

> +	/* slow path */
> +	if (mem_cgroup_get_local_zonestat(mem, LRU_INACTIVE_FILE))
> +		return true;
> +	if (mem_cgroup_get_local_zonestat(mem, LRU_ACTIVE_FILE))
> +		return true;
> +	if (noswap || !total_swap_pages)
> +		return false;
> +	if (mem_cgroup_get_local_zonestat(mem, LRU_INACTIVE_ANON))
> +		return true;
> +	if (mem_cgroup_get_local_zonestat(mem, LRU_ACTIVE_ANON))
> +		return true;
> +	return false;
> +}
>  #else
>  int mem_cgroup_select_victim_node(struct mem_cgroup *mem)
>  {
> @@ -1722,6 +1730,21 @@ static void mem_cgroup_numascan_init(str
>  {
>  	return 0;
>  }
> +
> +static bool mem_cgroup_reclaimable(struct mem_cgroup *mem, bool noswap)
> +{
> +	if (mem_cgroup_get_zonestat_node(mem, 0, LRU_INACTIVE_FILE))
> +		return true;
> +	if (mem_cgroup_get_zonestat_node(mem, 0, LRU_ACTIVE_FILE))
> +		return true;
> +	if (noswap || !total_swap_pages)
> +		return false;
> +	if (mem_cgroup_get_zonestat_node(mem, 0, LRU_INACTIVE_ANON))
> +		return true;
> +	if (mem_cgroup_get_zonestat_node(mem, 0, LRU_ACTIVE_ANON))
> +		return true;
> +	return false;
> +}
>  #endif

Code duplication doesn't look good. What about a common helper?
>  
>  
> @@ -1811,7 +1834,7 @@ again:
>  
>  	while (visit--) {
>  		victim = mem_cgroup_select_victim(root_mem);
> -		if (!mem_cgroup_local_usage(victim)) {
> +		if (!mem_cgroup_reclaimable(victim, noswap)) {
>  			/* this cgroup's local usage == 0 */
>  			css_put(&victim->css);
>  			continue;
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 6/7] memcg: calc NUMA node's weight for scan.

[KAMEZAWA Hiroyuki]

Re: [PATCH 6/7] memcg: calc NUMA node's weight for scan.

[Michal Hocko]

On Thu 16-06-11 12:56:33, KAMEZAWA Hiroyuki wrote:
> From fb8aaa2c5f7fd99dfcb5d2ecb3c1226a58caafea Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Thu, 16 Jun 2011 10:05:46 +0900
> Subject: [PATCH 6/7] memcg: calc NUMA node's weight for scan.
> 
> Now, by commit 889976, numa node scan of memcg is in round-robin.
> As commit log says, "a better algorithm is needed".
> 
> for implementing some good scheduling, one of required things is
> defining importance of each node at LRU scanning.
> 
> This patch defines each node's weight for scan as
> 
> swappiness = (memcg's swappiness)? memcg's swappiness : 1
> FILE = inactive_file + (inactive_file_is_low)? active_file : 0
> ANON = inactive_anon + (inactive_anon_is_low)? active_anon : 0
> 
> weight = (FILE * (200-swappiness) + ANON * swappiness)/200.

Shouldn't we consider the node size?
If we have a node which is almost full with file cache and then other
node wich is much bigger and it is mostly occupied by anonymous memory
than the other node might end up with higher weight.

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 6/7] memcg: calc NUMA node's weight for scan.

[Hiroyuki Kamezawa]

2011/6/23 Michal Hocko <mhocko@suse.cz>:
> On Thu 16-06-11 12:56:33, KAMEZAWA Hiroyuki wrote:
>> From fb8aaa2c5f7fd99dfcb5d2ecb3c1226a58caafea Mon Sep 17 00:00:00 2001
>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> Date: Thu, 16 Jun 2011 10:05:46 +0900
>> Subject: [PATCH 6/7] memcg: calc NUMA node's weight for scan.
>>
>> Now, by commit 889976, numa node scan of memcg is in round-robin.
>> As commit log says, "a better algorithm is needed".
>>
>> for implementing some good scheduling, one of required things is
>> defining importance of each node at LRU scanning.
>>
>> This patch defines each node's weight for scan as
>>
>> swappiness = (memcg's swappiness)? memcg's swappiness : 1
>> FILE = inactive_file + (inactive_file_is_low)? active_file : 0
>> ANON = inactive_anon + (inactive_anon_is_low)? active_anon : 0
>>
>> weight = (FILE * (200-swappiness) + ANON * swappiness)/200.
>
> Shouldn't we consider the node size?

Above one cheks FILE+ANON....it's size of node.

> If we have a node which is almost full with file cache and then other
> node wich is much bigger and it is mostly occupied by anonymous memory
> than the other node might end up with higher weight.

I used a porportional fair scheduling in the next patch and I expect I'll not
see heavy starvation of node balancing. And if inactive_anon_is_low(),
the weight of anon-only-node will jump up.

But yes, other "weight" calculation is possible. The point of this patch
series is introducing a scheduler which can handle "weight" of value.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 7/7] memcg: proportional fair vicitm node selection

[KAMEZAWA Hiroyuki]

Re: [PATCH 7/7] memcg: proportional fair vicitm node selection

[Michal Hocko]

On Thu 16-06-11 12:57:41, KAMEZAWA Hiroyuki wrote:
> From 4fbd49697456c227c86f1d5b46f2cd2169bf1c5b Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Thu, 16 Jun 2011 11:25:23 +0900
> Subject: [PATCH 7/7] memcg: proportional fair node vicitm selection
> 
> commit 889976 implements a round-robin scan of numa nodes for
> LRU scanning of memcg at hitting limit.
> But, round-robin is not very good.
> 
> This patch implements a proportionally fair victim selection of nodes
> rather than round-robin. The logic is fair against each node's weight.
> 
> Each node's weight is calculated periodically and we build an node's
> scheduling entity as
> 
>      total_ticket = 0;
>      for_each_node(node)
>      	node->ticket_start =  total_ticket;
>         node->ticket_end   =  total_ticket + this_node's_weight()
>         total_ticket = node->ticket_end;
> 
> Then, each nodes has some amounts of tickets in proportion to its own weight.
> 
> At selecting victim, a random number is selected and the node which contains
> the random number in [ticket_start, ticket_end) is selected as vicitm.
> This is a lottery scheduling algorithm.
> 
> For quick search of victim, this patch uses bsearch().
> 
> Test result:
>   on 8cpu box with 2 nodes.
>   limit memory to be 300MB and run httpd for 4096files/600MB working set.
>   do (normalized) random access by apache-bench and see scan_stat.
>   The test makes 40960 request. and see scan_stat.
>   (Because a httpd thread just use 10% cpu, the number of threads will
>    not be balanced between nodes. Then, file caches will not be balanced
>    between nodes.)

Have you also tried to test with balanced nodes? I mean, is there any
measurable overhead?

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 7/7] memcg: proportional fair vicitm node selection

[Hiroyuki Kamezawa]

2011/6/23 Michal Hocko <mhocko@suse.cz>:
> On Thu 16-06-11 12:57:41, KAMEZAWA Hiroyuki wrote:
>> From 4fbd49697456c227c86f1d5b46f2cd2169bf1c5b Mon Sep 17 00:00:00 2001
>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> Date: Thu, 16 Jun 2011 11:25:23 +0900
>> Subject: [PATCH 7/7] memcg: proportional fair node vicitm selection
>>
>> commit 889976 implements a round-robin scan of numa nodes for
>> LRU scanning of memcg at hitting limit.
>> But, round-robin is not very good.
>>
>> This patch implements a proportionally fair victim selection of nodes
>> rather than round-robin. The logic is fair against each node's weight.
>>
>> Each node's weight is calculated periodically and we build an node's
>> scheduling entity as
>>
>>      total_ticket = 0;
>>      for_each_node(node)
>>       node->ticket_start =  total_ticket;
>>         node->ticket_end   =  total_ticket + this_node's_weight()
>>         total_ticket = node->ticket_end;
>>
>> Then, each nodes has some amounts of tickets in proportion to its own weight.
>>
>> At selecting victim, a random number is selected and the node which contains
>> the random number in [ticket_start, ticket_end) is selected as vicitm.
>> This is a lottery scheduling algorithm.
>>
>> For quick search of victim, this patch uses bsearch().
>>
>> Test result:
>>   on 8cpu box with 2 nodes.
>>   limit memory to be 300MB and run httpd for 4096files/600MB working set.
>>   do (normalized) random access by apache-bench and see scan_stat.
>>   The test makes 40960 request. and see scan_stat.
>>   (Because a httpd thread just use 10% cpu, the number of threads will
>>    not be balanced between nodes. Then, file caches will not be balanced
>>    between nodes.)
>
> Have you also tried to test with balanced nodes? I mean, is there any
> measurable overhead?
>

Not enough yet. I checked OOM trouble this week :).

I may need to make another fake_numa setup + cpuset
to measurements. In usual path, new overhead is random32() and
bsearch().  I'll do some.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 7/7] memcg: proportional fair vicitm node selection

[Michal Hocko]

On Thu 23-06-11 23:10:11, Hiroyuki Kamezawa wrote:
> 2011/6/23 Michal Hocko <mhocko@suse.cz>:
> > On Thu 16-06-11 12:57:41, KAMEZAWA Hiroyuki wrote:
> >> From 4fbd49697456c227c86f1d5b46f2cd2169bf1c5b Mon Sep 17 00:00:00 2001
> >> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> >> Date: Thu, 16 Jun 2011 11:25:23 +0900
> >> Subject: [PATCH 7/7] memcg: proportional fair node vicitm selection
> >>
> >> commit 889976 implements a round-robin scan of numa nodes for
> >> LRU scanning of memcg at hitting limit.
> >> But, round-robin is not very good.
> >>
> >> This patch implements a proportionally fair victim selection of nodes
> >> rather than round-robin. The logic is fair against each node's weight.
> >>
> >> Each node's weight is calculated periodically and we build an node's
> >> scheduling entity as
> >>
> >>      total_ticket = 0;
> >>      for_each_node(node)
> >>       node->ticket_start =  total_ticket;
> >>         node->ticket_end   =  total_ticket + this_node's_weight()
> >>         total_ticket = node->ticket_end;
> >>
> >> Then, each nodes has some amounts of tickets in proportion to its own weight.
> >>
> >> At selecting victim, a random number is selected and the node which contains
> >> the random number in [ticket_start, ticket_end) is selected as vicitm.
> >> This is a lottery scheduling algorithm.
> >>
> >> For quick search of victim, this patch uses bsearch().
> >>
> >> Test result:
> >>   on 8cpu box with 2 nodes.
> >>   limit memory to be 300MB and run httpd for 4096files/600MB working set.
> >>   do (normalized) random access by apache-bench and see scan_stat.
> >>   The test makes 40960 request. and see scan_stat.
> >>   (Because a httpd thread just use 10% cpu, the number of threads will
> >>    not be balanced between nodes. Then, file caches will not be balanced
> >>    between nodes.)
> >
> > Have you also tried to test with balanced nodes? I mean, is there any
> > measurable overhead?
> >
> 
> Not enough yet. I checked OOM trouble this week :).
> 
> I may need to make another fake_numa setup + cpuset
> to measurements. 

What if you just use NUMA rotor for page cache?

> In usual path, new overhead is random32() and
> bsearch().  I'll do some.
> 
> Thanks,
> -Kame

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 7/7] memcg: proportional fair vicitm node selection

[Hiroyuki Kamezawa]

2011/6/23 Michal Hocko <mhocko@suse.cz>:
> On Thu 23-06-11 23:10:11, Hiroyuki Kamezawa wrote:
>> 2011/6/23 Michal Hocko <mhocko@suse.cz>:
>> > On Thu 16-06-11 12:57:41, KAMEZAWA Hiroyuki wrote:
>> >> From 4fbd49697456c227c86f1d5b46f2cd2169bf1c5b Mon Sep 17 00:00:00 2001
>> >> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> >> Date: Thu, 16 Jun 2011 11:25:23 +0900
>> >> Subject: [PATCH 7/7] memcg: proportional fair node vicitm selection
>> >>
>> >> commit 889976 implements a round-robin scan of numa nodes for
>> >> LRU scanning of memcg at hitting limit.
>> >> But, round-robin is not very good.
>> >>
>> >> This patch implements a proportionally fair victim selection of nodes
>> >> rather than round-robin. The logic is fair against each node's weight.
>> >>
>> >> Each node's weight is calculated periodically and we build an node's
>> >> scheduling entity as
>> >>
>> >>      total_ticket = 0;
>> >>      for_each_node(node)
>> >>       node->ticket_start =  total_ticket;
>> >>         node->ticket_end   =  total_ticket + this_node's_weight()
>> >>         total_ticket = node->ticket_end;
>> >>
>> >> Then, each nodes has some amounts of tickets in proportion to its own weight.
>> >>
>> >> At selecting victim, a random number is selected and the node which contains
>> >> the random number in [ticket_start, ticket_end) is selected as vicitm.
>> >> This is a lottery scheduling algorithm.
>> >>
>> >> For quick search of victim, this patch uses bsearch().
>> >>
>> >> Test result:
>> >>   on 8cpu box with 2 nodes.
>> >>   limit memory to be 300MB and run httpd for 4096files/600MB working set.
>> >>   do (normalized) random access by apache-bench and see scan_stat.
>> >>   The test makes 40960 request. and see scan_stat.
>> >>   (Because a httpd thread just use 10% cpu, the number of threads will
>> >>    not be balanced between nodes. Then, file caches will not be balanced
>> >>    between nodes.)
>> >
>> > Have you also tried to test with balanced nodes? I mean, is there any
>> > measurable overhead?
>> >
>>
>> Not enough yet. I checked OOM trouble this week :).
>>
>> I may need to make another fake_numa setup + cpuset
>> to measurements.
>
> What if you just use NUMA rotor for page cache?
>

Ok, I'll do try in the next week. Thank you for suggestion.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>