2013-09-05 06:29:09

by Greg Thelen

[permalink] [raw]
Subject: [PATCH 1/2 v3] memcg: refactor mem_control_numa_stat_show()

Refactor mem_control_numa_stat_show() to use a new stats structure for
smaller and simpler code. This consolidates nearly identical code.

text data bss dec hex filename
8,055,980 1,675,648 1,896,448 11,628,076 b16e2c vmlinux.before
8,055,276 1,675,648 1,896,448 11,627,372 b16b6c vmlinux.after

Signed-off-by: Greg Thelen <[email protected]>
Signed-off-by: Ying Han <[email protected]>
---
Changelog since v2:
- rebased to v3.11
- updated commit description

mm/memcontrol.c | 57 +++++++++++++++++++++++----------------------------------
1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0878ff7..4d2b037 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5378,45 +5378,34 @@ static int mem_cgroup_move_charge_write(struct cgroup *cgrp,
static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
struct seq_file *m)
{
+ struct numa_stat {
+ const char *name;
+ unsigned int lru_mask;
+ };
+
+ static const struct numa_stat stats[] = {
+ { "total", LRU_ALL },
+ { "file", LRU_ALL_FILE },
+ { "anon", LRU_ALL_ANON },
+ { "unevictable", BIT(LRU_UNEVICTABLE) },
+ { NULL, 0 } /* terminator */
+ };
+ const struct numa_stat *stat;
int nid;
- unsigned long total_nr, file_nr, anon_nr, unevictable_nr;
- unsigned long node_nr;
+ unsigned long nr;
struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);

- total_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL);
- seq_printf(m, "total=%lu", total_nr);
- for_each_node_state(nid, N_MEMORY) {
- node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid, LRU_ALL);
- seq_printf(m, " N%d=%lu", nid, node_nr);
- }
- seq_putc(m, '\n');
-
- file_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL_FILE);
- seq_printf(m, "file=%lu", file_nr);
- for_each_node_state(nid, N_MEMORY) {
- node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
- LRU_ALL_FILE);
- seq_printf(m, " N%d=%lu", nid, node_nr);
- }
- seq_putc(m, '\n');
-
- anon_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL_ANON);
- seq_printf(m, "anon=%lu", anon_nr);
- for_each_node_state(nid, N_MEMORY) {
- node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
- LRU_ALL_ANON);
- seq_printf(m, " N%d=%lu", nid, node_nr);
+ for (stat = stats; stat->name; stat++) {
+ nr = mem_cgroup_nr_lru_pages(memcg, stat->lru_mask);
+ seq_printf(m, "%s=%lu", stat->name, nr);
+ for_each_node_state(nid, N_MEMORY) {
+ nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
+ stat->lru_mask);
+ seq_printf(m, " N%d=%lu", nid, nr);
+ }
+ seq_putc(m, '\n');
}
- seq_putc(m, '\n');

- unevictable_nr = mem_cgroup_nr_lru_pages(memcg, BIT(LRU_UNEVICTABLE));
- seq_printf(m, "unevictable=%lu", unevictable_nr);
- for_each_node_state(nid, N_MEMORY) {
- node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
- BIT(LRU_UNEVICTABLE));
- seq_printf(m, " N%d=%lu", nid, node_nr);
- }
- seq_putc(m, '\n');
return 0;
}
#endif /* CONFIG_NUMA */
--
1.8.4


2013-09-05 06:34:42

by Greg Thelen

[permalink] [raw]
Subject: [PATCH 2/2 v3] memcg: support hierarchical memory.numa_stats

From: Ying Han <[email protected]>

The memory.numa_stat file was not hierarchical. Memory charged to the
children was not shown in parent's numa_stat.

This change adds the "hierarchical_" stats to the existing stats. The
new hierarchical stats include the sum of all children's values in
addition to the value of the memcg.

Tested: Create cgroup a, a/b and run workload under b. The values of
b are included in the "hierarchical_*" under a.

$ cd /sys/fs/cgroup
$ echo 1 > memory.use_hierarchy
$ mkdir a a/b

Run workload in a/b:
$ (echo $BASHPID >> a/b/cgroup.procs && cat /some/file && bash) &

The hierarchical_ fields in parent (a) show use of workload in a/b:
$ cat a/memory.numa_stat
total=0 N0=0 N1=0 N2=0 N3=0
file=0 N0=0 N1=0 N2=0 N3=0
anon=0 N0=0 N1=0 N2=0 N3=0
unevictable=0 N0=0 N1=0 N2=0 N3=0
hierarchical_total=61 N0=0 N1=41 N2=20 N3=0
hierarchical_file=14 N0=0 N1=0 N2=14 N3=0
hierarchical_anon=47 N0=0 N1=41 N2=6 N3=0
hierarchical_unevictable=0 N0=0 N1=0 N2=0 N3=0

The workload memory usage:
$ cat a/b/memory.numa_stat
total=73 N0=0 N1=41 N2=32 N3=0
file=14 N0=0 N1=0 N2=14 N3=0
anon=59 N0=0 N1=41 N2=18 N3=0
unevictable=0 N0=0 N1=0 N2=0 N3=0
hierarchical_total=73 N0=0 N1=41 N2=32 N3=0
hierarchical_file=14 N0=0 N1=0 N2=14 N3=0
hierarchical_anon=59 N0=0 N1=41 N2=18 N3=0
hierarchical_unevictable=0 N0=0 N1=0 N2=0 N3=0

Signed-off-by: Ying Han <[email protected]>
Signed-off-by: Greg Thelen <[email protected]>
---
Changelog since v2:
- reworded Documentation/cgroup/memory.txt
- updated commit description

Documentation/cgroups/memory.txt | 10 +++++++---
mm/memcontrol.c | 16 ++++++++++++++++
2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index 2a33306..d6d6479 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -571,15 +571,19 @@ an memcg since the pages are allowed to be allocated from any physical
node. One of the use cases is evaluating application performance by
combining this information with the application's CPU allocation.

-We export "total", "file", "anon" and "unevictable" pages per-node for
-each memcg. The ouput format of memory.numa_stat is:
+Each memcg's numa_stat file includes "total", "file", "anon" and "unevictable"
+per-node page counts including "hierarchical_<counter>" which sums of all
+hierarchical children's values in addition to the memcg's own value.
+
+The ouput format of memory.numa_stat is:

total=<total pages> N0=<node 0 pages> N1=<node 1 pages> ...
file=<total file pages> N0=<node 0 pages> N1=<node 1 pages> ...
anon=<total anon pages> N0=<node 0 pages> N1=<node 1 pages> ...
unevictable=<total anon pages> N0=<node 0 pages> N1=<node 1 pages> ...
+hierarchical_<counter>=<counter pages> N0=<node 0 pages> N1=<node 1 pages> ...

-And we have total = file + anon + unevictable.
+The "total" count is sum of file + anon + unevictable.

6. Hierarchy support

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4d2b037..0e5be30 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5394,6 +5394,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
int nid;
unsigned long nr;
struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
+ struct mem_cgroup *iter;

for (stat = stats; stat->name; stat++) {
nr = mem_cgroup_nr_lru_pages(memcg, stat->lru_mask);
@@ -5406,6 +5407,21 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
seq_putc(m, '\n');
}

+ for (stat = stats; stat->name; stat++) {
+ nr = 0;
+ for_each_mem_cgroup_tree(iter, memcg)
+ nr += mem_cgroup_nr_lru_pages(iter, stat->lru_mask);
+ seq_printf(m, "hierarchical_%s=%lu", stat->name, nr);
+ for_each_node_state(nid, N_MEMORY) {
+ nr = 0;
+ for_each_mem_cgroup_tree(iter, memcg)
+ nr += mem_cgroup_node_nr_lru_pages(
+ iter, nid, stat->lru_mask);
+ seq_printf(m, " N%d=%lu", nid, nr);
+ }
+ seq_putc(m, '\n');
+ }
+
return 0;
}
#endif /* CONFIG_NUMA */
--
1.8.4

2013-09-15 18:56:26

by Johannes Weiner

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] memcg: refactor mem_control_numa_stat_show()

Hi Greg,

On Wed, Sep 04, 2013 at 11:28:58PM -0700, Greg Thelen wrote:
> + struct numa_stat {
> + const char *name;
> + unsigned int lru_mask;
> + };
> +
> + static const struct numa_stat stats[] = {
> + { "total", LRU_ALL },
> + { "file", LRU_ALL_FILE },
> + { "anon", LRU_ALL_ANON },
> + { "unevictable", BIT(LRU_UNEVICTABLE) },
> + { NULL, 0 } /* terminator */
> + };

[...]

> + for (stat = stats; stat->name; stat++) {

Please drop the NULL terminator and use ARRAY_SIZE().

Otherwise, looks good to me.

2013-09-15 19:04:11

by Johannes Weiner

[permalink] [raw]
Subject: Re: [PATCH 2/2 v3] memcg: support hierarchical memory.numa_stats

Hello Greg!

On Wed, Sep 04, 2013 at 11:28:59PM -0700, Greg Thelen wrote:
> --- a/Documentation/cgroups/memory.txt
> +++ b/Documentation/cgroups/memory.txt
> @@ -571,15 +571,19 @@ an memcg since the pages are allowed to be allocated from any physical
> node. One of the use cases is evaluating application performance by
> combining this information with the application's CPU allocation.
>
> -We export "total", "file", "anon" and "unevictable" pages per-node for
> -each memcg. The ouput format of memory.numa_stat is:
> +Each memcg's numa_stat file includes "total", "file", "anon" and "unevictable"
> +per-node page counts including "hierarchical_<counter>" which sums of all
> +hierarchical children's values in addition to the memcg's own value.

"[...] which sums UP [...]"?

> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5394,6 +5394,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
> int nid;
> unsigned long nr;
> struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
> + struct mem_cgroup *iter;
>
> for (stat = stats; stat->name; stat++) {
> nr = mem_cgroup_nr_lru_pages(memcg, stat->lru_mask);
> @@ -5406,6 +5407,21 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
> seq_putc(m, '\n');
> }
>
> + for (stat = stats; stat->name; stat++) {

Move the struct mem_cgroup *iter declaration here?

> + nr = 0;
> + for_each_mem_cgroup_tree(iter, memcg)
> + nr += mem_cgroup_nr_lru_pages(iter, stat->lru_mask);
> + seq_printf(m, "hierarchical_%s=%lu", stat->name, nr);
> + for_each_node_state(nid, N_MEMORY) {
> + nr = 0;
> + for_each_mem_cgroup_tree(iter, memcg)
> + nr += mem_cgroup_node_nr_lru_pages(
> + iter, nid, stat->lru_mask);
> + seq_printf(m, " N%d=%lu", nid, nr);
> + }
> + seq_putc(m, '\n');
> + }
> +
> return 0;
> }
> #endif /* CONFIG_NUMA */

Rest looks fine to me.