Prev: [PATCH -tip 5/9] perf probe: Use elfutils-libdw for analyzing debuginfo
Next: [announce] gujin GPL bootloader version 2.8
From: Andrea Righi on 17 Mar 2010 19:00
On Tue, Mar 16, 2010 at 11:32:38AM +0900, Daisuke Nishimura wrote:
[snip]
> > @@ -3190,10 +3512,14 @@ struct {
> > } memcg_stat_strings[NR_MCS_STAT] = {
> > {"cache", "total_cache"},
> > {"rss", "total_rss"},
> > - {"mapped_file", "total_mapped_file"},
> > {"pgpgin", "total_pgpgin"},
> > {"pgpgout", "total_pgpgout"},
> > {"swap", "total_swap"},
> > + {"mapped_file", "total_mapped_file"},
> > + {"filedirty", "dirty_pages"},
> > + {"writeback", "writeback_pages"},
> > + {"writeback_tmp", "writeback_temp_pages"},
> > + {"nfs", "nfs_unstable"},
> > {"inactive_anon", "total_inactive_anon"},
> > {"active_anon", "total_active_anon"},
> > {"inactive_file", "total_inactive_file"},
> Why not using "total_xxx" for total_name ?

Agreed. I would be definitely more clear. Balbir, KAME-san, what do you
think?

>
> > @@ -3212,8 +3538,6 @@ static int mem_cgroup_get_local_stat(struct mem_cgroup *mem, void *data)
> > s->stat[MCS_CACHE] += val * PAGE_SIZE;
> > val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_RSS);
> > s->stat[MCS_RSS] += val * PAGE_SIZE;
> > - val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_FILE_MAPPED);
> > - s->stat[MCS_FILE_MAPPED] += val * PAGE_SIZE;
> > val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_PGPGIN_COUNT);
> > s->stat[MCS_PGPGIN] += val;
> > val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_PGPGOUT_COUNT);
> > @@ -3222,6 +3546,16 @@ static int mem_cgroup_get_local_stat(struct mem_cgroup *mem, void *data)
> > val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_SWAPOUT);
> > s->stat[MCS_SWAP] += val * PAGE_SIZE;
> > }
> > + val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_FILE_MAPPED);
> > + s->stat[MCS_FILE_MAPPED] += val * PAGE_SIZE;
> > + val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_FILE_DIRTY);
> > + s->stat[MCS_FILE_DIRTY] += val;
> > + val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_WRITEBACK);
> > + s->stat[MCS_WRITEBACK] += val;
> > + val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_WRITEBACK_TEMP);
> > + s->stat[MCS_WRITEBACK_TEMP] += val;
> > + val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_UNSTABLE_NFS);
> > + s->stat[MCS_UNSTABLE_NFS] += val;
> >
> I don't have a strong objection, but I prefer showing them in bytes.
> And can you add to mem_cgroup_stat_show() something like:
>
> for (i = 0; i < NR_MCS_STAT; i++) {
> if (i == MCS_SWAP && !do_swap_account)
> continue;
> + if (i >= MCS_FILE_STAT_STAR && i <= MCS_FILE_STAT_END &&
> + mem_cgroup_is_root(mem_cont))
> + continue;
> cb->fill(cb, memcg_stat_strings[i].local_name, mystat.stat[i]);
> }

I like this. And I also prefer to show these values in bytes.

>
> not to show file stat in root cgroup ? It's meaningless value anyway.
> Of course, you'd better mention it in [2/5] too.

OK.

Thanks,
-Andrea
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  | 
Pages: 1 2 3 4 5 6 7 8 9
Prev: [PATCH -tip 5/9] perf probe: Use elfutils-libdw for analyzing debuginfo
Next: [announce] gujin GPL bootloader version 2.8