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

[Johannes Weiner]

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Johannes Weiner]

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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Greg Thelen]

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

   text data  bssdec  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 
Signed-off-by: Ying Han 
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Greg Thelen]

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

   text data  bssdec  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 gthe...@google.com
Signed-off-by: Ying Han ying...@google.com
---
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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/