linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
To: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Hugh Dickins <hugh@veritas.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"balbir@linux.vnet.ibm.com" <balbir@linux.vnet.ibm.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Subject: Re: [PATCH mmotm] memcg: fix for documentation (Re: [BUGFIX][PATCH mmotm] memcg fix swap accounting leak (v3))
Date: Tue, 16 Dec 2008 13:53:10 +0900	[thread overview]
Message-ID: <20081216135310.9bef1c2d.kamezawa.hiroyu@jp.fujitsu.com> (raw)
In-Reply-To: <20081216130230.2978b8fc.nishimura@mxp.nes.nec.co.jp>

On Tue, 16 Dec 2008 13:02:30 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> Sorry for late reply.
> 
> > @@ -111,9 +111,40 @@ Under below explanation, we assume CONFI
> >  	(b) If the SwapCache has been mapped by processes, it has been
> >  	    charged already.
> >  
> > -	In case (a), we charge it. In case (b), we don't charge it.
> > -	(But racy state between (a) and (b) exists. We do check it.)
> > -	At charging, a charge recorded in swap_cgroup is moved to page_cgroup.
> > +	This swap-in is one of the most complicated work. In do_swap_page(),
> > +	following events occur when pte is unchanged.
> > +
> > +	(1) the page (SwapCache) is looked up.
> > +	(2) lock_page()
> > +	(3) try_charge_swapin()
> > +	(4) reuse_swap_page() (may call delete_swap_cache())
> > +	(5) commit_charge_swapin()
> > +	(6) swap_free().
> > +
> > +	Considering following situation for example.
> > +
> > +	(A) The page has not been charged before (2) and reuse_swap_page()
> > +	    doesn't call delete_from_swap_cache().
> > +	(B) The page has not been charged before (2) and reuse_swap_page()
> > +	    calls delete_from_swap_cache().
> > +	(C) The page has been charged before (2) and reuse_swap_page() doesn't
> > +	    call delete_from_swap_cache().
> > +	(D) The page has been charged before (2) and reuse_swap_page() calls
> > +	    delete_from_swap_cache().
> > +
> > +	    memory.usage/memsw.usage changes to this page/swp_entry will be
> > +	 Case          (A)      (B)       (C)     (D)
> > +         Event
> > +       Before (2)     0/ 1     0/ 1      1/ 1    1/ 1
> > +          ===========================================
> > +          (3)        +1/+1    +1/+1     +1/+1   +1/+1
> > +          (4)          -       0/ 0       -     -1/ 0
> > +          (5)         0/ 1     0/-1     -1/-1    0/ 0
> > +          (6)          -        -         -      0/-1
> > +          ===========================================
> > +       Result         1/ 1     1/1       1/ 1    1/ 1
> > +
> > +       In any cases, charges to this page should be 1/ 1.
> >  
> I've verified that charges will result in valid values by tracing source code
> in all of these cases, but in case of (B) I don't think commit_charge_swapin
> does memsw-- because PageSwapCache has been cleared already. swap_free does
> memsw-- in this case.
> 
> I attached a fix patch.
> 
you're right. it comes from this version's new change....sorry.

Acked-by; KAMEZAWA Hiroyuki <kamezaw.hiroyu@jp.fujitsu.com>


> Thanks,
> Daisuke Nishimura.
> 
> ===
> From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> 
> fix for documentation.
> 
> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> ---
>  Documentation/controllers/memcg_test.txt |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/controllers/memcg_test.txt b/Documentation/controllers/memcg_test.txt
> index 3c1458a..08d4d3e 100644
> --- a/Documentation/controllers/memcg_test.txt
> +++ b/Documentation/controllers/memcg_test.txt
> @@ -139,10 +139,10 @@ Under below explanation, we assume CONFIG_MEM_RES_CTRL_SWAP=y.
>            ===========================================
>            (3)        +1/+1    +1/+1     +1/+1   +1/+1
>            (4)          -       0/ 0       -     -1/ 0
> -          (5)         0/ 1     0/-1     -1/-1    0/ 0
> -          (6)          -        -         -      0/-1
> +          (5)         0/-1     0/ 0     -1/-1    0/ 0
> +          (6)          -       0/-1       -      0/-1
>            ===========================================
> -       Result         1/ 1     1/1       1/ 1    1/ 1
> +       Result         1/ 1     1/ 1      1/ 1    1/ 1
>  
>         In any cases, charges to this page should be 1/ 1.
>  
> 

--
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>

  reply	other threads:[~2008-12-16  4:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-12  8:29 [BUGFIX][PATCH mmotm] memcg fix swap accounting leak KAMEZAWA Hiroyuki
2008-12-12  9:43 ` Daisuke Nishimura
2008-12-12 11:16   ` KAMEZAWA Hiroyuki
2008-12-13  7:03     ` [BUGFIX][PATCH mmotm] memcg fix swap accounting leak (v2) KAMEZAWA Hiroyuki
2008-12-13  9:49       ` Hugh Dickins
2008-12-13 10:27         ` KAMEZAWA Hiroyuki
2008-12-15  7:07           ` [BUGFIX][PATCH mmotm] memcg fix swap accounting leak (v3) KAMEZAWA Hiroyuki
2008-12-15  8:37             ` Balbir Singh
2008-12-15  8:40               ` KAMEZAWA Hiroyuki
2008-12-15 10:34             ` Hugh Dickins
2008-12-16  4:02             ` [PATCH mmotm] memcg: fix for documentation (Re: [BUGFIX][PATCH mmotm] memcg fix swap accounting leak (v3)) Daisuke Nishimura
2008-12-16  4:53               ` KAMEZAWA Hiroyuki [this message]
2008-12-13 10:38         ` [BUGFIX][PATCH mmotm] memcg fix swap accounting leak (v2) KAMEZAWA Hiroyuki

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=20081216135310.9bef1c2d.kamezawa.hiroyu@jp.fujitsu.com \
    --to=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=akpm@linux-foundation.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=hugh@veritas.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nishimura@mxp.nes.nec.co.jp \
    /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