On Mon, 13 Dec 2010 12:26:47 +0100
Michal Nazarewicz [off-list ref] wrote:

quoted

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h

@@ -35,13 +35,24 @@
  */
 #define PAGE_ALLOC_COSTLY_ORDER 3
 
-#define MIGRATE_UNMOVABLE     0
-#define MIGRATE_RECLAIMABLE   1
-#define MIGRATE_MOVABLE       2
-#define MIGRATE_PCPTYPES      3 /* the number of types on the pcp lists */
-#define MIGRATE_RESERVE       3
-#define MIGRATE_ISOLATE       4 /* can't allocate from here */
-#define MIGRATE_TYPES         5
+enum {
+	MIGRATE_UNMOVABLE,
+	MIGRATE_RECLAIMABLE,
+	MIGRATE_MOVABLE,
+	MIGRATE_PCPTYPES,	/* the number of types on the pcp lists */
+	MIGRATE_RESERVE = MIGRATE_PCPTYPES,
+	MIGRATE_ISOLATE,	/* can't allocate from here */
+#ifdef CONFIG_MIGRATE_CMA
+	MIGRATE_CMA,		/* only movable */
+#endif
+	MIGRATE_TYPES
+};

KAMEZAWA Hiroyuki [off-list ref] writes:

I personaly would like to put MIGRATE_ISOLATE to be bottom of enum
because it means _not_for_allocation.

Will change.  I didn't want to change the value of MIGRATE_ISOLATE in
fear of breaking something but hopefully no one depends on
MIGRATE_ISOLATE's value. 

quoted

diff --git a/mm/compaction.c b/mm/compaction.c

@@ -824,11 +848,15 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,
  * This array describes the order lists are fallen back to when
  * the free lists for the desirable migrate type are depleted
  */
-static int fallbacks[MIGRATE_TYPES][MIGRATE_TYPES-1] = {
+static int fallbacks[MIGRATE_TYPES][4] = {
 	[MIGRATE_UNMOVABLE]   = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE,   MIGRATE_RESERVE },
 	[MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE,   MIGRATE_MOVABLE,   MIGRATE_RESERVE },
+#ifdef CONFIG_MIGRATE_CMA
+	[MIGRATE_MOVABLE]     = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_CMA    , MIGRATE_RESERVE },
+#else
 	[MIGRATE_MOVABLE]     = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE },
-	[MIGRATE_RESERVE]     = { MIGRATE_RESERVE,     MIGRATE_RESERVE,   MIGRATE_RESERVE }, /* Never used */
+#endif
+	[MIGRATE_RESERVE]     = { MIGRATE_RESERVE }, /* Never used */
 };
 
 /*

@@ -924,12 +952,12 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
 	/* Find the largest possible block of pages in the other list */
 	for (current_order = MAX_ORDER-1; current_order >= order;
 						--current_order) {
-		for (i = 0; i < MIGRATE_TYPES - 1; i++) {
+		for (i = 0; i < ARRAY_SIZE(fallbacks[0]); i++) {

Why fallbacks[0] ? and why do you need to change this ?

I've changed the dimensions of fallbacks matrix, in particular second
dimension from MIGRATE_TYPE - 1 to 4 so this place needed to be changed
as well.  Now, I think changing to ARRAY_SIZE() is just the safest
option available.  This is actually just a minor optimisation.

quoted

 			migratetype = fallbacks[start_migratetype][i];
 
 			/* MIGRATE_RESERVE handled later if necessary */
 			if (migratetype == MIGRATE_RESERVE)
-				continue;
+				break;
 

Isn't this change enough for your purpose ?

This is mostly just an optimisation really.  I'm not sure what you think
is my purpose here. ;)  It does fix an this issue of some of the
fallback[*] arrays having MIGRATETYPE_UNMOVABLE at the end.

quoted

@@ -1042,7 +1083,12 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
 			list_add(&page->lru, list);
 		else
 			list_add_tail(&page->lru, list);
-		set_page_private(page, migratetype);
+#ifdef CONFIG_MIGRATE_CMA
+		if (is_pageblock_cma(page))
+			set_page_private(page, MIGRATE_CMA);
+		else
+#endif
+			set_page_private(page, migratetype);

Hmm, doesn't this meet your changes which makes MIGRATE_CMA >
MIGRATE_PCPLIST ?  And I think putting mixture of pages marked of
MIGRATE_TYPE onto a pcplist is ugly.

You mean that pcplist page is on can disagree with page_private()?
I didn't think this was such a big deal honestly.  Unless MIGRATE_CMA <
MIGRATE_PCPTYPES, a special case is needed either here or in
free_pcppages_bulk(), so I think this comes down to whether to make
MIGRATE_CAM < MIGRATE_PCPTYPES for which see below.

quoted

@@ -1181,9 +1227,16 @@ void free_hot_cold_page(struct page *page, int cold)
 	 * offlined but treat RESERVE as movable pages so we can get those
 	 * areas back if necessary. Otherwise, we may have to free
 	 * excessively into the page allocator
+	 *
+	 * Still, do not change migration type of MIGRATE_CMA pages (if
+	 * they'd be recorded as MIGRATE_MOVABLE an unmovable page could
+	 * be allocated from MIGRATE_CMA block and we don't want to allow
+	 * that).  In this respect, treat MIGRATE_CMA like
+	 * MIGRATE_ISOLATE.
 	 */
 	if (migratetype >= MIGRATE_PCPTYPES) {
-		if (unlikely(migratetype == MIGRATE_ISOLATE)) {
+		if (unlikely(migratetype == MIGRATE_ISOLATE
+			  || is_migrate_cma(migratetype))) {
 			free_one_page(zone, page, 0, migratetype);
 			goto out;
 		}

Doesn't this add *BAD* performance impact for usual use of pages
marked as MIGRATE_CMA ? IIUC, All pcp pages must be _drained_ at page
migration after making migrate_type as ISOLATED. So, this change
should be unnecessary.

Come to think of it, it would appear that you are right.  I'll remove
this change.

BTW, How about changing MIGRATE_CMA < MIGRATE_PCPTYPES ? and allow to
have it's own pcp list ?

I think
==
 again:
         if (likely(order == 0)) {
                 struct per_cpu_pages *pcp;
                 struct list_head *list; 
 
                 local_irq_save(flags);
                 pcp = &this_cpu_ptr(zone->pageset)->pcp;
                 list = &pcp->lists[migratetype];
                 if (list_empty(list)) {
                         pcp->count += rmqueue_bulk(zone, 0,
                                         pcp->batch, list,
                                         migratetype, cold);
                         if (unlikely(list_empty(list))) {
+				 if (migrate_type == MIGRATE_MOVABLE) { /*allow extra fallback*/
+					migrate_type == MIGRATE_CMA
+					goto again;

(This unbalances local_irq_save but that's just a minor note.)

+			 	}
+			 }
                                 goto failed;
                 }

                if (cold)
                        page = list_entry(list->prev, struct page, lru);
                else
                        page = list_entry(list->next, struct page, lru);

                list_del(&page->lru);
                pcp->count--;
==
Will work enough as a fallback path which allows to allocate a memory
from CMA area if there aren't enough free pages. (and makes FALLBACK
type as)

fallbacks[MIGRATE_CMA] = {?????},

for no fallbacks.

Yes, I think that would work.  I didn't want to create a new pcp list
especially since in most respects it behaves just like MIGRATE_MOVABLE.
Moreover, with MIGRATE_MOVABLE and MIGRATE_CMA sharing the same pcp list
the above additional fallback path is not necessary and instead the
already existing __rmqueue_fallback() path can be used.

quoted

@@ -1272,7 +1325,8 @@ int split_free_page(struct page *page)
 	if (order >= pageblock_order - 1) {
 		struct page *endpage = page + (1 << order) - 1;
 		for (; page < endpage; page += pageblock_nr_pages)
-			set_pageblock_migratetype(page, MIGRATE_MOVABLE);
+			if (!is_pageblock_cma(page))
+				set_pageblock_migratetype(page, MIGRATE_MOVABLE);
 	}
 
 	return 1 << order;

@@ -5366,6 +5420,15 @@ int set_migratetype_isolate(struct page *page)
 	zone_idx = zone_idx(zone);
 
 	spin_lock_irqsave(&zone->lock, flags);
+	/*
+	 * Treat MIGRATE_CMA specially since it may contain immobile
+	 * CMA pages -- that's fine.  CMA is likely going to touch
+	 * only the mobile pages in the pageblokc.
+	 */
+	if (is_pageblock_cma(page)) {
+		ret = 0;
+		goto out;
+	}
 
 	pfn = page_to_pfn(page);
 	arg.start_pfn = pfn;

Hmm, I'm not sure why you dont' have any change in __free_one_page()
which overwrite pageblock type. Is MIGRATE_CMA range is aligned to
MAX_ORDER ? If so, please mention about it in patch description or
comment because of the patch order.

Yep, you're correct.  For MIGRATE_CMA to be usable, pages marked with it
must be aligned to MAX_ORDER_NR_PAGES.  I'll add that in a comment
somewhere.

-- 
Best regards,                                         _     _
 .o. | Liege of Serenly Enlightened Majesty of      o' \,=./ `o
 ..o | Computer Science,  Michal "mina86" Nazarewicz   (o o)
 ooo +--<mina86-tlen.pl>--<jid:mina86-jabber.org>--ooO--(_)--Ooo--