On Thu, 8 Sep 2011 09:40:22 +0200 Johannes Weiner <jweiner@xxxxxxxxxx> wrote: > There is a potential race between a thread charging a page and another > thread putting it back to the LRU list: > > charge: putback: > SetPageCgroupUsed SetPageLRU > PageLRU && add to memcg LRU PageCgroupUsed && add to memcg LRU > I assumed that all pages are charged before added to LRU. (i.e. event happens in charge->lru_lock->putback order.) But hmm, this assumption may be bad for maintainance. Do you find a code which adds pages to LRU before charge ? Hmm, if there are codes which recharge the page to other memcg, it will cause bug and my assumption may be harmful. > The order of setting one flag and checking the other is crucial, > otherwise the charge may observe !PageLRU while the putback observes > !PageCgroupUsed and the page is not linked to the memcg LRU at all. > > Global memory pressure may fix this by trying to isolate and putback > the page for reclaim, where that putback would link it to the memcg > LRU again. Without that, the memory cgroup is undeletable due to a > charge whose physical page can not be found and moved out. > > Signed-off-by: Johannes Weiner <jweiner@xxxxxxxxxx> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> > --- > mm/memcontrol.c | 21 ++++++++++++++++++++- > 1 files changed, 20 insertions(+), 1 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index d63dfb2..17708e1 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -990,6 +990,16 @@ void mem_cgroup_add_lru_list(struct page *page, enum lru_list lru) > return; > pc = lookup_page_cgroup(page); > VM_BUG_ON(PageCgroupAcctLRU(pc)); > + /* > + * putback: charge: > + * SetPageLRU SetPageCgroupUsed > + * smp_mb smp_mb > + * PageCgroupUsed && add to memcg LRU PageLRU && add to memcg LRU > + * > + * Ensure that one of the two sides adds the page to the memcg > + * LRU during a race. > + */ > + smp_mb(); > if (!PageCgroupUsed(pc)) > return; > /* Ensure pc->mem_cgroup is visible after reading PCG_USED. */ > @@ -1041,7 +1051,16 @@ static void mem_cgroup_lru_add_after_commit(struct page *page) > unsigned long flags; > struct zone *zone = page_zone(page); > struct page_cgroup *pc = lookup_page_cgroup(page); > - > + /* > + * putback: charge: > + * SetPageLRU SetPageCgroupUsed > + * smp_mb smp_mb > + * PageCgroupUsed && add to memcg LRU PageLRU && add to memcg LRU > + * > + * Ensure that one of the two sides adds the page to the memcg > + * LRU during a race. > + */ > + smp_mb(); > /* taking care of that the page is added to LRU while we commit it */ > if (likely(!PageLRU(page))) > return; > -- > 1.7.6 > > -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>