On Sat 05-01-13 18:52:12, Sha Zhengju wrote: > On Wed, Jan 2, 2013 at 8:27 PM, Michal Hocko <mhocko@xxxxxxx> wrote: > > On Wed 26-12-12 01:27:27, Sha Zhengju wrote: [...] > >> @@ -5396,18 +5406,70 @@ static inline void mem_cgroup_lru_names_not_uptodate(void) > >> BUILD_BUG_ON(ARRAY_SIZE(mem_cgroup_lru_names) != NR_LRU_LISTS); > >> } > >> > >> +long long root_memcg_local_stat(unsigned int i, long long val, > >> + long long nstat[]) > > > > Function should be static > > also > > nstat parameter is ugly because this can be done by the caller > > and also expecting that the caller already calculated val is not > > nice (and undocumented). This approach is really hackish and error > > prone. Why should we define a specific function rather than hooking into > > mem_cgroup_read_stat and doing all the stuff there? I think that would > > be much more maintainable. > > > > IMHO, hooking into mem_cgroup_read_stat may be also improper because > of the for_each_mem_cgroup traversal. I prefer to make mem_cgroup_read_stat > as the base func unit. But I'll repeal the function base on your opinion in next > version. Thanks for the advice! Maybe my "do all the stuff there" was confusing. I didn't mean to iterate through the hierarchy there. I just wanted to have mem_cgroup_root is a special case and it uses global counters there. -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>