Lumpy Reclaim V3

This is a repost of the lumpy reclaim patch set. This is basically unchanged from the last post, other than being rebased to 2.6.19-rc2-mm2. This has passed basic stress testing on a range of machines here.

[Sorry for the delay reposting, I had a test failure and needed to confirm it was not due to lumpy before posting.]

As before, I have left the changes broken out for the time being so as to make it clear what is the original and what is my fault. I would expect to roll this up before acceptance.

-apw

=== 8< ===
Lumpy Reclaim (V3)

When we are out of memory of a suitable size we enter reclaim. The current reclaim algorithm targets pages in LRU order, which is great for fairness but highly unsuitable if you desire pages at higher orders. To get pages of higher order we must shoot down a very high proportion of memory; >95% in a lot of cases.

This patch set adds a lumpy reclaim algorithm to the allocator. It targets groups of pages at the specified order anchored at the end of the active and inactive lists. This encourages groups of pages at the requested orders to move from active to inactive, and active to free lists. This behaviour is only triggered out of direct reclaim when higher order pages have been requested.

This patch set is particularly effective when utilised with an anti-fragmentation scheme which groups pages of similar reclaimability together.

This patch set (against 2.6.19-rc6-mm2) is based on Peter Zijlstra's lumpy reclaim V2 patch which forms the foundation. It comprises the following patches:

lumpy-reclaim-v2 -- Peter Zijlstra's lumpy reclaim prototype,

lumpy-cleanup-a-missplaced-comment-and-simplify-some-code --
cleanups to move a comment back to where it came from, to make the area edge selection more comprehensible and also cleans up the switch coding style to match the concensus in mm/*.c,

lumpy-ensure-we-respect-zone-boundaries -- bug fix to ensure we do
not attempt to take pages from adjacent zones, and

lumpy-take-the-other-active-inactive-pages-in-the-area -- patch to
increase aggression over the targetted order.

Testing of this patch set under high fragmentation high allocation load conditions shows significantly improved high order reclaim rates than a standard kernel. The stack here it is now within 5% of the best case linear-reclaim figures.

It would be interesting to see if this setup is also successful in reducing order-2 allocation failures that you have been seeing with jumbo frames.

Please consider for -mm.

-apw
(lumpy-v3r7)

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

lumpy reclaim v2

When trying to reclaim pages for a higher order allocation, make reclaim try to move lumps of pages (fitting the requested order) about, instead of single pages. This should significantly reduce the number of reclaimed pages for higher order allocations.

---

index c953c15..2f8b073 100644
@@ -374,7 +374,7 @@ static void free_more_memory(void)
 	for_each_online_pgdat(pgdat) {
 		zones = pgdat->node_zonelists[gfp_zone(GFP_NOFS)].zones;
 		if (*zones)
-			try_to_free_pages(zones, GFP_NOFS);
+			try_to_free_pages(zones, 0, GFP_NOFS);
 	}
 }
 

index 439f9a8..5c26736 100644
@@ -187,7 +187,7 @@ extern int rotate_reclaimable_page(struct page *page);
 extern void swap_setup(void);
 
 /* linux/mm/vmscan.c */
-extern unsigned long try_to_free_pages(struct zone **, gfp_t);
+extern unsigned long try_to_free_pages(struct zone **, int, gfp_t);
 extern unsigned long shrink_all_memory(unsigned long nr_pages);
 extern int vm_swappiness;
 extern int remove_mapping(struct address_space *mapping, struct page *page);

index 7938e46..78801c2 100644
@@ -1371,7 +1371,7 @@ nofail_alloc:
 	reclaim_state.reclaimed_slab = 0;
 	p->reclaim_state = &reclaim_state;
 
-	did_some_progress = try_to_free_pages(zonelist->zones, gfp_mask);
+	did_some_progress = try_to_free_pages(zonelist->zones, order, gfp_mask);
 
 	p->reclaim_state = NULL;
 

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

Poke. I'm still a no-compile on ia64.

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

Move the comment for isolate_lru_pages() back to its function and comment the new function. Add some running commentry on the area scan. Cleanup the indentation on switch to match the majority view in mm/*. Finally, clarify the boundary pfn calculations.

---

index 0f2d961..0effa3e 100644
@@ -606,21 +606,14 @@ keep:
 }
 
 /*
- * zone->lru_lock is heavily contended.  Some of the functions that
- * shrink the lists perform better by taking out a batch of pages
- * and working on them outside the LRU lock.
+ * Attempt to remove the specified page from its LRU.  Only take this
+ * page if it is of the appropriate PageActive status.  Pages which
+ * are being freed elsewhere are also ignored.
  *
- * For pagecache intensive workloads, this function is the hottest
- * spot in the kernel (apart from copy_*_user functions).
- *
- * Appropriate locks must be held before calling this function.
+ * @page:	page to consider
+ * @active:	active/inactive flag only take pages of this type
  *
- * @nr_to_scan:	The number of pages to look through on the list.
- * @src:	The LRU list to pull pages off.
- * @dst:	The temp list to put pages on to.
- * @scanned:	The number of pages that were scanned.
- *
- * returns how many pages were moved onto *@dst.
+ * returns 0 on success, -ve errno on failure.
  */
 int __isolate_lru_page(struct page *page, int active)
 {

@@ -642,6 +635,23 @@ int __isolate_lru_page(struct page *page, int active)
 	return ret;
 }
 
+/*
+ * zone->lru_lock is heavily contended.  Some of the functions that
+ * shrink the lists perform better by taking out a batch of pages
+ * and working on them outside the LRU lock.
+ *
+ * For pagecache intensive workloads, this function is the hottest
+ * spot in the kernel (apart from copy_*_user functions).
+ *
+ * Appropriate locks must be held before calling this function.
+ *
+ * @nr_to_scan:	The number of pages to look through on the list.
+ * @src:	The LRU list to pull pages off.
+ * @dst:	The temp list to put pages on to.
+ * @scanned:	The number of pages that were scanned.
+ *
+ * returns how many pages were moved onto *@dst.
+ */
 static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 		struct list_head *src, struct list_head *dst,
 		unsigned long *scanned, int order)

@@ -659,26 +669,31 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 
 		active = PageActive(page);
 		switch (__isolate_lru_page(page, active)) {
-			case 0:
-				list_move(&page->lru, dst);
-				nr_taken++;
-				break;
+		case 0:
+			list_move(&page->lru, dst);
+			nr_taken++;
+			break;
 
-			case -EBUSY:
-				/* else it is being freed elsewhere */
-				list_move(&page->lru, src);
-				continue;
+		case -EBUSY:
+			/* else it is being freed elsewhere */
+			list_move(&page->lru, src);
+			continue;
 
-			default:
-				BUG();
+		default:
+			BUG();
 		}
 
 		if (!order)
 			continue;
 
-		page_pfn = pfn = __page_to_pfn(page);
-		end_pfn = pfn &= ~((1 << order) - 1);
-		end_pfn += 1 << order;
+		/*
+		 * Attempt to take all pages in the order aligned region
+		 * surrounding the tag page.  Only take those pages of
+		 * the same active state as that tag page.
+		 */
+		page_pfn = __page_to_pfn(page);
+		pfn = page_pfn & ~((1 << order) - 1);
+		end_pfn = pfn + (1 << order);
 		for (; pfn < end_pfn; pfn++) {
 			if (unlikely(pfn == page_pfn))
 				continue;

@@ -688,17 +703,16 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 			scan++;
 			tmp = __pfn_to_page(pfn);
 			switch (__isolate_lru_page(tmp, active)) {
-				case 0:
-					list_move(&tmp->lru, dst);
-					nr_taken++;
-					continue;
-
-				case -EBUSY:
-					/* else it is being freed elsewhere */
-					list_move(&tmp->lru, src);
-				default:
-					break;
+			case 0:
+				list_move(&tmp->lru, dst);
+				nr_taken++;
+				continue;
 
+			case -EBUSY:
+				/* else it is being freed elsewhere */
+				list_move(&tmp->lru, src);
+			default:
+				break;
 			}
 			break;
 		}

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

When scanning an aligned order N area ensure we only pull out pages in the same zone as our tag page, else we will manipulate those pages' LRU under the wrong zone lru_lock. Bad.

---

index 0effa3e..85f626b 100644
@@ -660,6 +660,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 	struct page *page, *tmp;
 	unsigned long scan, pfn, end_pfn, page_pfn;
 	int active;
+	int zone_id;
 
 	for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) {
 		page = lru_to_page(src);

@@ -691,6 +692,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 		 * surrounding the tag page.  Only take those pages of
 		 * the same active state as that tag page.
 		 */
+		zone_id = page_zone_id(page);
 		page_pfn = __page_to_pfn(page);
 		pfn = page_pfn & ~((1 << order) - 1);
 		end_pfn = pfn + (1 << order);

@@ -700,8 +702,10 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 			if (unlikely(!pfn_valid(pfn)))
 				break;
 
-			scan++;
 			tmp = __pfn_to_page(pfn);
+			if (unlikely(page_zone_id(tmp) != zone_id))
+				continue;
+			scan++;
 			switch (__isolate_lru_page(tmp, active)) {
 			case 0:
 				list_move(&tmp->lru, dst);

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

When we scan an order N aligned area around our tag page take any other pages with a matching active state to that of the tag page. This will tend to demote areas of the order we are interested from the active list to the inactive list and from the end of the inactive list, increasing the chances of such areas coming free together.

---

index 85f626b..fc23d87 100644
@@ -710,7 +710,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 			case 0:
 				list_move(&tmp->lru, dst);
 				nr_taken++;
-				continue;
+				break;
 
 			case -EBUSY:
 				/* else it is being freed elsewhere */

@@ -718,7 +718,6 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 			default:
 				break;
 			}
-			break;
 		}
 	}
 

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

The patch sequencing appeared to be designed to make the code hard to review, so I clumped them all into a single diff:

I dunno who started adding these @'s into non-kernel-doc comments. I'll un-add them.

			if (expr)
				continue;

With the code as it is in this patch this is safe as there is an uncommented assumption that the active parameter is actually also the return from a call to PageActive and therefore should be comparible regardless of value. However, as you also point out elsewhere we are in fact looking that active value up every time we spin the search loop, firstly doing it loads more than required, and second potentially when unlucky actually picking the wrong value from a page in transition and doing bad things. Thus we will be moving to being explicit at the isolate_lru_pages level, at which point there will be risk here. We will need to coax each of these to booleans before comparison.

All true, I've folded your cleanups into the base code, they are clearly better.

Yes this is always true, the buddy guarentees that the struct pages are valid and present out to MAX_ORDER around any known valid page. The zone boundary _may_ (rarely) not be MAX_ORDER aligned, but thats ok as we will detect that below when we check for the page_zone_id matching the cursor page's zone_id; should it not bail.

I've added some commentary about the assumptions on which we are relying here.

Well its mostly true, but if you remember the zone rounding patches we decided the check was so cheap as we can simply can compare the zone ids on the pages which is in the page_flags which we are touching anyhow.

The code below (.+9) maintains this check for this situation. I've added commentry to the code to say what its for.

Yes, I've added commentary for each of these, they seem obvious the day you write them, and not a month later.

will

If we ask to scan 32 pages we may scan more pages should 1<<order be greater than 32. However, if we are not going to scan at least 1 block at the order we requested then there is no point even in starting. We would not be in direct reclaim (which is the only way order may be non-0) so we really do want any scan to be at least 1 block else its a no-op and should be stopped early -- at least that seems sane?

nice new

That is an issue. Will see what we can do about that.

-apw

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

more...

One concern is that when the code goes to reclaim a lump and fails, we end up reclaiming a number of pages which we didn't really want to reclaim. Regardless of the LRU status of those pages.

I think what we should do here is to add the appropriate vmstat counters for us to be able to assess the frequency of this occurring, then throw a spread of workloads at it. If that work indicates that there's a problem then we should look at being a bit smarter about whether all the pages look to be reclaimable and if not, restore them all and give up.

Also, I suspect it would be cleaner and faster to pass the `active' flag into isolate_lru_pages(), rather than calculating it on the fly. And I don't think we need to calculate it on every pass through the loop?

We really do need those vmstat counters to let us see how effective this thing is being. Basic success/fail stuff. Per-zone, I guess.

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

Sounds like a cue ... I'll go do that.

-apw

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

Yes, the "ineffective reclaim" is more of an issue with this than linear, and the cost metric we are working on should help us show that; and then help us evaluate the utility of pushing the pages back without releasing them.

Yes, what was obvious from the linear against lumpy was that the only valid comparison was on the 'effectiveness' metric (which was basically the same) and code complexity (where lumpy clearly wins). But we have no feel for 'cost'. We are working on a cost metric for reclaim which we can use to compare performance.

Yes this is actually IMO wrong for not doing this, and I've updated it.

Yes, though the ones I have seem sane the output isn't stable and we're investigating that right now.

-apw

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