> >> Signed-off-by: Glauber Costa <glommer@xxxxxxxxxxxxx> >> CC: Michal Hocko <mhocko@xxxxxxx> >> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> >> CC: Johannes Weiner <hannes@xxxxxxxxxxx> >> CC: Mel Gorman <mgorman@xxxxxxx> >> CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > After quick look, it seems most parts are good. But I have a concern. > > At memcg enablement, you move the numbers from vm_stat[] to res_counters. > Not only to res_counters. Mostly to mem_cgroup_stat_cpu, but I do move to res_counters as well. > Why you need it ? It's not explained. Because at this point, the bypass will no longer be in effect and we need accurate figures in root cgroup about what happened so far. If we always have root-level hierarchy, then the bypass could go on forever. But if we have not, we'll need to rely on whatever was in there. > And if it's necessary, uncharge will leak because page_cgroup is not marked > as PCG_USED, pc->mem_cgroup == NULL. So, res.usage will not be decreased. > The same problem happen when deriving an mz from a page. Since pc->mem_cgroup will be NULL. I am interpreting that as "root mem cgroup". Maybe even better would be to scan page cgroup writing a magic. Then if we see that magic we are sure it is an uninitialized pc. > Could you fix it if you need to move numbers to res_counter ? > At least for the pages in LRUs, I can scan them all, and update their page information. I am just wondering if this isn't a *very* expensive operation. Fine that we do it once, but still, is potentially scanning *all* pages in the system. So I've basically decided it is better to interpret pc->mem_cgroup = NULL as this uninitialized state. (and can change to a magic) -- 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>