memory-hotplug: only drain LRU when failed to offline pages

PATCH In review Wu Fengguang 2 participants 3 messages Oct 25, 2010

Subsystems (2)

Memory management Memory hot(un)plug

lore

Share & tools

lore

Wu Fengguang

author

2010-10-25 05:12 UTC #0 lore raw

PATCH 1/1 memory-hotplug: only drain LRU when failed to offline pages

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 <[email protected]>

---
 mm/memory_hotplug.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

mm/memory_hotplug.c +10 ~~-14~~

@@ -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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

KAMEZAWA Hiroyuki

2010-10-25 05:20 UTC #1 lore raw

Quoted 2 lines · On Mon, 25 Oct 2010 13:12:02 +0800 Wu Fengguang <[email protected]> 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.

Quoted 3 lines

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.

Quoted 1 line

Signed-off-by Wu Fengguang <[email protected]>

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

Quoted 24 lines

---
 mm/memory_hotplug.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

mm/memory_hotplug.c +1 -2

@@ -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 */

mm/memory_hotplug.c repeat: · line 843

Quoted 6 lines

mm/memory_hotplug.c +0 -3

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

Quoted 8 lines

mm/memory_hotplug.c +0 -4

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

mm/memory_hotplug.c repeat: · line 851

Quoted 10 lines

mm/memory_hotplug.c +2 -2

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

mm/memory_hotplug.c repeat: · line 853

Quoted 2 lines

mm/memory_hotplug.c +2 -0

+			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.)

mm/memory_hotplug.c repeat: · line 860

Quoted 8 lines

mm/memory_hotplug.c +2 -1

 			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

Wu Fengguang

author

2010-10-25 05:28 UTC #2 lore raw

Quoted 7 lines · KAMEZAWA Hiroyuki

Quoted 2 lines · On Mon, 25 Oct 2010 13:12:02 +0800 Wu Fengguang <[email protected]> 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).

Quoted 5 lines

Quoted 3 lines
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..

Quoted 3 lines

Quoted 1 line
Signed-off-by Wu Fengguang <[email protected]>
Sorry, this will HUNK. Could you wait until the end of merge window ?

OK, I'll resend.

Quoted 18 lines

---
 mm/memory_hotplug.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

mm/memory_hotplug.c +1 -1

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

Quoted 15 lines

mm/memory_hotplug.c repeat: · line 843

Quoted 13 lines

mm/memory_hotplug.c +0 -4

 	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.

Quoted 11 lines

mm/memory_hotplug.c +0 -5

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

Quoted 8 lines

mm/memory_hotplug.c repeat: · line 851

Quoted 7 lines

mm/memory_hotplug.c +2 -1

 			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.

Quoted 8 lines

mm/memory_hotplug.c repeat: · line 853
Quoted 2 lines
mm/memory_hotplug.c +2 -0
+			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.

Quoted 9 lines

mm/memory_hotplug.c repeat: · line 860
Quoted 8 lines
mm/memory_hotplug.c +2 -1
 			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