linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>,
	Balbir Singh <bsingharora@gmail.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	cgroups@vger.kernel.org
Subject: Re: [RFC][PATCH] memcg: remove PCG_ACCT_LRU.
Date: Fri, 2 Dec 2011 13:08:49 +0100	[thread overview]
Message-ID: <20111202120849.GA1295@cmpxchg.org> (raw)
In-Reply-To: <20111202190622.8e0488d6.kamezawa.hiroyu@jp.fujitsu.com>

On Fri, Dec 02, 2011 at 07:06:22PM +0900, KAMEZAWA Hiroyuki wrote:
> I'm now testing this patch, removing PCG_ACCT_LRU, onto mmotm.
> How do you think ?

> @@ -1024,18 +1026,8 @@ void mem_cgroup_lru_del_list(struct page *page, enum lru_list lru)
>  		return;
>  
>  	pc = lookup_page_cgroup(page);
> -	/*
> -	 * root_mem_cgroup babysits uncharged LRU pages, but
> -	 * PageCgroupUsed is cleared when the page is about to get
> -	 * freed.  PageCgroupAcctLRU remembers whether the
> -	 * LRU-accounting happened against pc->mem_cgroup or
> -	 * root_mem_cgroup.
> -	 */
> -	if (TestClearPageCgroupAcctLRU(pc)) {
> -		VM_BUG_ON(!pc->mem_cgroup);
> -		memcg = pc->mem_cgroup;
> -	} else
> -		memcg = root_mem_cgroup;
> +	memcg = pc->mem_cgroup ? pc->mem_cgroup : root_mem_cgroup;
> +	VM_BUG_ON(memcg != pc->mem_cgroup_lru);
>  	mz = page_cgroup_zoneinfo(memcg, page);
>  	/* huge page split is done under lru_lock. so, we have no races. */
>  	MEM_CGROUP_ZSTAT(mz, lru) -= 1 << compound_order(page);

Nobody clears pc->mem_cgroup upon uncharge, so this may end up
mistakenly lru-unaccount a page that was never charged against the
stale pc->mem_cgroup (e.g. a swap readahead page that has not been
charged yet gets isolated by reclaim).

On the other hand, pages that were uncharged just before the lru_del
MUST be lru-unaccounted against pc->mem_cgroup.

PageCgroupAcctLRU made it possible to tell those two scenarios apart.

A possible solution could be to clear pc->mem_cgroup when the page is
finally freed so that only pages that have been charged since their
last allocation have pc->mem_cgroup set.  But this means that the page
freeing hotpath will have to grow a lookup_page_cgroup(), amortizing
the winnings at least to some extent.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-12-02 12:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-02 10:06 KAMEZAWA Hiroyuki
2011-12-02 12:08 ` Johannes Weiner [this message]
2011-12-05  0:50   ` KAMEZAWA Hiroyuki
2011-12-06  0:13     ` Hugh Dickins
2011-12-06  0:58       ` KAMEZAWA Hiroyuki
2011-12-06  7:36         ` Hugh Dickins
2011-12-06 10:21           ` KAMEZAWA Hiroyuki
2011-12-06 23:50             ` Hugh Dickins
2011-12-07  1:48               ` KAMEZAWA Hiroyuki
2011-12-07  6:30                 ` Hugh Dickins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111202120849.GA1295@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=bsingharora@gmail.com \
    --cc=cgroups@vger.kernel.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox