mm: remove MIGRATE_ISOLATE check in hotpath

Now mm several functions test MIGRATE_ISOLATE and some of those are hotpath but MIGRATE_ISOLATE is used only if we enable CONFIG_MEMORY_ISOLATION(ie, CMA, memory-hotplug and memory-failure) which are not common config option. So let's not add unnecessary overhead and code when we don't enable CONFIG_MEMORY_ISOLATION.

Cc KOSAKI Motohiro <[email protected]>

Cc Michal Nazarewicz <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 include/linux/mmzone.h         |    2 ++
 include/linux/page-isolation.h |   19 +++++++++++++++++++
 mm/compaction.c                |    6 +++++-
 mm/page_alloc.c                |   16 ++++++++++------
 mm/vmstat.c                    |    2 ++
 5 files changed, 38 insertions(+), 7 deletions(-)

include/linux/mmzone.h +2 -0

index 73b64a3..4f4c8c2 100644
@@ -57,7 +57,9 @@ enum {
 	 */
 	MIGRATE_CMA,
 #endif
+#ifdef CONFIG_MEMORY_ISOLATION
 	MIGRATE_ISOLATE,	/* can't allocate from here */
+#endif
 	MIGRATE_TYPES
 };
 

include/linux/page-isolation.h +19 -0

index a92061e..3fff8e7 100644
@@ -1,6 +1,25 @@
 #ifndef __LINUX_PAGEISOLATION_H
 #define __LINUX_PAGEISOLATION_H
 
+#ifdef CONFIG_MEMORY_ISOLATION
+static inline bool is_migrate_isolate_page(struct page *page)
+{
+	return get_pageblock_migratetype(page) == MIGRATE_ISOLATE;
+}
+static inline bool is_migrate_isolate(int migratetype)
+{
+	return migratetype == MIGRATE_ISOLATE;
+}
+#else
+static inline bool is_migrate_isolate_page(struct page *page)
+{
+	return false;
+}
+static inline bool is_migrate_isolate(int migratetype)
+{
+	return false;
+}
+#endif
 
 bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
 			 bool skip_hwpoisoned_pages);

mm/compaction.c +5 -1

index 675937c..bb2a655 100644
@@ -15,6 +15,7 @@
 #include <linux/sysctl.h>
 #include <linux/sysfs.h>
 #include <linux/balloon_compaction.h>
+#include <linux/page-isolation.h>
 #include "internal.h"
 
 #ifdef CONFIG_COMPACTION

@@ -215,7 +216,10 @@ static bool suitable_migration_target(struct page *page)
 	int migratetype = get_pageblock_migratetype(page);
 
 	/* Don't interfere with memory hot-remove or the min_free_kbytes blocks */
-	if (migratetype == MIGRATE_ISOLATE || migratetype == MIGRATE_RESERVE)
+	if (migratetype == MIGRATE_RESERVE)
+		return false;
+
+	if (is_migrate_isolate(migratetype))
 		return false;
 
 	/* If the page is a large free page, then allow migration */

mm/page_alloc.c +10 -6

index 82117f5..319a8f0 100644
@@ -665,7 +665,7 @@ static void free_pcppages_bulk(struct zone *zone, int count,
 			/* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
 			__free_one_page(page, zone, 0, mt);
 			trace_mm_page_pcpu_drain(page, 0, mt);
-			if (likely(get_pageblock_migratetype(page) != MIGRATE_ISOLATE)) {
+			if (likely(!is_migrate_isolate_page(page))) {
 				__mod_zone_page_state(zone, NR_FREE_PAGES, 1);
 				if (is_migrate_cma(mt))
 					__mod_zone_page_state(zone, NR_FREE_CMA_PAGES, 1);

@@ -683,7 +683,7 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
 	zone->pages_scanned = 0;
 
 	__free_one_page(page, zone, order, migratetype);
-	if (unlikely(migratetype != MIGRATE_ISOLATE))
+	if (unlikely(!is_migrate_isolate(migratetype)))
 		__mod_zone_freepage_state(zone, 1 << order, migratetype);
 	spin_unlock(&zone->lock);
 }

@@ -911,7 +911,9 @@ static int fallbacks[MIGRATE_TYPES][4] = {
 	[MIGRATE_MOVABLE]     = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE,   MIGRATE_RESERVE },
 #endif
 	[MIGRATE_RESERVE]     = { MIGRATE_RESERVE }, /* Never used */
+#ifdef CONFIG_MEMORY_ISOLATION
 	[MIGRATE_ISOLATE]     = { MIGRATE_RESERVE }, /* Never used */
+#endif
 };
 
 /*

@@ -1137,7 +1139,7 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
 			list_add_tail(&page->lru, list);
 		if (IS_ENABLED(CONFIG_CMA)) {
 			mt = get_pageblock_migratetype(page);
-			if (!is_migrate_cma(mt) && mt != MIGRATE_ISOLATE)
+			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt))
 				mt = migratetype;
 		}
 		set_freepage_migratetype(page, mt);

@@ -1321,7 +1323,7 @@ void free_hot_cold_page(struct page *page, int cold)
 	 * excessively into the page allocator
 	 */
 	if (migratetype >= MIGRATE_PCPTYPES) {
-		if (unlikely(migratetype == MIGRATE_ISOLATE)) {
+		if (unlikely(is_migrate_isolate(migratetype))) {
 			free_one_page(zone, page, 0, migratetype);
 			goto out;
 		}

@@ -1402,7 +1404,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
 	order = page_order(page);
 	mt = get_pageblock_migratetype(page);
 
-	if (mt != MIGRATE_ISOLATE) {
+	if (!is_migrate_isolate(mt)) {
 		/* Obey watermarks as if the page was being allocated */
 		watermark = low_wmark_pages(zone) + (1 << order);
 		if (!zone_watermark_ok(zone, 0, watermark, 0, 0))

@@ -1425,7 +1427,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
 		struct page *endpage = page + (1 << order) - 1;
 		for (; page < endpage; page += pageblock_nr_pages) {
 			int mt = get_pageblock_migratetype(page);
-			if (mt != MIGRATE_ISOLATE && !is_migrate_cma(mt))
+			if (!is_migrate_isolate(mt) && !is_migrate_cma(mt))
 				set_pageblock_migratetype(page,
 							  MIGRATE_MOVABLE);
 		}

@@ -2911,7 +2913,9 @@ static void show_migration_types(unsigned char type)
 #ifdef CONFIG_CMA
 		[MIGRATE_CMA]		= 'C',
 #endif
+#ifdef CONFIG_MEMORY_ISOLATION
 		[MIGRATE_ISOLATE]	= 'I',
+#endif
 	};
 	char tmp[MIGRATE_TYPES + 1];
 	char *p = tmp;

mm/vmstat.c +2 -0

index 7a65e26..b0f1db1 100644
@@ -628,7 +628,9 @@ static char * const migratetype_names[MIGRATE_TYPES] = {
 #ifdef CONFIG_CMA
 	"CMA",
 #endif
+#ifdef CONFIG_MEMORY_ISOLATION
 	"Isolate",
+#endif
 };
 
 static void *frag_start(struct seq_file *m, loff_t *pos)

Now mm several functions test MIGRATE_ISOLATE and some of those are hotpath but MIGRATE_ISOLATE is used only if we enable CONFIG_MEMORY_ISOLATION(ie, CMA, memory-hotplug and memory-failure) which are not common config option. So let's not add unnecessary overhead and code when we don't enable CONFIG_MEMORY_ISOLATION.

Quoted 5 lines · On Tue, 15 Jan 2013 09:16:46 +0900 Minchan Kim <[email protected]> wrote:

Now mm several functions test MIGRATE_ISOLATE and some of those are hotpath but MIGRATE_ISOLATE is used only if we enable CONFIG_MEMORY_ISOLATION(ie, CMA, memory-hotplug and memory-failure) which are not common config option. So let's not add unnecessary overhead and code when we don't enable CONFIG_MEMORY_ISOLATION.

ugh. Better than nothing, I guess.

There remain call sites which do open-coded

	get_pageblock_migratetype(page) != MIGRATE_ISOLATE

(undo_isolate_page_range() is one). Wanna clean these up as well?

mm/page_alloc.c static void free_one_page(struct zone *zone, struct page *page, int order, · line 689

Quoted 13 lines

...

mm/page_alloc.c +1 -1

@@ -683,7 +683,7 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
 	zone->pages_scanned = 0;
 
 	__free_one_page(page, zone, order, migratetype);
-	if (unlikely(migratetype != MIGRATE_ISOLATE))
+	if (unlikely(!is_migrate_isolate(migratetype)))
 		__mod_zone_freepage_state(zone, 1 << order, migratetype);
 	spin_unlock(&zone->lock);
 }

The code both before and after this patch is assuming that the migratetype in free_one_page is likely to be MIGRATE_ISOLATE. Seems wrong. If CONFIG_MEMORY_ISOLATION=n this ends up doing if(unlikely(true)) which is harmless-but-amusing.

From the beginning of [2139cbe627, cma: fix counting of isolated pages],

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

Quoted 18 lines

mm/page_alloc.c static void free_one_page(struct zone *zone, struct page *page, int order, · line 689

Quoted 13 lines

...

mm/page_alloc.c +1 -1

@@ -683,7 +683,7 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
 	zone->pages_scanned = 0;
 
 	__free_one_page(page, zone, order, migratetype);
-	if (unlikely(migratetype != MIGRATE_ISOLATE))
+	if (unlikely(!is_migrate_isolate(migratetype)))
 		__mod_zone_freepage_state(zone, 1 << order, migratetype);
 	spin_unlock(&zone->lock);
 }

The code both before and after this patch is assuming that the migratetype in free_one_page is likely to be MIGRATE_ISOLATE. Seems wrong. If CONFIG_MEMORY_ISOLATION=n this ends up doing if(unlikely(true)) which is harmless-but-amusing.

Quoted 1 line

From the beginning of [2139cbe627, cma: fix counting of isolated pages],

it was wrong. We can't make sure it's very likely.
If it is called by order-0 page free path, it is but if it is called by high order page free path, we can't.
So I think it would be better to remove unlikley.

They are trivial patch so send it now or send it after you release first mmotm after finishing merge window?

Quoted 19 lines · On Mon, 25 Feb 2013 11:13:08 +0900 Minchan Kim <[email protected]> wrote:

Quoted 18 lines

mm/page_alloc.c static void free_one_page(struct zone *zone, struct page *page, int order, · line 689

Quoted 13 lines

...

mm/page_alloc.c +1 -1

@@ -683,7 +683,7 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
 	zone->pages_scanned = 0;
 
 	__free_one_page(page, zone, order, migratetype);
-	if (unlikely(migratetype != MIGRATE_ISOLATE))
+	if (unlikely(!is_migrate_isolate(migratetype)))
 		__mod_zone_freepage_state(zone, 1 << order, migratetype);
 	spin_unlock(&zone->lock);
 }

The code both before and after this patch is assuming that the migratetype in free_one_page is likely to be MIGRATE_ISOLATE. Seems wrong. If CONFIG_MEMORY_ISOLATION=n this ends up doing if(unlikely(true)) which is harmless-but-amusing.

Quoted 5 lines

Quoted 1 line

From the beginning of [2139cbe627, cma: fix counting of isolated pages],

it was wrong. We can't make sure it's very likely.
If it is called by order-0 page free path, it is but if it is called by high order page free path, we can't.
So I think it would be better to remove unlikley.

Order-0 pages surely preponderate, so I'd say that "likely" is the way to go.

I don't recall anyone ever demonstrating that likely/unlikely actually does anything useful. It would be interesting to have a play around, see if it actually does good things to the code generation.

I think someone (perhaps in or near Dave Jones?) once had a patch which added counters to likely/unlikely, so the kernel can accumulate and then report upon the hit/miss ratio at each site. iirc, an alarmingly large number of the sites were deoptimisations!

Quoted 3 lines

They are trivial patch so send it now or send it after you release first mmotm after finishing merge window?

It's in mainline now.

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