All of lore.kernel.org
 help / color / mirror / Atom feed
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
To: Wu Fengguang <fengguang.wu@intel.com>
Cc: Greg Thelen <gthelen@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"containers@lists.osdl.org" <containers@lists.osdl.org>,
	Andrea Righi <arighi@develer.com>,
	Balbir Singh <balbir@linux.vnet.ibm.com>,
	Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>,
	Minchan Kim <minchan.kim@gmail.com>,
	Ciju Rajan K <ciju@linux.vnet.ibm.com>,
	David Rientjes <rientjes@google.com>
Subject: Re: [PATCH v4 06/11] memcg: add dirty page accounting infrastructure
Date: Fri, 29 Oct 2010 20:17:26 +0900	[thread overview]
Message-ID: <20101029201726.370e462b.kamezawa.hiroyu@jp.fujitsu.com> (raw)
In-Reply-To: <20101029111300.GB29774@localhost>

On Fri, 29 Oct 2010 19:13:00 +0800
Wu Fengguang <fengguang.wu@intel.com> wrote:

> On Fri, Oct 29, 2010 at 03:09:09PM +0800, Greg Thelen wrote:
> 
> > +
> > +	case MEMCG_NR_FILE_DIRTY:
> > +		/* Use Test{Set,Clear} to only un/charge the memcg once. */
> > +		if (val > 0) {
> > +			if (TestSetPageCgroupFileDirty(pc))
> > +				val = 0;
> > +		} else {
> > +			if (!TestClearPageCgroupFileDirty(pc))
> > +				val = 0;
> > +		}
> 
> I'm wondering why TestSet/TestClear and even the cgroup page flags for
> dirty/writeback/unstable pages are necessary at all (it helps to
> document in changelog if there are any). For example, VFS will call
> TestSetPageDirty() before calling
> mem_cgroup_inc_page_stat(MEMCG_NR_FILE_DIRTY), so there should be no
> chance of false double counting.
> 

1. flag is necessary for moving accounting information between cgroups
   when account_move() occurs.

2. TestSet... is required because there are always race with page_cgroup_lock()'s
   lock bit.

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>

WARNING: multiple messages have this Message-ID (diff)
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
To: Wu Fengguang <fengguang.wu@intel.com>
Cc: Greg Thelen <gthelen@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"containers@lists.osdl.org" <containers@lists.osdl.org>,
	Andrea Righi <arighi@develer.com>,
	Balbir Singh <balbir@linux.vnet.ibm.com>,
	Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>,
	Minchan Kim <minchan.kim@gmail.com>,
	Ciju Rajan K <ciju@linux.vnet.ibm.com>,
	David Rientjes <rientjes@google.com>
Subject: Re: [PATCH v4 06/11] memcg: add dirty page accounting infrastructure
Date: Fri, 29 Oct 2010 20:17:26 +0900	[thread overview]
Message-ID: <20101029201726.370e462b.kamezawa.hiroyu@jp.fujitsu.com> (raw)
In-Reply-To: <20101029111300.GB29774@localhost>

On Fri, 29 Oct 2010 19:13:00 +0800
Wu Fengguang <fengguang.wu@intel.com> wrote:

> On Fri, Oct 29, 2010 at 03:09:09PM +0800, Greg Thelen wrote:
> 
> > +
> > +	case MEMCG_NR_FILE_DIRTY:
> > +		/* Use Test{Set,Clear} to only un/charge the memcg once. */
> > +		if (val > 0) {
> > +			if (TestSetPageCgroupFileDirty(pc))
> > +				val = 0;
> > +		} else {
> > +			if (!TestClearPageCgroupFileDirty(pc))
> > +				val = 0;
> > +		}
> 
> I'm wondering why TestSet/TestClear and even the cgroup page flags for
> dirty/writeback/unstable pages are necessary at all (it helps to
> document in changelog if there are any). For example, VFS will call
> TestSetPageDirty() before calling
> mem_cgroup_inc_page_stat(MEMCG_NR_FILE_DIRTY), so there should be no
> chance of false double counting.
> 

1. flag is necessary for moving accounting information between cgroups
   when account_move() occurs.

2. TestSet... is required because there are always race with page_cgroup_lock()'s
   lock bit.

Thanks,
-Kame


  reply	other threads:[~2010-10-29 11:17 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-29  7:09 [PATCH v4 00/11] memcg: per cgroup dirty page accounting Greg Thelen
2010-10-29  7:09 ` Greg Thelen
2010-10-29  7:09 ` [PATCH v4 01/11] memcg: add page_cgroup flags for dirty page tracking Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29  7:09 ` [PATCH v4 02/11] memcg: document cgroup dirty memory interfaces Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29 11:03   ` Wu Fengguang
2010-10-29 11:03     ` Wu Fengguang
2010-10-29 21:35     ` Greg Thelen
2010-10-29 21:35       ` Greg Thelen
2010-10-30  3:02       ` Wu Fengguang
2010-10-30  3:02         ` Wu Fengguang
2010-10-29 20:19   ` Andrew Morton
2010-10-29 20:19     ` Andrew Morton
2010-10-29 21:37     ` Greg Thelen
2010-10-29 21:37       ` Greg Thelen
2010-10-29  7:09 ` [PATCH v4 03/11] memcg: create extensible page stat update routines Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-31 14:48   ` Ciju Rajan K
2010-10-31 14:48     ` Ciju Rajan K
2010-10-31 20:11     ` Greg Thelen
2010-10-31 20:11       ` Greg Thelen
2010-11-01 20:16       ` Ciju Rajan K
2010-11-01 20:16         ` Ciju Rajan K
2010-11-02 19:35       ` Ciju Rajan K
2010-11-02 19:35         ` Ciju Rajan K
2010-10-29  7:09 ` [PATCH v4 04/11] memcg: add lock to synchronize page accounting and migration Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29  7:09 ` [PATCH v4 05/11] writeback: create dirty_info structure Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29  7:50   ` KAMEZAWA Hiroyuki
2010-10-29  7:50     ` KAMEZAWA Hiroyuki
2010-11-18  0:49   ` Andrew Morton
2010-11-18  0:49     ` Andrew Morton
2010-11-18  0:50     ` Andrew Morton
2010-11-18  0:50       ` Andrew Morton
2010-11-18  0:50       ` Andrew Morton
2010-11-18  2:02     ` Greg Thelen
2010-11-18  2:02       ` Greg Thelen
2010-10-29  7:09 ` [PATCH v4 06/11] memcg: add dirty page accounting infrastructure Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29 11:13   ` Wu Fengguang
2010-10-29 11:13     ` Wu Fengguang
2010-10-29 11:17     ` KAMEZAWA Hiroyuki [this message]
2010-10-29 11:17       ` KAMEZAWA Hiroyuki
2010-10-29  7:09 ` [PATCH v4 07/11] memcg: add kernel calls for memcg dirty page stats Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29  7:09 ` [PATCH v4 08/11] memcg: add dirty limits to mem_cgroup Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29  7:41   ` KAMEZAWA Hiroyuki
2010-10-29  7:41     ` KAMEZAWA Hiroyuki
2010-10-29 16:00     ` Greg Thelen
2010-10-29 16:00       ` Greg Thelen
2010-10-29  7:09 ` [PATCH v4 09/11] memcg: CPU hotplug lockdep warning fix Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29 20:19   ` Andrew Morton
2010-10-29 20:19     ` Andrew Morton
2010-10-29  7:09 ` [PATCH v4 10/11] memcg: add cgroupfs interface to memcg dirty limits Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29  7:43   ` KAMEZAWA Hiroyuki
2010-10-29  7:43     ` KAMEZAWA Hiroyuki
2010-10-29  7:09 ` [PATCH v4 11/11] memcg: check memcg dirty limits in page writeback Greg Thelen
2010-10-29  7:09   ` Greg Thelen
2010-10-29  7:48   ` KAMEZAWA Hiroyuki
2010-10-29  7:48     ` KAMEZAWA Hiroyuki
2010-10-29 16:06     ` Greg Thelen
2010-10-29 16:06       ` Greg Thelen
2010-10-31 20:03       ` Wu Fengguang
2010-10-31 20:03         ` Wu Fengguang
2010-10-29 20:19 ` [PATCH v4 00/11] memcg: per cgroup dirty page accounting Andrew Morton
2010-10-29 20:19   ` Andrew Morton
2010-10-30 21:46   ` Greg Thelen
2010-10-30 21:46     ` Greg Thelen
2010-11-02 19:33     ` Ciju Rajan K
2010-11-02 19:33       ` Ciju Rajan K

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=20101029201726.370e462b.kamezawa.hiroyu@jp.fujitsu.com \
    --to=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=akpm@linux-foundation.org \
    --cc=arighi@develer.com \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=ciju@linux.vnet.ibm.com \
    --cc=containers@lists.osdl.org \
    --cc=fengguang.wu@intel.com \
    --cc=gthelen@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan.kim@gmail.com \
    --cc=nishimura@mxp.nes.nec.co.jp \
    --cc=rientjes@google.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.