linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] memory-hotplug: only drain LRU when failed to offline pages
@ 2010-10-25  5:12 Wu Fengguang
  2010-10-25  5:15 ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 3+ messages in thread
From: Wu Fengguang @ 2010-10-25  5:12 UTC (permalink / raw)
  To: Andrew Morton, KAMEZAWA Hiroyuki
  Cc: Bob Liu, linux-mm@kvack.org, mel@csn.ul.ie,
	kosaki.motohiro@jp.fujitsu.com

do_migrate_range() offlines 1MB pages at one time and hence might be
called up to 16000 times when trying to offline 16GB memory. It makes
sense to avoid sending the costly IPIs to drain pages on all LRU for
the 99% cases that do_migrate_range() succeeds offlining some pages.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 mm/memory_hotplug.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

--- linux-next.orig/mm/memory_hotplug.c	2010-10-25 11:20:47.000000000 +0800
+++ linux-next/mm/memory_hotplug.c	2010-10-25 13:07:10.000000000 +0800
@@ -788,7 +788,7 @@ static int offline_pages(unsigned long s
 {
 	unsigned long pfn, nr_pages, expire;
 	long offlined_pages;
-	int ret, drain, retry_max, node;
+	int ret, retry_max, node;
 	struct zone *zone;
 	struct memory_notify arg;
 
@@ -827,7 +827,6 @@ static int offline_pages(unsigned long s
 
 	pfn = start_pfn;
 	expire = jiffies + timeout;
-	drain = 0;
 	retry_max = 5;
 repeat:
 	/* start memory hot removal */
@@ -838,34 +837,31 @@ repeat:
 	if (signal_pending(current))
 		goto failed_removal;
 	ret = 0;
-	if (drain) {
-		lru_add_drain_all();
-		flush_scheduled_work();
-		cond_resched();
-		drain_all_pages();
-	}
-
 	pfn = scan_lru_pages(start_pfn, end_pfn);
 	if (pfn) { /* We have page on LRU */
 		ret = do_migrate_range(pfn, end_pfn);
 		if (!ret) {
-			drain = 1;
 			goto repeat;
 		} else {
 			if (ret < 0)
 				if (--retry_max == 0)
 					goto failed_removal;
 			yield();
-			drain = 1;
+			lru_add_drain_all();
+			flush_scheduled_work();
+			cond_resched();
+			drain_all_pages();
 			goto repeat;
 		}
 	}
-	/* drain all zone's lru pagevec, this is asyncronous... */
+
+	/* drain all zone's lru pagevec, this is asynchronous... */
 	lru_add_drain_all();
 	flush_scheduled_work();
 	yield();
-	/* drain pcp pages , this is synchrouns. */
+	/* drain pcp pages , this is asynchronous. */
 	drain_all_pages();
+
 	/* check again */
 	offlined_pages = check_pages_isolated(start_pfn, end_pfn);
 	if (offlined_pages < 0) {
@@ -873,7 +869,7 @@ repeat:
 		goto failed_removal;
 	}
 	printk(KERN_INFO "Offlined Pages %ld\n", offlined_pages);
-	/* Ok, all of our target is islaoted.
+	/* Ok, all of our target is isolated.
 	   We cannot do rollback at this point. */
 	offline_isolated_pages(start_pfn, end_pfn);
 	/* reset pagetype flags and makes migrate type to be MOVABLE */

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] memory-hotplug: only drain LRU when failed to offline pages
  2010-10-25  5:12 [PATCH] memory-hotplug: only drain LRU when failed to offline pages Wu Fengguang
@ 2010-10-25  5:15 ` KAMEZAWA Hiroyuki
  2010-10-25  5:28   ` Wu Fengguang
  0 siblings, 1 reply; 3+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-10-25  5:15 UTC (permalink / raw)
  To: Wu Fengguang
  Cc: Andrew Morton, Bob Liu, linux-mm@kvack.org, mel@csn.ul.ie,
	kosaki.motohiro@jp.fujitsu.com

On Mon, 25 Oct 2010 13:12:02 +0800
Wu Fengguang <fengguang.wu@intel.com> wrote:

> do_migrate_range() offlines 1MB pages at one time and hence might be
> called up to 16000 times when trying to offline 16GB memory. 

But size of memory section is not such big.

> It makes sense to avoid sending the costly IPIs to drain pages on all LRU for
> the 99% cases that do_migrate_range() succeeds offlining some pages.
> 

did you test ? I think this patch should be tested by IBM guys.

> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>

Sorry, this will HUNK. Could you wait until the end of merge window ?


> ---
>  mm/memory_hotplug.c |   24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> --- linux-next.orig/mm/memory_hotplug.c	2010-10-25 11:20:47.000000000 +0800
> +++ linux-next/mm/memory_hotplug.c	2010-10-25 13:07:10.000000000 +0800
> @@ -788,7 +788,7 @@ static int offline_pages(unsigned long s
>  {
>  	unsigned long pfn, nr_pages, expire;
>  	long offlined_pages;
> -	int ret, drain, retry_max, node;
> +	int ret, retry_max, node;
>  	struct zone *zone;
>  	struct memory_notify arg;
>  
> @@ -827,7 +827,6 @@ static int offline_pages(unsigned long s
>  
>  	pfn = start_pfn;
>  	expire = jiffies + timeout;
> -	drain = 0;
>  	retry_max = 5;
>  repeat:
>  	/* start memory hot removal */
> @@ -838,34 +837,31 @@ repeat:
>  	if (signal_pending(current))
>  		goto failed_removal;
>  	ret = 0;
> -	if (drain) {
> -		lru_add_drain_all();
> -		flush_scheduled_work();

this flush_scheduled_work() is removed in recent work of Tejun Heo.

> -		cond_resched();
> -		drain_all_pages();
> -	}
> -
>  	pfn = scan_lru_pages(start_pfn, end_pfn);
>  	if (pfn) { /* We have page on LRU */
>  		ret = do_migrate_range(pfn, end_pfn);
>  		if (!ret) {
> -			drain = 1;
>  			goto repeat;
>  		} else {
>  			if (ret < 0)
>  				if (--retry_max == 0)
>  					goto failed_removal;
>  			yield();
> -			drain = 1;
> +			lru_add_drain_all();
> +			flush_scheduled_work();
This flush is unnecessary.

> +			cond_resched();
> +			drain_all_pages();

I think followin is  better order.

drain_all_pages();      # SEND IPI and asynchronous.
lru_add_drain_pages();  # call schedule_work ony by one and it's synchronous.
cond_resched();	# may not be unnecessary (lru_add_drain_pages() will sleep.)

>  			goto repeat;
>  		}
>  	}
> -	/* drain all zone's lru pagevec, this is asyncronous... */
> +
> +	/* drain all zone's lru pagevec, this is asynchronous... */
>  	lru_add_drain_all();
>  	flush_scheduled_work();
This flush() is dropped by recent works of Tejun Heo's workqueue updates.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] memory-hotplug: only drain LRU when failed to offline pages
  2010-10-25  5:15 ` KAMEZAWA Hiroyuki
@ 2010-10-25  5:28   ` Wu Fengguang
  0 siblings, 0 replies; 3+ messages in thread
From: Wu Fengguang @ 2010-10-25  5:28 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, Bob Liu, linux-mm@kvack.org, mel@csn.ul.ie,
	kosaki.motohiro@jp.fujitsu.com

On Mon, Oct 25, 2010 at 01:15:19PM +0800, KAMEZAWA Hiroyuki wrote:
> On Mon, 25 Oct 2010 13:12:02 +0800
> Wu Fengguang <fengguang.wu@intel.com> wrote:
> 
> > do_migrate_range() offlines 1MB pages at one time and hence might be
> > called up to 16000 times when trying to offline 16GB memory. 
> 
> But size of memory section is not such big.

It's NR_OFFLINE_AT_ONCE_PAGES=256 (1MB).

> > It makes sense to avoid sending the costly IPIs to drain pages on all LRU for
> > the 99% cases that do_migrate_range() succeeds offlining some pages.
> > 
> 
> did you test ? I think this patch should be tested by IBM guys.

Only compile tested..

> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> 
> Sorry, this will HUNK. Could you wait until the end of merge window ?

OK, I'll resend.

> 
> > ---
> >  mm/memory_hotplug.c |   24 ++++++++++--------------
> >  1 file changed, 10 insertions(+), 14 deletions(-)
> > 
> > --- linux-next.orig/mm/memory_hotplug.c	2010-10-25 11:20:47.000000000 +0800
> > +++ linux-next/mm/memory_hotplug.c	2010-10-25 13:07:10.000000000 +0800
> > @@ -788,7 +788,7 @@ static int offline_pages(unsigned long s
> >  {
> >  	unsigned long pfn, nr_pages, expire;
> >  	long offlined_pages;
> > -	int ret, drain, retry_max, node;
> > +	int ret, retry_max, node;
> >  	struct zone *zone;
> >  	struct memory_notify arg;
> >  
> > @@ -827,7 +827,6 @@ static int offline_pages(unsigned long s
> >  
> >  	pfn = start_pfn;
> >  	expire = jiffies + timeout;
> > -	drain = 0;
> >  	retry_max = 5;
> >  repeat:
> >  	/* start memory hot removal */
> > @@ -838,34 +837,31 @@ repeat:
> >  	if (signal_pending(current))
> >  		goto failed_removal;
> >  	ret = 0;
> > -	if (drain) {
> > -		lru_add_drain_all();
> > -		flush_scheduled_work();
> 
> this flush_scheduled_work() is removed in recent work of Tejun Heo.

Ah yes.

> > -		cond_resched();
> > -		drain_all_pages();
> > -	}
> > -
> >  	pfn = scan_lru_pages(start_pfn, end_pfn);
> >  	if (pfn) { /* We have page on LRU */
> >  		ret = do_migrate_range(pfn, end_pfn);
> >  		if (!ret) {
> > -			drain = 1;
> >  			goto repeat;
> >  		} else {
> >  			if (ret < 0)
> >  				if (--retry_max == 0)
> >  					goto failed_removal;
> >  			yield();
> > -			drain = 1;
> > +			lru_add_drain_all();
> > +			flush_scheduled_work();
> This flush is unnecessary.

OK. 

> > +			cond_resched();
> > +			drain_all_pages();
> 
> I think followin is  better order.
> 
> drain_all_pages();      # SEND IPI and asynchronous.
> lru_add_drain_pages();  # call schedule_work ony by one and it's synchronous.
> cond_resched();	# may not be unnecessary (lru_add_drain_pages() will sleep.)

That looks better. I'll remove cond_resched() too.

> >  			goto repeat;
> >  		}
> >  	}
> > -	/* drain all zone's lru pagevec, this is asyncronous... */
> > +
> > +	/* drain all zone's lru pagevec, this is asynchronous... */
> >  	lru_add_drain_all();
> >  	flush_scheduled_work();
> This flush() is dropped by recent works of Tejun Heo's workqueue updates.

OK.

Thanks,
Fengguang

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-10-25  5:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-25  5:12 [PATCH] memory-hotplug: only drain LRU when failed to offline pages Wu Fengguang
2010-10-25  5:15 ` KAMEZAWA Hiroyuki
2010-10-25  5:28   ` Wu Fengguang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).