linux-kernel - Re: [patch 8/8] mm: make per-memcg lru lists exclusive

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110812191718.GE29086@cmpxchg.org>
Date:	Fri, 12 Aug 2011 21:17:18 +0200
From:	Johannes Weiner <hannes@...xchg.org>
To:	Ying Han <yinghan@...gle.com>
Cc:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Daisuke Nishimura <nishimura@....nes.nec.co.jp>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	Michal Hocko <mhocko@...e.cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>,
	Minchan Kim <minchan.kim@...il.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Mel Gorman <mgorman@...e.de>, Greg Thelen <gthelen@...gle.com>,
	Michel Lespinasse <walken@...gle.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [patch 8/8] mm: make per-memcg lru lists exclusive

On Fri, Aug 12, 2011 at 10:08:18AM -0700, Ying Han wrote:
> On Fri, Aug 12, 2011 at 1:34 AM, Johannes Weiner <hannes@...xchg.org> wrote:
> > And in reality, we only care about properly memcg-unaccounting the
> > old lru state before we change pc->mem_cgroup, so this becomes
> >
> >        if (!PageLRU(page))
> >                 return;
> >        spin_lock_irqsave(&zone->lru_lock, flags);
> >         if (!PageCgroupUsed(pc))
> >                mem_cgroup_lru_del(page);
> >         spin_unlock_irqrestore(&zone->lru_lock, flags);
> >
> > I don't see why we should care if the page stays physically linked
> > to the list.
> 
> Can you clarify that?

Well, I don't see anything wrong with leaving it on the LRU.  We just
need to unaccount the page from pc->mem_cgroup's lru stats before the
page is charged, pc->mem_cgroup overwritten, and the account lost.

> > The handling after committing the charge becomes this:
> >
> > -       if (likely(!PageLRU(page)))
> > -               return;
> >        spin_lock_irqsave(&zone->lru_lock, flags);
> >         lru = page_lru(page);
> >        if (PageLRU(page) && !PageCgroupAcctLRU(pc)) {
> >                del_page_from_lru_list(zone, page, lru);
> >                add_page_to_lru_list(zone, page, lru);
> >        }
> >
> > If the page is not on the LRU, someone else will put it there and link
> > it up properly.  If it is on the LRU and already memcg-accounted then
> > it must be on the right lruvec as setting pc->mem_cgroup and PCG_USED
> > is properly ordered.  Otherwise, it has to be physically moved to the
> > correct lruvec and memcg-accounted for.
> 
> While working on the zone->lru_lock patch, i have been questioning myself on
> the PageLRU and PageCgroupAcctLRU bit. Here is my question:
> 
> It looks to me that PageLRU indicates the page is linked to per-zone lru
> list, and PageCgroupAcctLRU indicates the page is charged to a memcg and
> also linked to memcg's private lru list. All of these work nicely when we
> have both global and private (per-memcg) lru list, but i can not put them
> together after this patch.
> 
> Now page is linked to private lru always either memcg or root. While linked
> to either lru list, the page could be uncharged (like swapcache). No matter
> what, i am thinking whether or not we can get rid of the AcctLRU bit from pc
> and use LRU bit only here.

As I said above: if after the commit the page is on the LRU (PageLRU
set), pc->mem_cgroup's lru stats may or may not include the page, and
the page may or may not be on the right lruvec.

If someone had the page isolated (reclaim?) while we charge it and put
it back, the page may either be charged or uncharged at the time of
putback.

	unused: PageLRU is set, but page possibly on the wrong lruvec
	(root_mem_cgroup's per default, see mem_cgroup_lru_add_list)
	and not properly accounted for.  We can detect this case by
	seeing AcctLRU cleared.

	used: PageLRU is set, page on the right lruvec and properly
	accounted.  We can detect this case by seeing that
	mem_cgroup_lru_add_list() set AcctLRU.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/