mm/page_allo.c: restructure free-page stealing code and fix a bug

The free-page stealing code in __rmqueue_fallback() is somewhat hard to follow, and has an incredible amount of subtlety hidden inside! First off, there is a minor bug in the reporting of change-of-ownership of pageblocks. Under some conditions, we try to move upto 'pageblock_nr_pages' no. of pages to the preferred allocation list. But we change the ownership of that pageblock to the preferred type only if we manage to successfully move atleast half of that pageblock (or if page_group_by_mobility_disabled is set). However, the current code ignores the latter part and sets the 'migratetype' variable to the preferred type, irrespective of whether we actually changed the pageblock migratetype of that block or not. So, the page_alloc_extfrag tracepoint can end up printing incorrect info (i.e., 'change_ownership' might be shown as 1 when it must have been 0). So fixing this involves moving the update of the 'migratetype' variable to the right place. But looking closer, we observe that the 'migratetype' variable is used subsequently for checks such as "is_migrate_cma()". Obviously the intent there is to check if the *fallback* type is MIGRATE_CMA, but since we already set the 'migratetype' variable to start_migratetype, we end up checking if the *preferred* type is MIGRATE_CMA!! To make things more interesting, this actually doesn't cause a bug in practice, because we never change *anything* if the fallback type is CMA. So, restructure the code in such a way that it is trivial to understand what is going on, and also fix the above mentioned bug. And while at it, also add a comment explaining the subtlety behind the migratetype used in the call to expand(). [akpm@linux-foundation.org: remove unneeded `inline', small coding-style fix] Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Cc: Mel Gorman <mel@csn.ul.ie> Cc: Minchan Kim <minchan@kernel.org> Cc: Cody P Schafer <cody@linux.vnet.ibm.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

mm/page_allo.c: restructure free-page stealing code and fix a bug
The free-page stealing code in __rmqueue_fallback() is somewhat hard to follow, and has an incredible amount of subtlety hidden inside! First off, there is a minor bug in the reporting of change-of-ownership of pageblocks. Under some conditions, we try to move upto 'pageblock_nr_pages' no. of pages to the preferred allocation list. But we change the ownership of that pageblock to the preferred type only if we manage to successfully move atleast half of that pageblock (or if page_group_by_mobility_disabled is set). However, the current code ignores the latter part and sets the 'migratetype' variable to the preferred type, irrespective of whether we actually changed the pageblock migratetype of that block or not. So, the page_alloc_extfrag tracepoint can end up printing incorrect info (i.e., 'change_ownership' might be shown as 1 when it must have been 0). So fixing this involves moving the update of the 'migratetype' variable to the right place. But looking closer, we observe that the 'migratetype' variable is used subsequently for checks such as "is_migrate_cma()". Obviously the intent there is to check if the *fallback* type is MIGRATE_CMA, but since we already set the 'migratetype' variable to start_migratetype, we end up checking if the *preferred* type is MIGRATE_CMA!! To make things more interesting, this actually doesn't cause a bug in practice, because we never change *anything* if the fallback type is CMA. So, restructure the code in such a way that it is trivial to understand what is going on, and also fix the above mentioned bug. And while at it, also add a comment explaining the subtlety behind the migratetype used in the call to expand(). [akpm@linux-foundation.org: remove unneeded `inline', small coding-style fix] Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Cc: Mel Gorman <mel@csn.ul.ie> Cc: Minchan Kim <minchan@kernel.org> Cc: Cody P Schafer <cody@linux.vnet.ibm.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Srivatsa S. Bhat · Linus Torvalds
1 parent b8af29418a
Showing 1 changed file with 59 additions and 36 deletions Side-by-side Diff
mm/page_alloc.c
@@ -1008,6 +1008,52 @@
 	}
 }
  
+/*
+ * If breaking a large block of pages, move all free pages to the preferred
+ * allocation list. If falling back for a reclaimable kernel allocation, be
+ * more aggressive about taking ownership of free pages.
+ *
+ * On the other hand, never change migration type of MIGRATE_CMA pageblocks
+ * nor move CMA pages to different free lists. We don't want unmovable pages
+ * to be allocated from MIGRATE_CMA areas.
+ *
+ * Returns the new migratetype of the pageblock (or the same old migratetype
+ * if it was unchanged).
+ */
+static int try_to_steal_freepages(struct zone *zone, struct page *page,
+				  int start_type, int fallback_type)
+{
+	int current_order = page_order(page);
+
+	if (is_migrate_cma(fallback_type))
+		return fallback_type;
+
+	/* Take ownership for orders >= pageblock_order */
+	if (current_order >= pageblock_order) {
+		change_pageblock_range(page, current_order, start_type);
+		return start_type;
+	}
+
+	if (current_order >= pageblock_order / 2 ||
+	    start_type == MIGRATE_RECLAIMABLE ||
+	    page_group_by_mobility_disabled) {
+		int pages;
+
+		pages = move_freepages_block(zone, page, start_type);
+
+		/* Claim the whole block if over half of it is free */
+		if (pages >= (1 << (pageblock_order-1)) ||
+				page_group_by_mobility_disabled) {
+
+			set_pageblock_migratetype(page, start_type);
+			return start_type;
+		}
+
+	}
+
+	return fallback_type;
+}
+
 /* Remove an element from the buddy allocator from the fallback list */
 static inline struct page *
 __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
@@ -1015,7 +1061,7 @@
 	struct free_area *area;
 	int current_order;
 	struct page *page;
-	int migratetype, i;
+	int migratetype, new_type, i;
  
 	/* Find the largest possible block of pages in the other list */
 	for (current_order = MAX_ORDER-1; current_order >= order;
  
  
  
@@ -1035,51 +1081,28 @@
 					struct page, lru);
 			area->nr_free--;
  
-			/*
-			 * If breaking a large block of pages, move all free
-			 * pages to the preferred allocation list. If falling
-			 * back for a reclaimable kernel allocation, be more
-			 * aggressive about taking ownership of free pages
-			 *
-			 * On the other hand, never change migration
-			 * type of MIGRATE_CMA pageblocks nor move CMA
-			 * pages on different free lists. We don't
-			 * want unmovable pages to be allocated from
-			 * MIGRATE_CMA areas.
-			 */
-			if (!is_migrate_cma(migratetype) &&
-			    (current_order >= pageblock_order / 2 ||
-			     start_migratetype == MIGRATE_RECLAIMABLE ||
-			     page_group_by_mobility_disabled)) {
-				int pages;
-				pages = move_freepages_block(zone, page,
-								start_migratetype);
+			new_type = try_to_steal_freepages(zone, page,
+							  start_migratetype,
+							  migratetype);
  
-				/* Claim the whole block if over half of it is free */
-				if (pages >= (1 << (pageblock_order-1)) ||
-						page_group_by_mobility_disabled)
-					set_pageblock_migratetype(page,
-								start_migratetype);
-
-				migratetype = start_migratetype;
-			}
-
 			/* Remove the page from the freelists */
 			list_del(&page->lru);
 			rmv_page_order(page);
  
-			/* Take ownership for orders >= pageblock_order */
-			if (current_order >= pageblock_order &&
-			    !is_migrate_cma(migratetype))
-				change_pageblock_range(page, current_order,
-							start_migratetype);
-
+			/*
+			 * Borrow the excess buddy pages as well, irrespective
+			 * of whether we stole freepages, or took ownership of
+			 * the pageblock or not.
+			 *
+			 * Exception: When borrowing from MIGRATE_CMA, release
+			 * the excess buddy pages to CMA itself.
+			 */
 			expand(zone, page, order, current_order, area,
 			       is_migrate_cma(migratetype)
 			     ? migratetype : start_migratetype);
  
 			trace_mm_page_alloc_extfrag(page, order, current_order,
-				start_migratetype, migratetype);
+				start_migratetype, new_type);
  
 			return page;
 		}
...	...	@@ -1008,6 +1008,52 @@
1008	1008	}
1009	1009	}
1010	1010
	1011	+/*
	1012	+ * If breaking a large block of pages, move all free pages to the preferred
	1013	+ * allocation list. If falling back for a reclaimable kernel allocation, be
	1014	+ * more aggressive about taking ownership of free pages.
	1015	+ *
	1016	+ * On the other hand, never change migration type of MIGRATE_CMA pageblocks
	1017	+ * nor move CMA pages to different free lists. We don't want unmovable pages
	1018	+ * to be allocated from MIGRATE_CMA areas.
	1019	+ *
	1020	+ * Returns the new migratetype of the pageblock (or the same old migratetype
	1021	+ * if it was unchanged).
	1022	+ */
	1023	+static int try_to_steal_freepages(struct zone zone, struct page page,
	1024	+ int start_type, int fallback_type)
	1025	+{
	1026	+ int current_order = page_order(page);
	1027	+
	1028	+ if (is_migrate_cma(fallback_type))
	1029	+ return fallback_type;
	1030	+
	1031	+ /* Take ownership for orders >= pageblock_order */
	1032	+ if (current_order >= pageblock_order) {
	1033	+ change_pageblock_range(page, current_order, start_type);
	1034	+ return start_type;
	1035	+ }
	1036	+
	1037	+ if (current_order >= pageblock_order / 2 \|\|
	1038	+ start_type == MIGRATE_RECLAIMABLE \|\|
	1039	+ page_group_by_mobility_disabled) {
	1040	+ int pages;
	1041	+
	1042	+ pages = move_freepages_block(zone, page, start_type);
	1043	+
	1044	+ /* Claim the whole block if over half of it is free */
	1045	+ if (pages >= (1 << (pageblock_order-1)) \|\|
	1046	+ page_group_by_mobility_disabled) {
	1047	+
	1048	+ set_pageblock_migratetype(page, start_type);
	1049	+ return start_type;
	1050	+ }
	1051	+
	1052	+ }
	1053	+
	1054	+ return fallback_type;
	1055	+}
	1056	+
1011	1057	/* Remove an element from the buddy allocator from the fallback list */
1012	1058	static inline struct page *
1013	1059	__rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
...	...	@@ -1015,7 +1061,7 @@
1015	1061	struct free_area *area;
1016	1062	int current_order;
1017	1063	struct page *page;
1018		- int migratetype, i;
	1064	+ int migratetype, new_type, i;
1019	1065
1020	1066	/* Find the largest possible block of pages in the other list */
1021	1067	for (current_order = MAX_ORDER-1; current_order >= order;
1022	1068
1023	1069
1024	1070
...	...	@@ -1035,51 +1081,28 @@
1035	1081	struct page, lru);
1036	1082	area->nr_free--;
1037	1083
1038		- /*
1039		- * If breaking a large block of pages, move all free
1040		- * pages to the preferred allocation list. If falling
1041		- * back for a reclaimable kernel allocation, be more
1042		- * aggressive about taking ownership of free pages
1043		- *
1044		- * On the other hand, never change migration
1045		- * type of MIGRATE_CMA pageblocks nor move CMA
1046		- * pages on different free lists. We don't
1047		- * want unmovable pages to be allocated from
1048		- * MIGRATE_CMA areas.
1049		- */
1050		- if (!is_migrate_cma(migratetype) &&
1051		- (current_order >= pageblock_order / 2 \|\|
1052		- start_migratetype == MIGRATE_RECLAIMABLE \|\|
1053		- page_group_by_mobility_disabled)) {
1054		- int pages;
1055		- pages = move_freepages_block(zone, page,
1056		- start_migratetype);
	1084	+ new_type = try_to_steal_freepages(zone, page,
	1085	+ start_migratetype,
	1086	+ migratetype);
1057	1087
1058		- /* Claim the whole block if over half of it is free */
1059		- if (pages >= (1 << (pageblock_order-1)) \|\|
1060		- page_group_by_mobility_disabled)
1061		- set_pageblock_migratetype(page,
1062		- start_migratetype);
1063		-
1064		- migratetype = start_migratetype;
1065		- }
1066		-
1067	1088	/* Remove the page from the freelists */
1068	1089	list_del(&page->lru);
1069	1090	rmv_page_order(page);
1070	1091
1071		- /* Take ownership for orders >= pageblock_order */
1072		- if (current_order >= pageblock_order &&
1073		- !is_migrate_cma(migratetype))
1074		- change_pageblock_range(page, current_order,
1075		- start_migratetype);
1076		-
	1092	+ /*
	1093	+ * Borrow the excess buddy pages as well, irrespective
	1094	+ * of whether we stole freepages, or took ownership of
	1095	+ * the pageblock or not.
	1096	+ *
	1097	+ * Exception: When borrowing from MIGRATE_CMA, release
	1098	+ * the excess buddy pages to CMA itself.
	1099	+ */
1077	1100	expand(zone, page, order, current_order, area,
1078	1101	is_migrate_cma(migratetype)
1079	1102	? migratetype : start_migratetype);
1080	1103
1081	1104	trace_mm_page_alloc_extfrag(page, order, current_order,
1082		- start_migratetype, migratetype);
	1105	+ start_migratetype, new_type);
1083	1106
1084	1107	return page;
1085	1108	}