On Fri 20-01-12 11:19:47, KAMEZAWA Hiroyuki wrote: > On Thu, 19 Jan 2012 15:47:12 +0100 > Michal Hocko <mhocko@xxxxxxx> wrote: > > > On Fri 13-01-12 17:45:10, KAMEZAWA Hiroyuki wrote: > > > From 3df71cef5757ee6547916c4952f04a263c1b8ddb Mon Sep 17 00:00:00 2001 > > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> > > > Date: Fri, 13 Jan 2012 17:07:35 +0900 > > > Subject: [PATCH 7/7] memcg: make mem_cgroup_begin_update_stat to use global pcpu. > > > > > > Now, a per-cpu flag to show the memcg is under account moving is > > > now implemented as per-memcg-per-cpu. > > > > > > So, when accessing this, we need to access memcg 1st. But this > > > function is called even when status update doesn't occur. Then, > > > accessing struct memcg is an overhead in such case. > > > > > > This patch removes per-cpu-per-memcg MEM_CGROUP_ON_MOVE and add > > > per-cpu vairable to do the same work. For per-memcg, atomic > > > counter is added. By this, mem_cgroup_begin_update_stat() will > > > just access percpu variable in usual case and don't need to find & access > > > memcg. This reduces overhead. > > > > I agree that move_account is not a hotpath and that we don't have > > to optimize for it but I guess we can do better. If we use a cookie > > parameter for > > mem_cgroup_{begin,end}_update_stat(struct page *page, unsigned long *cookie) > > then we can stab page_cgroup inside and use the last bit for > > locked. Then we do not have to call lookup_page_cgroup again in > > mem_cgroup_update_page_stat and just replace page by the cookie. > > What do you think? > > > > Because these routine is called as > > mem_cgroup_begin_update_stat() > if (condition) > set_page_flag > mem_cgroup_update_stat() > mem_cgroup_end_update_stat() > > In earlier version(not posted), I did so. Now, I don't because of 2 reasons. > > 1. I wonder it's better not to have extra arguments in begin_xxx it > will be overhead itself. I am not sure this could be noticable. > 2. my work's final purpose is integrate page_cgroup to struct page. > If I can do, lookup_page_cgroup() cost will be almost 0 and we'll revert > the cookie, finally. OK > So, can't we keep this update routine simple for a while ? Sure. I was just concerned that global move account state might be an issue because we would have an side effect interaction between different cgroups. > If we saw it's finally impossible to integrate page_cgroup to page, > I'd like to consider 'cookie' again. > > BTW, If we use spinlock and need to do irq_disable() in begin_update_stat() > we'll need to pass *flags... Right, you said that dirty page accounting requires to be called from IRQ context as well. > > Thanks, > -Kame > > -- > To unsubscribe from this list: send the line "unsubscribe cgroups" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html