Re: [PATCH] fix unused/stale swap cache handling on memcg v3

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Fri, 17 Apr 2009 16:50:36 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > > +	opl = orphan_lru(page_to_nid(page), page_zonenum(page));
> > >  	list_del_init(&pc->lru);
> > > +	opl->count--;
> > >  }
> > >  
> > >  static inline void add_orphan_list(struct page *page, struct page_cgroup *pc)
> > >  {
> > > +	int nid = page_to_nid(page);
> > > +	int zid = page_zonenum(page);
> > >  	struct orphan_list_zone *opl;
> > >  
> > >  	SetPageCgroupOrphan(pc);
> > 
> > here too.
> > 
> I think PCG_ORPHAN is protected by zone->lru_lock.
> 

There is different condition for swap caches from file-cache/anonymous pages.

File Cache and Anon pages are marked as USED before the first call of add_to_lru.
So, commit_charge_swapin()'s following code never breaks page_cgroup->flags.

 948         pc->mem_cgroup = mem;
 949         smp_wmb();
 950         pc->flags = pcg_default_flags[ctype];

Then, pc->flags can be broken.

please notice that

  43 /* Cache flag is set only once (at allocation) */
  44 TESTPCGFLAG(Cache, CACHE)
  45 
  46 TESTPCGFLAG(Used, USED)
  47 CLEARPCGFLAG(Used, USED)

ClearPageCgroupUsed() is only operation which modifes page_cgroup->flags,
but it's done under lock.

If you want to avoid lock_page_cgroup(), please rewrite commit_charge_swapin to do

    SetPageCgroupUsed(pc);
    SetPageCgroupCache(pc);
    ....
    or
   atomic_cmpxchg(&pc->flags, oldval, pcg_dafaule_flags[ctype]);

or some.

I'd like to divide lock bit and flags bit etc.. but cannot find a way to do it.


Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>