All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex,Shi" <alex.shi@intel.com>
To: Mel Gorman <mgorman@suse.de>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
	"P@draigBrady.com" <P@draigBrady.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"andrea@cpushare.com" <andrea@cpushare.com>,
	"Chen, Tim C" <tim.c.chen@intel.com>,
	"Li, Shaohua" <shaohua.li@intel.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"riel@redhat.com" <riel@redhat.com>,
	"luto@mit.edu" <luto@mit.edu>
Subject: Re: [PATCH] kswapd: avoid unnecessary rebalance after an unsuccessful balancing
Date: Mon, 01 Aug 2011 08:45:17 +0800	[thread overview]
Message-ID: <1312159517.27358.2446.camel@debian> (raw)
In-Reply-To: <20110729154031.GV3010@suse.de>

On Fri, 2011-07-29 at 23:40 +0800, Mel Gorman wrote:
> On Fri, Jul 29, 2011 at 11:23:10PM +0800, Alex Shi wrote:
> > In commit 215ddd66, Mel Gorman said kswapd is better to sleep after a
> > unsuccessful balancing if there is tighter reclaim request pending in
> > the balancing. In this scenario, the 'order' and 'classzone_idx'
> > that are checked for tighter request judgment is incorrect, since they
> > aren't the one kswapd should read from new pgdat, but the last time pgdat
> > value for just now balancing. Then kswapd will skip try_to_sleep func
> > and rebalance the last pgdat request. It's not our expected behavior.
> > 
> > So, I added new variables to distinguish the returned order/classzone_idx
> > from last balancing, that can resolved above issue in that scenario.
> > 
> 
> I'm afraid this changelog is very difficult to read and I do not see
> what problem you are trying to solve and I do not see what this patch
> might solve.
> 
> When balance_pgdat() returns with a lower classzone or order, the values
> stored in pgdat are not re-read and instead it tries to go to sleep
> based on the starting request. Something like;

Thanks for your comments, I will use this comments style next time, list
request A, B etc. 

> 
> 1. Read pgdat request A (classzone_idx, order)

Assume the order of A > 0, like is 3.

> 2. balance_pgdat()
> 3.	During pgdat, a new pgdat request B (classzone_idx, order) is placed
> 4. balance_pgdat() returns but failed so classzone_idx is lower

Another balance_pgdat() failure indicate is returned order == 0, am I
right?  If so, the next step of kswapd is not trying to sleep, but do
request A balance again.  And I thought this behavior doesn't match the
comments in kswapd. 

> 5. Try to sleep based on pgdat request A 

> 
> i.e. pgdat request B is not read and there is a comment explaining
> why pgdat request B is not read after balance_pgdat() fails.
> 
> This patch adds some variables that might improve the readability
> for some people but otherwise I can't see what problem is being
> fixed. What did I miss?
> 



WARNING: multiple messages have this Message-ID (diff)
From: "Alex,Shi" <alex.shi@intel.com>
To: Mel Gorman <mgorman@suse.de>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
	"P@draigBrady.com" <P@draigBrady.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"andrea@cpushare.com" <andrea@cpushare.com>,
	"Chen, Tim C" <tim.c.chen@intel.com>,
	"Li, Shaohua" <shaohua.li@intel.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"riel@redhat.com" <riel@redhat.com>,
	"luto@mit.edu" <luto@mit.edu>
Subject: Re: [PATCH] kswapd: avoid unnecessary rebalance after an unsuccessful balancing
Date: Mon, 01 Aug 2011 08:45:17 +0800	[thread overview]
Message-ID: <1312159517.27358.2446.camel@debian> (raw)
In-Reply-To: <20110729154031.GV3010@suse.de>

On Fri, 2011-07-29 at 23:40 +0800, Mel Gorman wrote:
> On Fri, Jul 29, 2011 at 11:23:10PM +0800, Alex Shi wrote:
> > In commit 215ddd66, Mel Gorman said kswapd is better to sleep after a
> > unsuccessful balancing if there is tighter reclaim request pending in
> > the balancing. In this scenario, the 'order' and 'classzone_idx'
> > that are checked for tighter request judgment is incorrect, since they
> > aren't the one kswapd should read from new pgdat, but the last time pgdat
> > value for just now balancing. Then kswapd will skip try_to_sleep func
> > and rebalance the last pgdat request. It's not our expected behavior.
> > 
> > So, I added new variables to distinguish the returned order/classzone_idx
> > from last balancing, that can resolved above issue in that scenario.
> > 
> 
> I'm afraid this changelog is very difficult to read and I do not see
> what problem you are trying to solve and I do not see what this patch
> might solve.
> 
> When balance_pgdat() returns with a lower classzone or order, the values
> stored in pgdat are not re-read and instead it tries to go to sleep
> based on the starting request. Something like;

Thanks for your comments, I will use this comments style next time, list
request A, B etc. 

> 
> 1. Read pgdat request A (classzone_idx, order)

Assume the order of A > 0, like is 3.

> 2. balance_pgdat()
> 3.	During pgdat, a new pgdat request B (classzone_idx, order) is placed
> 4. balance_pgdat() returns but failed so classzone_idx is lower

Another balance_pgdat() failure indicate is returned order == 0, am I
right?  If so, the next step of kswapd is not trying to sleep, but do
request A balance again.  And I thought this behavior doesn't match the
comments in kswapd. 

> 5. Try to sleep based on pgdat request A 

> 
> i.e. pgdat request B is not read and there is a comment explaining
> why pgdat request B is not read after balance_pgdat() fails.
> 
> This patch adds some variables that might improve the readability
> for some people but otherwise I can't see what problem is being
> fixed. What did I miss?
> 


--
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-08-01  0:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-29 15:23 [PATCH] kswapd: avoid unnecessary rebalance after an unsuccessful balancing Alex Shi
2011-07-29 15:23 ` Alex Shi
2011-07-29 15:40 ` Mel Gorman
2011-07-29 15:40   ` Mel Gorman
2011-08-01  0:45   ` Alex,Shi [this message]
2011-08-01  0:45     ` Alex,Shi
2011-08-02 10:22     ` Mel Gorman
2011-08-02 10:22       ` Mel Gorman
2011-08-03  0:49       ` Alex,Shi
2011-08-03  0:49         ` Alex,Shi
2011-07-29 18:21 ` Pádraig Brady
2011-07-29 18:21   ` Pádraig Brady

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=1312159517.27358.2446.camel@debian \
    --to=alex.shi@intel.com \
    --cc=P@draigBrady.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrea@cpushare.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@mit.edu \
    --cc=mgorman@suse.de \
    --cc=riel@redhat.com \
    --cc=shaohua.li@intel.com \
    --cc=tim.c.chen@intel.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.