[PATCH v4 0/4] cma: fix watermark checking

[PATCH v4 0/4] cma: fix watermark checking

[Bartlomiej Zolnierkiewicz]

Free pages belonging to Contiguous Memory Allocator (CMA) areas cannot be
used by unmovable allocations and this fact should be accounted for while
doing zone watermark checking.  Additionaly while CMA pages are isolated
they shouldn't be included in the total number of free pages (as they
cannot be allocated while they are isolated).  The following patch series
should fix both issues.  It is based on top of recent Minchan's CMA series
(https://lkml.org/lkml/2012/8/14/81 "[RFC 0/2] Reduce alloc_contig_range
latency").

v4:
- update patch description in patch #1
- add unlikely() to patch #2 (noticed by Minchan Kim)
- fix whitespace change in patch #2 (noticed by Mel Gorman)
- add __mod_zone_freepage_state() in patch #3 (from Minchan)
- fold patch #4 into patch #5 (suggested by Minchan)
- use ALLOC_CMA alloc flag instead of gfp_flags (from Minchan)

v3:
- fix tracing in free_pcppages_bulk()
- fix counting of free CMA pages (broken by v2)

v2:
- no need to call get_pageblock_migratetype() in free_one_page() in patch #1
  (thanks to review from Michal Nazarewicz)
- fix issues pointed in http://www.spinics.net/lists/linux-mm/msg41017.html
  in patch #2 (ditto)
- remove no longer needed is_cma_pageblock() from patch #2


Bartlomiej Zolnierkiewicz (4):
  mm: fix tracing in free_pcppages_bulk()
  cma: fix counting of isolated pages
  cma: count free CMA pages
  cma: fix watermark checking

 include/linux/mmzone.h | 16 ++++++++++++
 include/linux/vmstat.h |  8 ++++++
 mm/compaction.c        |  8 +++++-
 mm/page_alloc.c        | 68 +++++++++++++++++++++++++++++++-------------------
 mm/page_isolation.c    | 13 +++++++---
 mm/vmstat.c            |  1 +
 6 files changed, 85 insertions(+), 29 deletions(-)

-- 
1.7.11.3

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

[PATCH v4 1/4] mm: fix tracing in free_pcppages_bulk()

[Bartlomiej Zolnierkiewicz]

page->private gets re-used in __free_one_page() to store page order
(so trace_mm_page_pcpu_drain() may print order instead of migratetype)
thus migratetype value must be cached locally.

Fixes regression introduced in a701623 ("mm: fix migratetype bug
which slowed swapping").

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: Mel Gorman <mgorman@suse.de>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 mm/page_alloc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 93a3433..e9da55c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -668,12 +668,15 @@ static void free_pcppages_bulk(struct zone *zone, int count,
 			batch_free = to_free;
 
 		do {
+			int mt;
+
 			page = list_entry(list->prev, struct page, lru);
 			/* must delete as __free_one_page list manipulates */
 			list_del(&page->lru);
+			mt = page_private(page);
 			/* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
-			__free_one_page(page, zone, 0, page_private(page));
-			trace_mm_page_pcpu_drain(page, 0, page_private(page));
+			__free_one_page(page, zone, 0, mt);
+			trace_mm_page_pcpu_drain(page, 0, mt);
 		} while (--to_free && --batch_free && !list_empty(list));
 	}
 	__mod_zone_page_state(zone, NR_FREE_PAGES, count);
-- 
1.7.11.3

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

[PATCH v4 2/4] cma: fix counting of isolated pages

[Bartlomiej Zolnierkiewicz]

Isolated free pages shouldn't be accounted to NR_FREE_PAGES counter.
Fix it by properly decreasing/increasing NR_FREE_PAGES counter in
set_migratetype_isolate()/unset_migratetype_isolate() and removing
counter adjustment for isolated pages from free_one_page() and
split_free_page().

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 mm/page_alloc.c     |  9 +++++++--
 mm/page_isolation.c | 12 +++++++++---
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e9da55c..6a59e42 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -691,7 +691,8 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
 	zone->pages_scanned = 0;
 
 	__free_one_page(page, zone, order, migratetype);
-	__mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
+	if (unlikely(migratetype != MIGRATE_ISOLATE))
+		__mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
 	spin_unlock(&zone->lock);
 }
 
@@ -1397,6 +1398,7 @@ int split_free_page(struct page *page, bool check_wmark)
 	unsigned int order;
 	unsigned long watermark;
 	struct zone *zone;
+	int mt;
 
 	BUG_ON(!PageBuddy(page));
 
@@ -1414,7 +1416,10 @@ int split_free_page(struct page *page, bool check_wmark)
 	list_del(&page->lru);
 	zone->free_area[order].nr_free--;
 	rmv_page_order(page);
-	__mod_zone_page_state(zone, NR_FREE_PAGES, -(1UL << order));
+
+	mt = get_pageblock_migratetype(page);
+	if (unlikely(mt != MIGRATE_ISOLATE))
+		__mod_zone_page_state(zone, NR_FREE_PAGES, -(1UL << order));
 
 	/* Split into individual pages */
 	set_page_refcounted(page);
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 247d1f1..3ca1716 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -76,8 +76,12 @@ int set_migratetype_isolate(struct page *page)
 
 out:
 	if (!ret) {
+		unsigned long nr_pages;
+
 		set_pageblock_isolate(page);
-		move_freepages_block(zone, page, MIGRATE_ISOLATE);
+		nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE);
+
+		__mod_zone_page_state(zone, NR_FREE_PAGES, -nr_pages);
 	}
 
 	spin_unlock_irqrestore(&zone->lock, flags);
@@ -89,12 +93,14 @@ out:
 void unset_migratetype_isolate(struct page *page, unsigned migratetype)
 {
 	struct zone *zone;
-	unsigned long flags;
+	unsigned long flags, nr_pages;
+
 	zone = page_zone(page);
 	spin_lock_irqsave(&zone->lock, flags);
 	if (get_pageblock_migratetype(page) != MIGRATE_ISOLATE)
 		goto out;
-	move_freepages_block(zone, page, migratetype);
+	nr_pages = move_freepages_block(zone, page, migratetype);
+	__mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
 	restore_pageblock_isolate(page, migratetype);
 out:
 	spin_unlock_irqrestore(&zone->lock, flags);
-- 
1.7.11.3

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

[PATCH v4 3/4] cma: count free CMA pages

[Bartlomiej Zolnierkiewicz]

Add NR_FREE_CMA_PAGES counter to be later used for checking watermark
in __zone_watermark_ok().  For simplicity and to avoid #ifdef hell make
this counter always available (not only when CONFIG_CMA=y).

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 include/linux/mmzone.h |  1 +
 include/linux/vmstat.h |  8 ++++++++
 mm/page_alloc.c        | 26 +++++++++++++++++++-------
 mm/page_isolation.c    |  5 +++--
 mm/vmstat.c            |  1 +
 5 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index ca034a1..904889d 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -140,6 +140,7 @@ enum zone_stat_item {
 	NUMA_OTHER,		/* allocation from other node */
 #endif
 	NR_ANON_TRANSPARENT_HUGEPAGES,
+	NR_FREE_CMA_PAGES,
 	NR_VM_ZONE_STAT_ITEMS };
 
 /*
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index ad2cfd5..a5bb150 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -253,6 +253,14 @@ static inline void refresh_zone_stat_thresholds(void) { }
 
 #endif		/* CONFIG_SMP */
 
+static inline void __mod_zone_freepage_state(struct zone *zone, int nr_pages,
+					     int migratetype)
+{
+	__mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
+	if (is_migrate_cma(migratetype))
+		__mod_zone_page_state(zone, NR_FREE_CMA_PAGES, nr_pages);
+}
+
 extern const char * const vmstat_text[];
 
 #endif /* _LINUX_VMSTAT_H */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6a59e42..287f79d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -558,7 +558,8 @@ static inline void __free_one_page(struct page *page,
 		if (page_is_guard(buddy)) {
 			clear_page_guard_flag(buddy);
 			set_page_private(page, 0);
-			__mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
+			__mod_zone_freepage_state(zone, 1 << order,
+						  migratetype);
 		} else {
 			list_del(&buddy->lru);
 			zone->free_area[order].nr_free--;
@@ -677,6 +678,8 @@ 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 (is_migrate_cma(mt))
+				__mod_zone_page_state(zone, NR_FREE_CMA_PAGES, 1);
 		} while (--to_free && --batch_free && !list_empty(list));
 	}
 	__mod_zone_page_state(zone, NR_FREE_PAGES, count);
@@ -692,7 +695,7 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
 
 	__free_one_page(page, zone, order, migratetype);
 	if (unlikely(migratetype != MIGRATE_ISOLATE))
-		__mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
+		__mod_zone_freepage_state(zone, 1 << order, migratetype);
 	spin_unlock(&zone->lock);
 }
 
@@ -815,7 +818,8 @@ static inline void expand(struct zone *zone, struct page *page,
 			set_page_guard_flag(&page[size]);
 			set_page_private(&page[size], high);
 			/* Guard pages are not available for any usage */
-			__mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << high));
+			__mod_zone_freepage_state(zone, -(1 << high),
+						  migratetype);
 			continue;
 		}
 #endif
@@ -1141,6 +1145,9 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
 		}
 		set_page_private(page, mt);
 		list = &page->lru;
+		if (is_migrate_cma(mt))
+			__mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
+					      -(1 << order));
 	}
 	__mod_zone_page_state(zone, NR_FREE_PAGES, -(i << order));
 	spin_unlock(&zone->lock);
@@ -1419,7 +1426,7 @@ int split_free_page(struct page *page, bool check_wmark)
 
 	mt = get_pageblock_migratetype(page);
 	if (unlikely(mt != MIGRATE_ISOLATE))
-		__mod_zone_page_state(zone, NR_FREE_PAGES, -(1UL << order));
+		__mod_zone_freepage_state(zone, -(1UL << order), mt);
 
 	/* Split into individual pages */
 	set_page_refcounted(page);
@@ -1494,7 +1501,8 @@ again:
 		spin_unlock(&zone->lock);
 		if (!page)
 			goto failed;
-		__mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
+		__mod_zone_freepage_state(zone, -(1 << order),
+					  get_pageblock_migratetype(page));
 	}
 
 	__count_zone_vm_events(PGALLOC, zone, 1 << order);
@@ -2862,7 +2870,8 @@ void show_free_areas(unsigned int filter)
 		" unevictable:%lu"
 		" dirty:%lu writeback:%lu unstable:%lu\n"
 		" free:%lu slab_reclaimable:%lu slab_unreclaimable:%lu\n"
-		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
+		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
+		" free_cma:%lu\n",
 		global_page_state(NR_ACTIVE_ANON),
 		global_page_state(NR_INACTIVE_ANON),
 		global_page_state(NR_ISOLATED_ANON),
@@ -2879,7 +2888,8 @@ void show_free_areas(unsigned int filter)
 		global_page_state(NR_FILE_MAPPED),
 		global_page_state(NR_SHMEM),
 		global_page_state(NR_PAGETABLE),
-		global_page_state(NR_BOUNCE));
+		global_page_state(NR_BOUNCE),
+		global_page_state(NR_FREE_CMA_PAGES));
 
 	for_each_populated_zone(zone) {
 		int i;
@@ -2911,6 +2921,7 @@ void show_free_areas(unsigned int filter)
 			" pagetables:%lukB"
 			" unstable:%lukB"
 			" bounce:%lukB"
+			" free_cma:%lukB"
 			" writeback_tmp:%lukB"
 			" pages_scanned:%lu"
 			" all_unreclaimable? %s"
@@ -2940,6 +2951,7 @@ void show_free_areas(unsigned int filter)
 			K(zone_page_state(zone, NR_PAGETABLE)),
 			K(zone_page_state(zone, NR_UNSTABLE_NFS)),
 			K(zone_page_state(zone, NR_BOUNCE)),
+			K(zone_page_state(zone, NR_FREE_CMA_PAGES)),
 			K(zone_page_state(zone, NR_WRITEBACK_TEMP)),
 			zone->pages_scanned,
 			(zone->all_unreclaimable ? "yes" : "no")
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 3ca1716..bce97c9 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -77,11 +77,12 @@ int set_migratetype_isolate(struct page *page)
 out:
 	if (!ret) {
 		unsigned long nr_pages;
+		int mt = get_pageblock_migratetype(page);
 
 		set_pageblock_isolate(page);
 		nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE);
 
-		__mod_zone_page_state(zone, NR_FREE_PAGES, -nr_pages);
+		__mod_zone_freepage_state(zone, -nr_pages, mt);
 	}
 
 	spin_unlock_irqrestore(&zone->lock, flags);
@@ -100,7 +101,7 @@ void unset_migratetype_isolate(struct page *page, unsigned migratetype)
 	if (get_pageblock_migratetype(page) != MIGRATE_ISOLATE)
 		goto out;
 	nr_pages = move_freepages_block(zone, page, migratetype);
-	__mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
+	__mod_zone_freepage_state(zone, nr_pages, migratetype);
 	restore_pageblock_isolate(page, migratetype);
 out:
 	spin_unlock_irqrestore(&zone->lock, flags);
diff --git a/mm/vmstat.c b/mm/vmstat.c
index df7a674..7c102e6 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -722,6 +722,7 @@ const char * const vmstat_text[] = {
 	"numa_other",
 #endif
 	"nr_anon_transparent_hugepages",
+	"nr_free_cma",
 	"nr_dirty_threshold",
 	"nr_dirty_background_threshold",
 
-- 
1.7.11.3

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

[PATCH v4 4/4] cma: fix watermark checking

[Bartlomiej Zolnierkiewicz]

* Add ALLOC_CMA alloc flag and pass it to [__]zone_watermark_ok()
  (from Minchan Kim).

* During watermark check decrease available free pages number by
  free CMA pages number if necessary (unmovable allocations cannot
  use pages from CMA areas).

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 include/linux/mmzone.h | 15 +++++++++++++++
 mm/compaction.c        |  8 +++++++-
 mm/page_alloc.c        | 30 ++++++++++++++----------------
 3 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 904889d..be8abdb 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -231,6 +231,21 @@ enum zone_watermarks {
 #define low_wmark_pages(z) (z->watermark[WMARK_LOW])
 #define high_wmark_pages(z) (z->watermark[WMARK_HIGH])
 
+/* The ALLOC_WMARK bits are used as an index to zone->watermark */
+#define ALLOC_WMARK_MIN		WMARK_MIN
+#define ALLOC_WMARK_LOW		WMARK_LOW
+#define ALLOC_WMARK_HIGH	WMARK_HIGH
+#define ALLOC_NO_WATERMARKS	0x04 /* don't check watermarks at all */
+
+/* Mask to get the watermark bits */
+#define ALLOC_WMARK_MASK	(ALLOC_NO_WATERMARKS-1)
+
+#define ALLOC_HARDER		0x10 /* try to alloc harder */
+#define ALLOC_HIGH		0x20 /* __GFP_HIGH set */
+#define ALLOC_CPUSET		0x40 /* check for correct cpuset */
+
+#define ALLOC_CMA		0x80
+
 struct per_cpu_pages {
 	int count;		/* number of pages in the list */
 	int high;		/* high watermark, emptying needed */
diff --git a/mm/compaction.c b/mm/compaction.c
index 4b902aa..36d79ea 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -868,6 +868,7 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
 	struct zoneref *z;
 	struct zone *zone;
 	int rc = COMPACT_SKIPPED;
+	int alloc_flags = 0;
 
 	/*
 	 * Check whether it is worth even starting compaction. The order check is
@@ -879,6 +880,10 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
 
 	count_vm_event(COMPACTSTALL);
 
+#ifdef CONFIG_CMA
+	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
+		alloc_flags |= ALLOC_CMA;
+#endif
 	/* Compact each zone in the list */
 	for_each_zone_zonelist_nodemask(zone, z, zonelist, high_zoneidx,
 								nodemask) {
@@ -889,7 +894,8 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
 		rc = max(status, rc);
 
 		/* If a normal allocation would succeed, stop compacting */
-		if (zone_watermark_ok(zone, order, low_wmark_pages(zone), 0, 0))
+		if (zone_watermark_ok(zone, order, low_wmark_pages(zone), 0,
+				      alloc_flags))
 			break;
 	}
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 287f79d..5985cbf 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1519,19 +1519,6 @@ failed:
 	return NULL;
 }
 
-/* The ALLOC_WMARK bits are used as an index to zone->watermark */
-#define ALLOC_WMARK_MIN		WMARK_MIN
-#define ALLOC_WMARK_LOW		WMARK_LOW
-#define ALLOC_WMARK_HIGH	WMARK_HIGH
-#define ALLOC_NO_WATERMARKS	0x04 /* don't check watermarks at all */
-
-/* Mask to get the watermark bits */
-#define ALLOC_WMARK_MASK	(ALLOC_NO_WATERMARKS-1)
-
-#define ALLOC_HARDER		0x10 /* try to alloc harder */
-#define ALLOC_HIGH		0x20 /* __GFP_HIGH set */
-#define ALLOC_CPUSET		0x40 /* check for correct cpuset */
-
 #ifdef CONFIG_FAIL_PAGE_ALLOC
 
 static struct {
@@ -1626,7 +1613,10 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark,
 		min -= min / 2;
 	if (alloc_flags & ALLOC_HARDER)
 		min -= min / 4;
-
+#ifdef CONFIG_CMA
+	if (!(alloc_flags & ALLOC_CMA))
+		free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
+#endif
 	if (free_pages <= min + lowmem_reserve)
 		return false;
 	for (o = 0; o < order; o++) {
@@ -2333,7 +2323,10 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
 				 unlikely(test_thread_flag(TIF_MEMDIE))))
 			alloc_flags |= ALLOC_NO_WATERMARKS;
 	}
-
+#ifdef CONFIG_CMA
+	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
+		alloc_flags |= ALLOC_CMA;
+#endif
 	return alloc_flags;
 }
 
@@ -2559,6 +2552,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 	struct page *page = NULL;
 	int migratetype = allocflags_to_migratetype(gfp_mask);
 	unsigned int cpuset_mems_cookie;
+	int alloc_flags = ALLOC_WMARK_LOW|ALLOC_CPUSET;
 
 	gfp_mask &= gfp_allowed_mask;
 
@@ -2587,9 +2581,13 @@ retry_cpuset:
 	if (!preferred_zone)
 		goto out;
 
+#ifdef CONFIG_CMA
+	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
+		alloc_flags |= ALLOC_CMA;
+#endif
 	/* First allocation attempt */
 	page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
-			zonelist, high_zoneidx, ALLOC_WMARK_LOW|ALLOC_CPUSET,
+			zonelist, high_zoneidx, alloc_flags,
 			preferred_zone, migratetype);
 	if (unlikely(!page))
 		page = __alloc_pages_slowpath(gfp_mask, order,
-- 
1.7.11.3

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

Re: [PATCH v4 1/4] mm: fix tracing in free_pcppages_bulk()

[Yasuaki Ishimatsu]

Hi Bartlomiej,

2012/09/14 23:29, Bartlomiej Zolnierkiewicz wrote:
> page->private gets re-used in __free_one_page() to store page order
> (so trace_mm_page_pcpu_drain() may print order instead of migratetype)
> thus migratetype value must be cached locally.
> 
> Fixes regression introduced in a701623 ("mm: fix migratetype bug
> which slowed swapping").

I think the regression has been alreadly fixed by following Mincahn's patches.

https://lkml.org/lkml/2012/9/6/635

=> Hi Minchan,

   Am I wrong?

Thanks,
Yasuaki Ishimatsu
 
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Acked-by: Minchan Kim <minchan@kernel.org>
> Acked-by: Mel Gorman <mgorman@suse.de>
> Cc: Hugh Dickins <hughd@google.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>   mm/page_alloc.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 93a3433..e9da55c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -668,12 +668,15 @@ static void free_pcppages_bulk(struct zone *zone, int count,
>   			batch_free = to_free;
>   
>   		do {
> +			int mt;
> +
>   			page = list_entry(list->prev, struct page, lru);
>   			/* must delete as __free_one_page list manipulates */
>   			list_del(&page->lru);
> +			mt = page_private(page);
>   			/* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
> -			__free_one_page(page, zone, 0, page_private(page));
> -			trace_mm_page_pcpu_drain(page, 0, page_private(page));
> +			__free_one_page(page, zone, 0, mt);
> +			trace_mm_page_pcpu_drain(page, 0, mt);
>   		} while (--to_free && --batch_free && !list_empty(list));
>   	}
>   	__mod_zone_page_state(zone, NR_FREE_PAGES, count);
> 


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

Re: [PATCH v4 1/4] mm: fix tracing in free_pcppages_bulk()

[Minchan Kim]

Hi Yasuaki,

On Wed, Sep 19, 2012 at 04:07:19PM +0900, Yasuaki Ishimatsu wrote:
> Hi Bartlomiej,
> 
> 2012/09/14 23:29, Bartlomiej Zolnierkiewicz wrote:
> > page->private gets re-used in __free_one_page() to store page order
> > (so trace_mm_page_pcpu_drain() may print order instead of migratetype)
> > thus migratetype value must be cached locally.
> > 
> > Fixes regression introduced in a701623 ("mm: fix migratetype bug
> > which slowed swapping").
> 
> I think the regression has been alreadly fixed by following Mincahn's patches.
> 
> https://lkml.org/lkml/2012/9/6/635
> 
> => Hi Minchan,
> 
>    Am I wrong?

This patch isn't related to mine.
In addition, this patch don't need to be a part of this series.

-- 
Kind regards,
Minchan Kim

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

Re: [PATCH v4 1/4] mm: fix tracing in free_pcppages_bulk()

[Yasuaki Ishimatsu]

Hi Minchan,

2012/09/19 16:32, Minchan Kim wrote:
> Hi Yasuaki,
>
> On Wed, Sep 19, 2012 at 04:07:19PM +0900, Yasuaki Ishimatsu wrote:
>> Hi Bartlomiej,
>>
>> 2012/09/14 23:29, Bartlomiej Zolnierkiewicz wrote:
>>> page->private gets re-used in __free_one_page() to store page order
>>> (so trace_mm_page_pcpu_drain() may print order instead of migratetype)
>>> thus migratetype value must be cached locally.
>>>
>>> Fixes regression introduced in a701623 ("mm: fix migratetype bug
>>> which slowed swapping").
>>
>> I think the regression has been alreadly fixed by following Mincahn's patches.
>>
>> https://lkml.org/lkml/2012/9/6/635
>>
>> => Hi Minchan,
>>
>>     Am I wrong?
>
> This patch isn't related to mine.

According to the description, the regression occurs by clearing migratetype
info from page->private at __free_one_page(). If we apply your patches,
migratetype info is stored into page->index. So the migratetype info is not
cleared. Thus we do not need to cache the info locally.

Thanks,
Yasuaki Ishimatsu

> In addition, this patch don't need to be a part of this series.
>



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

Re: [PATCH v4 1/4] mm: fix tracing in free_pcppages_bulk()

[Minchan Kim]

On Wed, Sep 19, 2012 at 04:45:45PM +0900, Yasuaki Ishimatsu wrote:
> Hi Minchan,
> 
> 2012/09/19 16:32, Minchan Kim wrote:
> >Hi Yasuaki,
> >
> >On Wed, Sep 19, 2012 at 04:07:19PM +0900, Yasuaki Ishimatsu wrote:
> >>Hi Bartlomiej,
> >>
> >>2012/09/14 23:29, Bartlomiej Zolnierkiewicz wrote:
> >>>page->private gets re-used in __free_one_page() to store page order
> >>>(so trace_mm_page_pcpu_drain() may print order instead of migratetype)
> >>>thus migratetype value must be cached locally.
> >>>
> >>>Fixes regression introduced in a701623 ("mm: fix migratetype bug
> >>>which slowed swapping").
> >>
> >>I think the regression has been alreadly fixed by following Mincahn's patches.
> >>
> >>https://lkml.org/lkml/2012/9/6/635
> >>
> >>=> Hi Minchan,
> >>
> >>    Am I wrong?
> >
> >This patch isn't related to mine.
> 
> According to the description, the regression occurs by clearing migratetype
> info from page->private at __free_one_page(). If we apply your patches,
> migratetype info is stored into page->index. So the migratetype info is not
> cleared. Thus we do not need to cache the info locally.

Brain-dead. I totally forgot my implementation.
You're absolutely right but it's a just side-effect of my implementation
so description of my patch doesn't say anything about this bug.

Although my patch solve this problem by chance, we should merge 
this patch before mine and rebase mine onto this, I think.

Andrew?


-- 
Kind regards,
Minchan Kim

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

Re: [PATCH v4 1/4] mm: fix tracing in free_pcppages_bulk()

[KOSAKI Motohiro]

On Fri, Sep 14, 2012 at 10:29 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> page->private gets re-used in __free_one_page() to store page order
> (so trace_mm_page_pcpu_drain() may print order instead of migratetype)
> thus migratetype value must be cached locally.
>
> Fixes regression introduced in a701623 ("mm: fix migratetype bug
> which slowed swapping").
>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Acked-by: Minchan Kim <minchan@kernel.org>
> Acked-by: Mel Gorman <mgorman@suse.de>
> Cc: Hugh Dickins <hughd@google.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

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

Re: [PATCH v4 1/4] mm: fix tracing in free_pcppages_bulk()

[Andrew Morton]

On Fri, 14 Sep 2012 16:29:31 +0200
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:

> page->private gets re-used in __free_one_page() to store page order
> (so trace_mm_page_pcpu_drain() may print order instead of migratetype)
> thus migratetype value must be cached locally.
> 
> Fixes regression introduced in a701623 ("mm: fix migratetype bug
> which slowed swapping").

Grumble.  Please describe a bug when fixing it!  I've added here the
text "This caused incorrect data to be attached to the
mm_page_pcpu_drain trace event", which is hopefully correct enough.

As it's been this way for 2.5 years, I assume that this can wait for
3.7.

> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -668,12 +668,15 @@ static void free_pcppages_bulk(struct zone *zone, int count,
>  			batch_free = to_free;
>  
>  		do {
> +			int mt;
> +
>  			page = list_entry(list->prev, struct page, lru);
>  			/* must delete as __free_one_page list manipulates */
>  			list_del(&page->lru);
> +			mt = page_private(page);
>  			/* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
> -			__free_one_page(page, zone, 0, page_private(page));
> -			trace_mm_page_pcpu_drain(page, 0, page_private(page));
> +			__free_one_page(page, zone, 0, mt);
> +			trace_mm_page_pcpu_drain(page, 0, mt);
>  		} while (--to_free && --batch_free && !list_empty(list));
>  	}
>  	__mod_zone_page_state(zone, NR_FREE_PAGES, count);

More grumble.  Look:

akpm:/usr/src/25> grep migratetype mm/page_alloc.c | wc -l
115

We should respect the established naming conventions.  But reusing
local var `maigratetype' here is not good practice, so how about

--- a/mm/page_alloc.c~mm-fix-tracing-in-free_pcppages_bulk-fix
+++ a/mm/page_alloc.c
@@ -668,7 +668,7 @@ static void free_pcppages_bulk(struct zo
 			batch_free = to_free;
 
 		do {
-			int mt;
+			int mt;	/* migratetype of the to-be-freed page */
 
 			page = list_entry(list->prev, struct page, lru);
 			/* must delete as __free_one_page list manipulates */
_

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

Re: [PATCH v4 4/4] cma: fix watermark checking

[Andrew Morton]

On Fri, 14 Sep 2012 16:29:34 +0200
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:

> * Add ALLOC_CMA alloc flag and pass it to [__]zone_watermark_ok()
>   (from Minchan Kim).

What is its meaning and why was it added.

> * During watermark check decrease available free pages number by
>   free CMA pages number if necessary (unmovable allocations cannot
>   use pages from CMA areas).
> 
> ...
>
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -231,6 +231,21 @@ enum zone_watermarks {
>  #define low_wmark_pages(z) (z->watermark[WMARK_LOW])
>  #define high_wmark_pages(z) (z->watermark[WMARK_HIGH])
>  
> +/* The ALLOC_WMARK bits are used as an index to zone->watermark */
> +#define ALLOC_WMARK_MIN		WMARK_MIN
> +#define ALLOC_WMARK_LOW		WMARK_LOW
> +#define ALLOC_WMARK_HIGH	WMARK_HIGH
> +#define ALLOC_NO_WATERMARKS	0x04 /* don't check watermarks at all */
> +
> +/* Mask to get the watermark bits */
> +#define ALLOC_WMARK_MASK	(ALLOC_NO_WATERMARKS-1)
> +
> +#define ALLOC_HARDER		0x10 /* try to alloc harder */
> +#define ALLOC_HIGH		0x20 /* __GFP_HIGH set */
> +#define ALLOC_CPUSET		0x40 /* check for correct cpuset */
> +

Unneeded newline.

> +#define ALLOC_CMA		0x80

All the other enumerations were documented.  ALLOC_CMA was left
undocumented, despite sorely needing documentation.

>  struct per_cpu_pages {
>  	int count;		/* number of pages in the list */
>  	int high;		/* high watermark, emptying needed */
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 4b902aa..36d79ea 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -868,6 +868,7 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
>  	struct zoneref *z;
>  	struct zone *zone;
>  	int rc = COMPACT_SKIPPED;
> +	int alloc_flags = 0;
>  
>  	/*
>  	 * Check whether it is worth even starting compaction. The order check is
> @@ -879,6 +880,10 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
>  
>  	count_vm_event(COMPACTSTALL);
>  
> +#ifdef CONFIG_CMA
> +	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
> +		alloc_flags |= ALLOC_CMA;

I find this rather obscure.  What is the significance of
MIGRATE_MOVABLE here?  If it had been 

:	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_CMA)
:		alloc_flags |= ALLOC_CMA;

then I'd have read straight past it.  But it's unclear what's happening
here.  If we didn't have to resort to telepathy to understand the
meaning of ALLOC_CMA, this wouldn't be so hard.

> +#endif
>  	/* Compact each zone in the list */
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist, high_zoneidx,
>  								nodemask) {
> @@ -889,7 +894,8 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
>  		rc = max(status, rc);
>  
>  		/* If a normal allocation would succeed, stop compacting */
> -		if (zone_watermark_ok(zone, order, low_wmark_pages(zone), 0, 0))
> +		if (zone_watermark_ok(zone, order, low_wmark_pages(zone), 0,
> +				      alloc_flags))
>  			break;
>  	}
>  
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 287f79d..5985cbf 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1519,19 +1519,6 @@ failed:
>  	return NULL;
>  }
>  
> -/* The ALLOC_WMARK bits are used as an index to zone->watermark */
> -#define ALLOC_WMARK_MIN		WMARK_MIN
> -#define ALLOC_WMARK_LOW		WMARK_LOW
> -#define ALLOC_WMARK_HIGH	WMARK_HIGH
> -#define ALLOC_NO_WATERMARKS	0x04 /* don't check watermarks at all */
> -
> -/* Mask to get the watermark bits */
> -#define ALLOC_WMARK_MASK	(ALLOC_NO_WATERMARKS-1)
> -
> -#define ALLOC_HARDER		0x10 /* try to alloc harder */
> -#define ALLOC_HIGH		0x20 /* __GFP_HIGH set */
> -#define ALLOC_CPUSET		0x40 /* check for correct cpuset */

Perhaps mm/internal.h wouild have been a better place to move these.

>  #ifdef CONFIG_FAIL_PAGE_ALLOC
>  
>  static struct {
> @@ -1626,7 +1613,10 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark,
>  		min -= min / 2;
>  	if (alloc_flags & ALLOC_HARDER)
>  		min -= min / 4;
> -
> +#ifdef CONFIG_CMA
> +	if (!(alloc_flags & ALLOC_CMA))
> +		free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);

Again, the negated test looks weird or just wrong.



Please do something to make this code more understandable.

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

Re: [PATCH v4 4/4] cma: fix watermark checking

[Bartlomiej Zolnierkiewicz]

On Wednesday 19 September 2012 21:51:02 Andrew Morton wrote:
> On Fri, 14 Sep 2012 16:29:34 +0200
> Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> 
> > * Add ALLOC_CMA alloc flag and pass it to [__]zone_watermark_ok()
> >   (from Minchan Kim).
> 
> What is its meaning and why was it added.

ALLOC_CMA flag means that allocation can use CMA areas to get free pages
from.  Free CMA pages are accounted by system as normal free pages
(NR_FREE_PAGES) but they can be used only by movable type allocations
(otherwise they cannot be migrated once we want to do CMA allocation)
so we have to fix free_pages in __zone_watermark_ok() or the watermark
check will be too optimistic for unmovable allocations.

> > * During watermark check decrease available free pages number by
> >   free CMA pages number if necessary (unmovable allocations cannot
> >   use pages from CMA areas).
> > 
> > ...
> >
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -231,6 +231,21 @@ enum zone_watermarks {
> >  #define low_wmark_pages(z) (z->watermark[WMARK_LOW])
> >  #define high_wmark_pages(z) (z->watermark[WMARK_HIGH])
> >  
> > +/* The ALLOC_WMARK bits are used as an index to zone->watermark */
> > +#define ALLOC_WMARK_MIN		WMARK_MIN
> > +#define ALLOC_WMARK_LOW		WMARK_LOW
> > +#define ALLOC_WMARK_HIGH	WMARK_HIGH
> > +#define ALLOC_NO_WATERMARKS	0x04 /* don't check watermarks at all */
> > +
> > +/* Mask to get the watermark bits */
> > +#define ALLOC_WMARK_MASK	(ALLOC_NO_WATERMARKS-1)
> > +
> > +#define ALLOC_HARDER		0x10 /* try to alloc harder */
> > +#define ALLOC_HIGH		0x20 /* __GFP_HIGH set */
> > +#define ALLOC_CPUSET		0x40 /* check for correct cpuset */
> > +
> 
> Unneeded newline.
> 
> > +#define ALLOC_CMA		0x80
> 
> All the other enumerations were documented.  ALLOC_CMA was left
> undocumented, despite sorely needing documentation.
> 
> >  struct per_cpu_pages {
> >  	int count;		/* number of pages in the list */
> >  	int high;		/* high watermark, emptying needed */
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index 4b902aa..36d79ea 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -868,6 +868,7 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
> >  	struct zoneref *z;
> >  	struct zone *zone;
> >  	int rc = COMPACT_SKIPPED;
> > +	int alloc_flags = 0;
> >  
> >  	/*
> >  	 * Check whether it is worth even starting compaction. The order check is
> > @@ -879,6 +880,10 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
> >  
> >  	count_vm_event(COMPACTSTALL);
> >  
> > +#ifdef CONFIG_CMA
> > +	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
> > +		alloc_flags |= ALLOC_CMA;
> 
> I find this rather obscure.  What is the significance of
> MIGRATE_MOVABLE here?  If it had been 
> 
> :	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_CMA)
> :		alloc_flags |= ALLOC_CMA;
> 
> then I'd have read straight past it.  But it's unclear what's happening
> here.  If we didn't have to resort to telepathy to understand the
> meaning of ALLOC_CMA, this wouldn't be so hard.
> 
> > +#endif
> >  	/* Compact each zone in the list */
> >  	for_each_zone_zonelist_nodemask(zone, z, zonelist, high_zoneidx,
> >  								nodemask) {
> > @@ -889,7 +894,8 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
> >  		rc = max(status, rc);
> >  
> >  		/* If a normal allocation would succeed, stop compacting */
> > -		if (zone_watermark_ok(zone, order, low_wmark_pages(zone), 0, 0))
> > +		if (zone_watermark_ok(zone, order, low_wmark_pages(zone), 0,
> > +				      alloc_flags))
> >  			break;
> >  	}
> >  
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 287f79d..5985cbf 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1519,19 +1519,6 @@ failed:
> >  	return NULL;
> >  }
> >  
> > -/* The ALLOC_WMARK bits are used as an index to zone->watermark */
> > -#define ALLOC_WMARK_MIN		WMARK_MIN
> > -#define ALLOC_WMARK_LOW		WMARK_LOW
> > -#define ALLOC_WMARK_HIGH	WMARK_HIGH
> > -#define ALLOC_NO_WATERMARKS	0x04 /* don't check watermarks at all */
> > -
> > -/* Mask to get the watermark bits */
> > -#define ALLOC_WMARK_MASK	(ALLOC_NO_WATERMARKS-1)
> > -
> > -#define ALLOC_HARDER		0x10 /* try to alloc harder */
> > -#define ALLOC_HIGH		0x20 /* __GFP_HIGH set */
> > -#define ALLOC_CPUSET		0x40 /* check for correct cpuset */
> 
> Perhaps mm/internal.h wouild have been a better place to move these.
> 
> >  #ifdef CONFIG_FAIL_PAGE_ALLOC
> >  
> >  static struct {
> > @@ -1626,7 +1613,10 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark,
> >  		min -= min / 2;
> >  	if (alloc_flags & ALLOC_HARDER)
> >  		min -= min / 4;
> > -
> > +#ifdef CONFIG_CMA
> > +	if (!(alloc_flags & ALLOC_CMA))
> > +		free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
> 
> Again, the negated test looks weird or just wrong.
> 
> 
> 
> Please do something to make this code more understandable.

Here is an incremental patch:

From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: [PATCH] cma: fix watermark checking cleanup

Changes:
* document ALLOC_CMA
* add comment to __zone_watermark_ok()
* move ALLOC_* defines to mm/internal.h

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 include/linux/mmzone.h |   15 ---------------
 mm/internal.h          |   14 ++++++++++++++
 mm/page_alloc.c        |    1 +
 3 files changed, 15 insertions(+), 15 deletions(-)

Index: b/include/linux/mmzone.h
===================================================================
--- a/include/linux/mmzone.h	2012-09-24 11:09:10.700992708 +0200
+++ b/include/linux/mmzone.h	2012-09-24 11:11:36.520992691 +0200
@@ -231,21 +231,6 @@ enum zone_watermarks {
 #define low_wmark_pages(z) (z->watermark[WMARK_LOW])
 #define high_wmark_pages(z) (z->watermark[WMARK_HIGH])
 
-/* The ALLOC_WMARK bits are used as an index to zone->watermark */
-#define ALLOC_WMARK_MIN		WMARK_MIN
-#define ALLOC_WMARK_LOW		WMARK_LOW
-#define ALLOC_WMARK_HIGH	WMARK_HIGH
-#define ALLOC_NO_WATERMARKS	0x04 /* don't check watermarks at all */
-
-/* Mask to get the watermark bits */
-#define ALLOC_WMARK_MASK	(ALLOC_NO_WATERMARKS-1)
-
-#define ALLOC_HARDER		0x10 /* try to alloc harder */
-#define ALLOC_HIGH		0x20 /* __GFP_HIGH set */
-#define ALLOC_CPUSET		0x40 /* check for correct cpuset */
-
-#define ALLOC_CMA		0x80
-
 struct per_cpu_pages {
 	int count;		/* number of pages in the list */
 	int high;		/* high watermark, emptying needed */
Index: b/mm/internal.h
===================================================================
--- a/mm/internal.h	2012-09-24 11:10:04.848992709 +0200
+++ b/mm/internal.h	2012-09-24 11:11:39.972992691 +0200
@@ -356,3 +356,17 @@ extern unsigned long vm_mmap_pgoff(struc
         unsigned long, unsigned long);
 
 extern void set_pageblock_order(void);
+
+/* The ALLOC_WMARK bits are used as an index to zone->watermark */
+#define ALLOC_WMARK_MIN		WMARK_MIN
+#define ALLOC_WMARK_LOW		WMARK_LOW
+#define ALLOC_WMARK_HIGH	WMARK_HIGH
+#define ALLOC_NO_WATERMARKS	0x04 /* don't check watermarks at all */
+
+/* Mask to get the watermark bits */
+#define ALLOC_WMARK_MASK	(ALLOC_NO_WATERMARKS-1)
+
+#define ALLOC_HARDER		0x10 /* try to alloc harder */
+#define ALLOC_HIGH		0x20 /* __GFP_HIGH set */
+#define ALLOC_CPUSET		0x40 /* check for correct cpuset */
+#define ALLOC_CMA		0x80 /* allow allocations from CMA areas */
Index: b/mm/page_alloc.c
===================================================================
--- a/mm/page_alloc.c	2012-09-24 11:12:22.212992717 +0200
+++ b/mm/page_alloc.c	2012-09-24 11:15:28.212992661 +0200
@@ -1614,6 +1614,7 @@ static bool __zone_watermark_ok(struct z
 	if (alloc_flags & ALLOC_HARDER)
 		min -= min / 4;
 #ifdef CONFIG_CMA
+	/* If allocation can't use CMA areas don't use free CMA pages */
 	if (!(alloc_flags & ALLOC_CMA))
 		free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
 #endif

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

Re: [PATCH v4 4/4] cma: fix watermark checking

[Andrew Morton]

On Mon, 24 Sep 2012 11:30:43 +0200
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:

> On Wednesday 19 September 2012 21:51:02 Andrew Morton wrote:
> > >  
> > >  	/*
> > >  	 * Check whether it is worth even starting compaction. The order check is
> > > @@ -879,6 +880,10 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
> > >  
> > >  	count_vm_event(COMPACTSTALL);
> > >  
> > > +#ifdef CONFIG_CMA
> > > +	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
> > > +		alloc_flags |= ALLOC_CMA;
> > 
> > I find this rather obscure.  What is the significance of
> > MIGRATE_MOVABLE here?  If it had been 
> > 
> > :	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_CMA)
> > :		alloc_flags |= ALLOC_CMA;
> > 
> > then I'd have read straight past it.  But it's unclear what's happening
> > here.  If we didn't have to resort to telepathy to understand the
> > meaning of ALLOC_CMA, this wouldn't be so hard.

This?

Or am I being more than usually thick?  Is everyone else finding

	if (allocflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
		alloc_flags |= ALLOC_CMA;

to be blindingly obvious?


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