OpenVZ Forum - RDF feed
type: channel
items: [RFC] [PATCH] memory controller statistics
items: Re: [RFC] [PATCH] memory controller statistics

[RFC] [PATCH] memory controller statistics
type: item
description: hi, i implemented some statistics for your memory controller. it's tested with 2.6.23-rc2-mm2 + memory controller v7. i think it can be applied to 2.6.23-rc4-mm1 as well. YAMOMOTO Takshi todo: something like nr_active/inactive in /proc/vmstat. --- ./mm/memcontrol.c.BACKUP 2007-08-29 17:13:09.000000000 +0900 +++ ./mm/memcontrol.c 2007-09-06 16:26:13.000000000 +0900 @@ -24,6 +24,7 @@ #include <linux/page-flags.h> #include <linux/bit_spinlock.h> #include <linux/rcupdate.h> +#include <linux/seq_file.h> #include <linux/swap.h> #include <linux/spinlock.h> #include <linux/fs.h> @@ -33,6 +34,54 @@ struct container_subsys mem_container_subsys; static const int MEM_CONTAINER_RECLAIM_RETRIES = 5; +enum mem_container_stat_index { + /* + * for MEM_CONTAINER_TYPE_ALL, usage == pagecache + rss + */ + MEMCONT_STAT_PAGECACHE, + MEMCONT_STAT_RSS, + + /* + * redundant; usage == charge - uncharge + */ + MEMCONT_STAT_CHARGE, + MEMCONT_STAT_UNCHARGE, + + /* + * mostly for debug + */ + MEMCONT_STAT_ISOLATE, + MEMCONT_STAT_ISOLATE_FAIL, + MEMCONT_STAT_NSTATS, +}; + +static const char * const mem_container_stat_desc[] = { + [MEMCONT_STAT_PAGECACHE] = "page_cache", + [MEMCONT_STAT_RSS] = "rss", + [MEMCONT_STAT_CHARGE] = "charge", + [MEMCONT_STAT_UNCHARGE] = "uncharge", + [MEMCONT_STAT_ISOLATE] = "isolate", + [MEMCONT_STAT_ISOLATE_FAIL] = "isolate_fail", +}; + +struct mem_container_stat { + atomic_t count[MEMCONT_STAT_NSTATS]; +}; + +static void mem_container_stat_inc(struct mem_container_stat * stat, + enum mem_container_stat_index idx) +{ + + atomic_inc(&stat->count[idx]); +} + +static void mem_container_stat_dec(struct mem_container_stat * stat, + enum mem_container_stat_index idx) +{ + + atomic_dec(&stat->count[idx]); +} + /* * The memory controller data structure. The memory controller controls both * page cache and RSS per container. We would eventually like to provide @@ -62,6 +111,7 @@ struct mem_container { */ spinlock_t lru_lock; unsigned long control_type; /* control RSS or RSS+Pag…
creator: yamamoto
date: 2007-09-07T03:39:42-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: Hi, On Fri, 7 Sep 2007 12:39:42 +0900 (JST) yamamoto@valinux.co.jp (YAMAMOTO Takashi) wrote: > +enum mem_container_stat_index { > + /* > + * for MEM_CONTAINER_TYPE_ALL, usage == pagecache + rss > + */ > + MEMCONT_STAT_PAGECACHE, > + MEMCONT_STAT_RSS, > + > + /* > + * redundant; usage == charge - uncharge > + */ > + MEMCONT_STAT_CHARGE, > + MEMCONT_STAT_UNCHARGE, > + > + /* > + * mostly for debug > + */ > + MEMCONT_STAT_ISOLATE, > + MEMCONT_STAT_ISOLATE_FAIL, > + MEMCONT_STAT_NSTATS, > +}; > + please add comments on each statistics name. It's uneasy to catch the meaning of ISOLATE/ISOLATE_FAIL without comments. > +static const char * const mem_container_stat_desc[] = { > + [MEMCONT_STAT_PAGECACHE] = "page_cache", > + [MEMCONT_STAT_RSS] = "rss", > + [MEMCONT_STAT_CHARGE] = "charge", > + [MEMCONT_STAT_UNCHARGE] = "uncharge", > + [MEMCONT_STAT_ISOLATE] = "isolate", > + [MEMCONT_STAT_ISOLATE_FAIL] = "isolate_fail", > +}; > + > +struct mem_container_stat { > + atomic_t count[MEMCONT_STAT_NSTATS]; > +}; > + > +static void mem_container_stat_inc(struct mem_container_stat * stat, > + enum mem_container_stat_index idx) > +{ > + > + atomic_inc(&stat->count[idx]); > +} > + > +static void mem_container_stat_dec(struct mem_container_stat * stat, > + enum mem_container_stat_index idx) > +{ > + > + atomic_dec(&stat->count[idx]); > +} > + Can we do this accounting as mod_zone_page_state()(in mm/vmstat.c) ? (use per-cpu data for accounting.) > +/* XXX hack; shouldn't be here. it really belongs to struct page_container. */ > +#define PAGE_CONTAINER_CACHE_BIT 0x1 > +#define PAGE_CONTAINER_CACHE (1 << PAGE_CONTAINER_CACHE_BIT) > + Is this used for remebering whether a page is charged as page-cache or not ? > + page_assign_page_container_flags(page, > + is_cache ? PAGE_CONTAINER_CACHE : 0, pc); > + > + stat = &mem->stat; > + if (is_cache) { > + mem_container_stat_inc(stat, MEMCONT_STAT_PAGECACHE); > + } else { > + mem_container_stat_inc(stat, MEMCONT_STAT_RSS); > + } nitpick,in linux sty…
creator: KAMEZAWA Hiroyuki
date: 2007-09-07T05:02:34-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: hi, thanks for comments. > Hi, > > On Fri, 7 Sep 2007 12:39:42 +0900 (JST) > yamamoto@valinux.co.jp (YAMAMOTO Takashi) wrote: > > > +enum mem_container_stat_index { > > + /* > > + * for MEM_CONTAINER_TYPE_ALL, usage == pagecache + rss > > + */ > > + MEMCONT_STAT_PAGECACHE, > > + MEMCONT_STAT_RSS, > > + > > + /* > > + * redundant; usage == charge - uncharge > > + */ > > + MEMCONT_STAT_CHARGE, > > + MEMCONT_STAT_UNCHARGE, > > + > > + /* > > + * mostly for debug > > + */ > > + MEMCONT_STAT_ISOLATE, > > + MEMCONT_STAT_ISOLATE_FAIL, > > + MEMCONT_STAT_NSTATS, > > +}; > > + > please add comments on each statistics name. sure. > It's uneasy to catch the meaning of > ISOLATE/ISOLATE_FAIL without comments. they aren't useful for users who don't read the relevant code. probably they should be just removed. > > +static const char * const mem_container_stat_desc[] = { > > + [MEMCONT_STAT_PAGECACHE] = "page_cache", > > + [MEMCONT_STAT_RSS] = "rss", > > + [MEMCONT_STAT_CHARGE] = "charge", > > + [MEMCONT_STAT_UNCHARGE] = "uncharge", > > + [MEMCONT_STAT_ISOLATE] = "isolate", > > + [MEMCONT_STAT_ISOLATE_FAIL] = "isolate_fail", > > +}; > > + > > +struct mem_container_stat { > > + atomic_t count[MEMCONT_STAT_NSTATS]; > > +}; > > + > > +static void mem_container_stat_inc(struct mem_container_stat * stat, > > + enum mem_container_stat_index idx) > > +{ > > + > > + atomic_inc(&stat->count[idx]); > > +} > > + > > +static void mem_container_stat_dec(struct mem_container_stat * stat, > > + enum mem_container_stat_index idx) > > +{ > > + > > + atomic_dec(&stat->count[idx]); > > +} > > + > > Can we do this accounting as mod_zone_page_state()(in mm/vmstat.c) ? > (use per-cpu data for accounting.) we can do so later. > > +/* XXX hack; shouldn't be here. it really belongs to struct page_container. */ > > +#define PAGE_CONTAINER_CACHE_BIT 0x1 > > +#define PAGE_CONTAINER_CACHE (1 << PAGE_CONTAINER_CACHE_BIT) > > + > > Is this used for remebering whether a page is charged as page-cache or not ? yes…
creator: yamamoto
date: 2007-09-07T05:38:29-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: > hi, > > i implemented some statistics for your memory controller. > > it's tested with 2.6.23-rc2-mm2 + memory controller v7. > i think it can be applied to 2.6.23-rc4-mm1 as well. > > YAMOMOTO Takshi > > todo: something like nr_active/inactive in /proc/vmstat. here's the version i'm working on currently. any comments? changes from the previous version: - adapt to 2.6.23-rc8-mm2, container -> cgroup rename. - reflect some of comments on this list. - rename some macros as suggested by balbir - sprinkle some inlines as suggested by balbir. - remove "isolate" statistics - remove PAGE_CONTAINER_CACHE hack and add "flags" member to page_container instead. - make counters atomic_long_t. - add some comments. - coding style. - implement nr_active/nr_inactive. they show numbers of pages on per-cgroup lru lists. todo: - consider to make counters per-cpu. - more statistics. YAMAMOTO Takashi --- linux-2.6.23-rc8-mm2-stat/mm/memcontrol.c.BACKUP 2007-10-01 17:19:57.000000000 +0900 +++ linux-2.6.23-rc8-mm2-stat/mm/memcontrol.c 2007-10-04 12:42:05.000000000 +0900 @@ -25,6 +25,7 @@ #include <linux/backing-dev.h> #include <linux/bit_spinlock.h> #include <linux/rcupdate.h> +#include <linux/seq_file.h> #include <linux/swap.h> #include <linux/spinlock.h> #include <linux/fs.h> @@ -34,6 +35,52 @@ struct cgroup_subsys mem_cgroup_subsys; static const int MEM_CGROUP_RECLAIM_RETRIES = 5; +enum mem_cgroup_stat_index { + /* + * for MEM_CONTAINER_TYPE_ALL, usage == pagecache + rss + */ + MEM_CGROUP_STAT_PAGECACHE, /* # of pages charged as cache */ + MEM_CGROUP_STAT_RSS, /* # of pages charged as rss */ + + /* + * redundant; usage == charge - uncharge + */ + MEM_CGROUP_STAT_CHARGE, /* # of pages charged */ + MEM_CGROUP_STAT_UNCHARGE, /* # of pages uncharged */ + + MEM_CGROUP_STAT_ACTIVE, /* # of pages on active_list */ + MEM_CGROUP_STAT_INACTIVE, /* # of pages on inactive_list */ + + MEM_CGROUP_STAT_NSTATS, +}; + +static const char * const mem_cgroup_stat_desc[] = { + [MEM_CGROUP_STAT_PAGECACHE]…
creator: yamamoto
date: 2007-10-04T05:13:32-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: YAMAMOTO Takashi wrote: >> hi, >> >> i implemented some statistics for your memory controller. >> >> it's tested with 2.6.23-rc2-mm2 + memory controller v7. >> i think it can be applied to 2.6.23-rc4-mm1 as well. >> >> YAMOMOTO Takshi >> >> todo: something like nr_active/inactive in /proc/vmstat. > > here's the version i'm working on currently. any comments? > > changes from the previous version: > > - adapt to 2.6.23-rc8-mm2, container -> cgroup rename. > - reflect some of comments on this list. > - rename some macros as suggested by balbir > - sprinkle some inlines as suggested by balbir. > - remove "isolate" statistics > - remove PAGE_CONTAINER_CACHE hack and > add "flags" member to page_container instead. > - make counters atomic_long_t. > - add some comments. > - coding style. > - implement nr_active/nr_inactive. they show numbers of pages on > per-cgroup lru lists. > > todo: > - consider to make counters per-cpu. > - more statistics. > > YAMAMOTO Takashi Hi, YAMAMOTO-San Looks much better, I did a quick review, it looks nice to me. I'll test the patches and get back. Thank you for working on this, -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-10-05T03:51:06-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: Balbir Singh wrote: > YAMAMOTO Takashi wrote: >>> hi, >>> >>> i implemented some statistics for your memory controller. >>> >>> it's tested with 2.6.23-rc2-mm2 + memory controller v7. >>> i think it can be applied to 2.6.23-rc4-mm1 as well. >>> >>> YAMOMOTO Takshi >>> >>> todo: something like nr_active/inactive in /proc/vmstat. >> here's the version i'm working on currently. any comments? >> >> changes from the previous version: >> >> - adapt to 2.6.23-rc8-mm2, container -> cgroup rename. >> - reflect some of comments on this list. >> - rename some macros as suggested by balbir >> - sprinkle some inlines as suggested by balbir. >> - remove "isolate" statistics >> - remove PAGE_CONTAINER_CACHE hack and >> add "flags" member to page_container instead. >> - make counters atomic_long_t. >> - add some comments. >> - coding style. >> - implement nr_active/nr_inactive. they show numbers of pages on >> per-cgroup lru lists. >> >> todo: >> - consider to make counters per-cpu. >> - more statistics. >> >> YAMAMOTO Takashi > > Hi, YAMAMOTO-San > > Looks much better, I did a quick review, it looks nice to me. > I'll test the patches and get back. > > Thank you for working on this, > Forgot to mention one other detail, could we track the statistics in bytes? We track usage and limit in bytes currently. I am open to opinions on this, but the last time we discussed numbers, tracking bytes was the preferred approach. -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-10-07T05:56:59-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: > Forgot to mention one other detail, could we track the statistics > in bytes? We track usage and limit in bytes currently. I am open > to opinions on this, but the last time we discussed numbers, > tracking bytes was the preferred approach. while /proc/vmstat and others uses number of pages? IMO, numbers of pages make more sense for this kind of statistics. however, if there's a consensus, i can follow. after all, it's merely a matter of taste. YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2007-10-08T22:55:28-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: YAMAMOTO Takashi wrote: >> Forgot to mention one other detail, could we track the statistics >> in bytes? We track usage and limit in bytes currently. I am open >> to opinions on this, but the last time we discussed numbers, >> tracking bytes was the preferred approach. > > while /proc/vmstat and others uses number of pages? > IMO, numbers of pages make more sense for this kind of statistics. > Yes, I thought so too, but consensus was built around using bytes for limit and usage for the memory controller. Keeping all stats in the same units would make it less confusing for users. > however, if there's a consensus, i can follow. after all, > it's merely a matter of taste. > I agree > YAMAMOTO Takashi -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-10-09T03:11:39-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: hi, > > i implemented some statistics for your memory controller. here's a new version. changes from the previous: - make counters per-cpu. - value *= PAGE_SIZE YAMAMOTO Takashi --- linux-2.6.23-rc8-mm2-stat/mm/memcontrol.c.BACKUP 2007-10-01 17:19:57.000000000 +0900 +++ linux-2.6.23-rc8-mm2-stat/mm/memcontrol.c 2007-10-09 13:41:43.000000000 +0900 @@ -25,6 +25,7 @@ #include <linux/backing-dev.h> #include <linux/bit_spinlock.h> #include <linux/rcupdate.h> +#include <linux/seq_file.h> #include <linux/swap.h> #include <linux/spinlock.h> #include <linux/fs.h> @@ -34,6 +35,63 @@ struct cgroup_subsys mem_cgroup_subsys; static const int MEM_CGROUP_RECLAIM_RETRIES = 5; +enum mem_cgroup_stat_index { + /* + * for MEM_CONTAINER_TYPE_ALL, usage == pagecache + rss + */ + MEM_CGROUP_STAT_PAGECACHE, /* # of pages charged as cache */ + MEM_CGROUP_STAT_RSS, /* # of pages charged as rss */ + + /* + * redundant; usage == charge - uncharge + */ + MEM_CGROUP_STAT_CHARGE, /* # of pages charged */ + MEM_CGROUP_STAT_UNCHARGE, /* # of pages uncharged */ + + MEM_CGROUP_STAT_ACTIVE, /* # of pages on active_list */ + MEM_CGROUP_STAT_INACTIVE, /* # of pages on inactive_list */ + + MEM_CGROUP_STAT_NSTATS, +}; + +static const struct mem_cgroup_stat_desc { + const char *msg; + u64 unit; +} mem_cgroup_stat_desc[] = { + [MEM_CGROUP_STAT_PAGECACHE] = { "page_cache", PAGE_SIZE, }, + [MEM_CGROUP_STAT_RSS] = { "rss", PAGE_SIZE, }, + [MEM_CGROUP_STAT_CHARGE] = { "charge", PAGE_SIZE }, + [MEM_CGROUP_STAT_UNCHARGE] = { "uncharge", PAGE_SIZE, }, + [MEM_CGROUP_STAT_ACTIVE] = { "active", PAGE_SIZE, }, + [MEM_CGROUP_STAT_INACTIVE] = { "inactive", PAGE_SIZE, }, +}; + +struct mem_cgroup_stat_cpu { + u64 count[MEM_CGROUP_STAT_NSTATS]; +} ____cacheline_aligned_in_smp; + +struct mem_cgroup_stat { + struct mem_cgroup_stat_cpu cpustat[NR_CPUS]; +}; + +static inline void mem_cgroup_stat_inc(struct mem_cgroup_stat * stat, + enum mem_cgroup_stat_index idx) +{ + unsigned int cpu = get_cpu(); + + stat->cpustat[cpu].count[i…
creator: yamamoto
date: 2007-10-10T01:01:17-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: On Wed, 10 Oct 2007 10:01:17 +0900 (JST) yamamoto@valinux.co.jp (YAMAMOTO Takashi) wrote: > hi, > > > > i implemented some statistics for your memory controller. > > here's a new version. > > changes from the previous: > - make counters per-cpu. > - value *= PAGE_SIZE > YAMAMOTO-san, I like this work and the patch seems good. But will HUNK with my work to some extent ;) So, I'd like to merge this patch on my patch set (against -mm) if you don't mind. If it's ok, please give your "Signed-off-by" line. Then, I'll merge this to mine. Thanks, -Kame _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: KAMEZAWA Hiroyuki
date: 2007-10-10T06:44:15-00:00

Re: [RFC] [PATCH] memory controller statistics
type: item
description: > On Wed, 10 Oct 2007 10:01:17 +0900 (JST) > yamamoto@valinux.co.jp (YAMAMOTO Takashi) wrote: > > > hi, > > > > > > i implemented some statistics for your memory controller. > > > > here's a new version. > > > > changes from the previous: > > - make counters per-cpu. > > - value *= PAGE_SIZE > > > > YAMAMOTO-san, I like this work and the patch seems good. > But will HUNK with my work to some extent ;) > > So, I'd like to merge this patch on my patch set (against -mm) if you don't mind. > If it's ok, please give your "Signed-off-by" line. Then, I'll merge this to mine. > > Thanks, > -Kame sure. here's the latest version. changes from the previous: - fix a race in uncharge. (check PCGF_ACTIVE with mem->lru_lock held) YAMAMOTO Takashi Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp> --- --- linux-2.6.23-rc8-mm2-stat/mm/memcontrol.c.BACKUP 2007-10-01 17:19:57.000000000 +0900 +++ linux-2.6.23-rc8-mm2-stat/mm/memcontrol.c 2007-10-10 12:40:48.000000000 +0900 @@ -25,6 +25,7 @@ #include <linux/backing-dev.h> #include <linux/bit_spinlock.h> #include <linux/rcupdate.h> +#include <linux/seq_file.h> #include <linux/swap.h> #include <linux/spinlock.h> #include <linux/fs.h> @@ -34,6 +35,63 @@ struct cgroup_subsys mem_cgroup_subsys; static const int MEM_CGROUP_RECLAIM_RETRIES = 5; +enum mem_cgroup_stat_index { + /* + * for MEM_CONTAINER_TYPE_ALL, usage == pagecache + rss + */ + MEM_CGROUP_STAT_PAGECACHE, /* # of pages charged as cache */ + MEM_CGROUP_STAT_RSS, /* # of pages charged as rss */ + + /* + * redundant; usage == charge - uncharge + */ + MEM_CGROUP_STAT_CHARGE, /* # of pages charged */ + MEM_CGROUP_STAT_UNCHARGE, /* # of pages uncharged */ + + MEM_CGROUP_STAT_ACTIVE, /* # of pages on active_list */ + MEM_CGROUP_STAT_INACTIVE, /* # of pages on inactive_list */ + + MEM_CGROUP_STAT_NSTATS, +}; + +static const struct mem_cgroup_stat_desc { + const char *msg; + u64 unit; +} mem_cgroup_stat_desc[] = { + [MEM_CGROUP_STAT_PAGECACHE] = { "page_cache", PAGE_SIZE, }, + [MEM_C…
creator: yamamoto
date: 2007-10-10T08:19:23-00:00