Re: [patch] Revert "memcg: add memory.vmscan_stat"

KAMEZAWA Hiroyuki
2011-08-30T07:43:17Z

On Tue, 30 Aug 2011 16:20:50 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> Hmm...removing hierarchy part completely seems fine to me.
>
Another idea here.

==
Revert hierarchy support in vmscan_stat.

It turns out to be further study/use-case is required.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
Documentation/cgroups/memory.txt | 27 ++-------------------------
include/linux/memcontrol.h | 1 -
mm/memcontrol.c | 25 -------------------------
3 files changed, 2 insertions(+), 51 deletions(-)

Index: mmotm-Aug29/Documentation/cgroups/memory.txt
===================================================================
--- mmotm-Aug29.orig/Documentation/cgroups/memory.txt
+++ mmotm-Aug29/Documentation/cgroups/memory.txt
@@ -448,8 +448,8 @@ memory cgroup creation and can be reset

This file contains following statistics.

-[param]_[file_or_anon]_pages_by_[reason]_[under_heararchy]
-[param]_elapsed_ns_by_[reason]_[under_hierarchy]
+[param]_[file_or_anon]_pages_by_[reason]
+[param]_elapsed_ns_by_[reason]

For example,

@@ -470,9 +470,6 @@ Now, 2 reason are supported
system - global memory pressure + softlimit
(global memory pressure not under softlimit is not handled now)

-When under_hierarchy is added in the tail, the number indicates the
-total memcg scan of its children and itself.
-
elapsed_ns is a elapsed time in nanosecond. This may include sleep time
and not indicates CPU usage. So, please take this as just showing
latency.
@@ -500,26 +497,6 @@ freed_pages_by_system 0
freed_anon_pages_by_system 0
freed_file_pages_by_system 0
elapsed_ns_by_system 0
-scanned_pages_by_limit_under_hierarchy 9471864
-scanned_anon_pages_by_limit_under_hierarchy 6640629
-scanned_file_pages_by_limit_under_hierarchy 2831235
-rotated_pages_by_limit_under_hierarchy 4243974
-rotated_anon_pages_by_limit_under_hierarchy 3971968
-rotated_file_pages_by_limit_under_hierarchy 272006
-freed_pages_by_limit_under_hierarchy 2318492
-freed_anon_pages_by_limit_under_hierarchy 962052
-freed_file_pages_by_limit_under_hierarchy 1356440
-elapsed_ns_by_limit_under_hierarchy 351386416101
-scanned_pages_by_system_under_hierarchy 0
-scanned_anon_pages_by_system_under_hierarchy 0
-scanned_file_pages_by_system_under_hierarchy 0
-rotated_pages_by_system_under_hierarchy 0
-rotated_anon_pages_by_system_under_hierarchy 0
-rotated_file_pages_by_system_under_hierarchy 0
-freed_pages_by_system_under_hierarchy 0
-freed_anon_pages_by_system_under_hierarchy 0
-freed_file_pages_by_system_under_hierarchy 0
-elapsed_ns_by_system_under_hierarchy 0

5.3 swappiness

Index: mmotm-Aug29/mm/memcontrol.c
===================================================================
--- mmotm-Aug29.orig/mm/memcontrol.c
+++ mmotm-Aug29/mm/memcontrol.c
@@ -229,7 +229,6 @@ enum {
struct scanstat {
spinlock_t lock;
unsigned long stats[NR_SCAN_CONTEXT][NR_SCANSTATS];
- unsigned long rootstats[NR_SCAN_CONTEXT][NR_SCANSTATS];
};

const char *scanstat_string[NR_SCANSTATS] = {
@@ -246,7 +245,6 @@ const char *scanstat_string[NR_SCANSTATS
};
#define SCANSTAT_WORD_LIMIT "_by_limit"
#define SCANSTAT_WORD_SYSTEM "_by_system"
-#define SCANSTAT_WORD_HIERARCHY "_under_hierarchy"

/*
@@ -1710,11 +1708,6 @@ static void mem_cgroup_record_scanstat(s
spin_lock(&memcg->scanstat.lock);
__mem_cgroup_record_scanstat(memcg->scanstat.stats[context], rec);
spin_unlock(&memcg->scanstat.lock);
-
- memcg = rec->root;
- spin_lock(&memcg->scanstat.lock);
- __mem_cgroup_record_scanstat(memcg->scanstat.rootstats[context], rec);
- spin_unlock(&memcg->scanstat.lock);
}

/*
@@ -1758,8 +1751,6 @@ static int mem_cgroup_hierarchical_recla
else
rec.context = SCAN_BY_LIMIT;

- rec.root = root_memcg;
-
while (1) {
victim = mem_cgroup_select_victim(root_memcg);
if (victim == root_memcg) {
@@ -4728,20 +4719,6 @@ static int mem_cgroup_vmscan_stat_read(s
cb->fill(cb, string, memcg->scanstat.stats[SCAN_BY_SYSTEM][i]);
}

- for (i = 0; i < NR_SCANSTATS; i++) {
- strcpy(string, scanstat_string[i]);
- strcat(string, SCANSTAT_WORD_LIMIT);
- strcat(string, SCANSTAT_WORD_HIERARCHY);
- cb->fill(cb,
- string, memcg->scanstat.rootstats[SCAN_BY_LIMIT][i]);
- }
- for (i = 0; i < NR_SCANSTATS; i++) {
- strcpy(string, scanstat_string[i]);
- strcat(string, SCANSTAT_WORD_SYSTEM);
- strcat(string, SCANSTAT_WORD_HIERARCHY);
- cb->fill(cb,
- string, memcg->scanstat.rootstats[SCAN_BY_SYSTEM][i]);
- }
return 0;
}

@@ -4752,8 +4729,6 @@ static int mem_cgroup_reset_vmscan_stat(

spin_lock(&memcg->scanstat.lock);
memset(&memcg->scanstat.stats, 0, sizeof(memcg->scanstat.stats));
- memset(&memcg->scanstat.rootstats,
- 0, sizeof(memcg->scanstat.rootstats));
spin_unlock(&memcg->scanstat.lock);
return 0;
}
Index: mmotm-Aug29/include/linux/memcontrol.h
===================================================================
--- mmotm-Aug29.orig/include/linux/memcontrol.h
+++ mmotm-Aug29/include/linux/memcontrol.h
@@ -42,7 +42,6 @@ extern unsigned long mem_cgroup_isolate_

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[2]; /* the number of scanned pages */
unsigned long nr_rotated[2]; /* the number of rotated pages */