[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b239d23646a4054d65c791ca7118c1d1.squirrel@webmail-b.css.fujitsu.com>
Date: Sat, 18 Apr 2009 01:30:50 +0900 (JST)
From: "KAMEZAWA Hiroyuki" <kamezawa.hiroyu@...fujitsu.com>
To: balbir@...ux.vnet.ibm.com
Cc: "KAMEZAWA Hiroyuki" <kamezawa.hiroyu@...fujitsu.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Andrew Morton" <akpm@...ux-foundation.org>
Subject: Re: [PATCH] Add file based RSS accounting for memory resource
controller (v3)
Balbir Singh wrote:
> Hi, Kame,
>
> How does this look? I did not use the mapped flag in page_cgroup
> flags. page_is_file_cache and page_mapped worked as well.
>
> Feature: Add file RSS tracking per memory cgroup
>
> From: Balbir Singh <balbir@...ux.vnet.ibm.com>
>
> Changelog v3 -> v2
> 1. Fix get_cpu(), put_cpu() matching. Moved away from get_cpu() and use
> smp_processor_id(), since we are in preempt disable context
> 2. Use pc->mem_cgroup to identify the mem_cgroup instead of mm
> 3. page_add_file_rmap() and page_remove_rmap() argument changes are
> undone.
>
> Changelog v2 -> v1
>
> 1. Rename file_rss to mapped_file
> 2. Add hooks into mem_cgroup_move_account for updating MAPPED_FILE
> statistics
> 3. Use a better name for the statistics routine.
>
>
> We currently don't track file RSS, the RSS we report is actually anon RSS.
> All the file mapped pages, come in through the page cache and get
> accounted
> there. This patch adds support for accounting file RSS pages. It should
>
> 1. Help improve the metrics reported by the memory resource controller
> 2. Will form the basis for a future shared memory accounting heuristic
> that has been proposed by Kamezawa.
>
> Unfortunately, we cannot rename the existing "rss" keyword used in
> memory.stat
> to "anon_rss". We however, add "mapped_file" data and hope to educate the
> end
> user through documentation.
>
> Signed-off-by: Balbir Singh <balbir@...ux.vnet.ibm.com>
> ---
>
> include/linux/memcontrol.h | 7 ++++-
> mm/memcontrol.c | 66
> +++++++++++++++++++++++++++++++++++++++++++-
> mm/rmap.c | 5 +++
> 3 files changed, 75 insertions(+), 3 deletions(-)
>
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 18146c9..05a5c11 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -116,7 +116,7 @@ static inline bool mem_cgroup_disabled(void)
> }
>
> extern bool mem_cgroup_oom_called(struct task_struct *task);
> -
> +void mem_cgroup_update_mapped_file_stat(struct page *page, int val);
> #else /* CONFIG_CGROUP_MEM_RES_CTLR */
> struct mem_cgroup;
>
> @@ -264,6 +264,11 @@ mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
> struct task_struct *p)
> {
> }
>
> +static inline void mem_cgroup_update_mapped_file_stat(struct page *page,
> + int val)
> +{
> +}
> +
> #endif /* CONFIG_CGROUP_MEM_CONT */
>
> #endif /* _LINUX_MEMCONTROL_H */
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e44fb0f..562bd76 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -62,7 +62,8 @@ enum mem_cgroup_stat_index {
> * For MEM_CONTAINER_TYPE_ALL, usage = pagecache + rss.
> */
> MEM_CGROUP_STAT_CACHE, /* # of pages charged as cache */
> - MEM_CGROUP_STAT_RSS, /* # of pages charged as rss */
> + MEM_CGROUP_STAT_RSS, /* # of pages charged as anon rss */
> + MEM_CGROUP_STAT_MAPPED_FILE, /* # of pages charged as file rss */
> MEM_CGROUP_STAT_PGPGIN_COUNT, /* # of pages paged in */
> MEM_CGROUP_STAT_PGPGOUT_COUNT, /* # of pages paged out */
>
> @@ -321,6 +322,44 @@ static bool mem_cgroup_is_obsolete(struct mem_cgroup
> *mem)
> return css_is_removed(&mem->css);
> }
>
> +/*
> + * Currently used to update mapped file statistics, but the routine can
> be
> + * generalized to update other statistics as well.
> + */
> +void mem_cgroup_update_mapped_file_stat(struct page *page, int val)
> +{
> + struct mem_cgroup *mem;
> + struct mem_cgroup_stat *stat;
> + struct mem_cgroup_stat_cpu *cpustat;
> + int cpu;
> + struct page_cgroup *pc;
> +
> + if (!page_is_file_cache(page))
> + return;
> +
> + pc = lookup_page_cgroup(page);
> + if (unlikely(!pc))
> + return;
> +
> + lock_page_cgroup(pc);
> + mem = pc->mem_cgroup;
> + if (!mem)
> + goto done;
> +
> + if (!PageCgroupUsed(pc))
> + goto done;
> +
> + /*
> + * Preemption is already disabled, we don't need get_cpu()
> + */
> + cpu = smp_processor_id();
> + stat = &mem->stat;
> + cpustat = &stat->cpustat[cpu];
> +
> + __mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_MAPPED_FILE, val);
> +done:
> + unlock_page_cgroup(pc);
> +}
>
> /*
> * Call callback function against all cgroup under hierarchy tree.
> @@ -1096,6 +1135,10 @@ static int mem_cgroup_move_account(struct
> page_cgroup *pc,
> struct mem_cgroup_per_zone *from_mz, *to_mz;
> int nid, zid;
> int ret = -EBUSY;
> + struct page *page;
> + int cpu;
> + struct mem_cgroup_stat *stat;
> + struct mem_cgroup_stat_cpu *cpustat;
>
> VM_BUG_ON(from == to);
> VM_BUG_ON(PageLRU(pc->page));
> @@ -1116,6 +1159,23 @@ static int mem_cgroup_move_account(struct
> page_cgroup *pc,
>
> res_counter_uncharge(&from->res, PAGE_SIZE);
> mem_cgroup_charge_statistics(from, pc, false);
> +
> + page = pc->page;
> + if (page_is_file_cache(page) && page_mapped(page)) {
*Maybe* safe, but could you add comments ?
ok, let's start testing under -mm.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Nice feature :)
Thanks,
-Kame
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists