On Thu, Oct 06, 2011 at 03:54:41PM +0200, Marek Szyprowski wrote:

From: KAMEZAWA Hiroyuki kamezawa.hiroyu@jp.fujitsu.com

Memory hotplug is a logic for making pages unused in the specified range of pfn. So, some of core logics can be used for other purpose as allocating a very large contigous memory block.

This patch moves some functions from mm/memory_hotplug.c to mm/page_isolation.c. This helps adding a function for large-alloc in page_isolation.c with memory-unplug technique.

Signed-off-by: KAMEZAWA Hiroyuki kamezawa.hiroyu@jp.fujitsu.com [m.nazarewicz: reworded commit message] Signed-off-by: Michal Nazarewicz m.nazarewicz@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com [m.szyprowski: rebased and updated to Linux v3.0-rc1] Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com CC: Michal Nazarewicz mina86@mina86.com Acked-by: Arnd Bergmann arnd@arndb.de

---

include/linux/page-isolation.h | 7 +++ mm/memory_hotplug.c | 111 -------------------------------------- mm/page_isolation.c | 114 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 111 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h index 051c1b1..58cdbac 100644 --- a/include/linux/page-isolation.h +++ b/include/linux/page-isolation.h @@ -33,5 +33,12 @@ test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn); extern int set_migratetype_isolate(struct page *page); extern void unset_migratetype_isolate(struct page *page); +/*

• • For migration.
• */

+int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn);

bool

+unsigned long scan_lru_pages(unsigned long start, unsigned long end);

Both function names are misleading.

+int do_migrate_range(unsigned long start_pfn, unsigned long end_pfn);

scan_lru_pages as it is used by memory hotplug is also extremely expensive. Unplugging memory is rare so the performance is not a concern but it make be for CMA.

I think it would have made more sense to either express this as an iterator like for_each_lru_page_in_range() and use cursors or reuse the compaction code. As it is, these functions are a bit rough. I'm biased but this code seems to have very similar responsibilities to the compaction.c code for isolate_migratepages and how it handles migration. It also knows how to avoid isolating so much memory as to put the system in the risk of being livelocked, isolate pages from the LRU in batch etc.

This is not a show-stopper as such but personally I would prefer that the memory hotplug code be sharing code with compaction than CMA adding a new dependency on memory hotplug.

At this point, I'm going to apologise for not reviewing this a long long time ago.

On Thu, Oct 06, 2011 at 03:54:42PM +0200, Marek Szyprowski wrote:

From: KAMEZAWA Hiroyuki kamezawa.hiroyu@jp.fujitsu.com

This commit introduces alloc_contig_freed_pages() function which allocates (ie. removes from buddy system) free pages in range. Caller has to guarantee that all pages in range are in buddy system.

Straight away, I'm wondering why you didn't use

mm/compaction.c#isolate_freepages()

It knows how to isolate pages within ranges. All its control information is passed via struct compact_control() which I recognise may be awkward for CMA but compaction.c know how to manage all the isolated pages and pass them to migrate.c appropriately.

I haven't read all the patches yet but isolate_freepages() does break everything up into order-0 pages. This may not be to your liking but it would not be possible to change.

Along with this function, a free_contig_pages() function is provided which frees all (or a subset of) pages allocated with alloc_contig_free_pages().

mm/compaction.c#release_freepages()

Michal Nazarewicz has modified the function to make it easier to allocate not MAX_ORDER_NR_PAGES aligned pages by making it return pfn of one-past-the-last allocated page.

Signed-off-by: KAMEZAWA Hiroyuki kamezawa.hiroyu@jp.fujitsu.com Signed-off-by: Michal Nazarewicz m.nazarewicz@samsung.com [m.nazarewicz: added checks if all allocated pages comes from the same memory zone] Signed-off-by: Michal Nazarewicz mina86@mina86.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com [m.szyprowski: fixed wrong condition in VM_BUG_ON assert] Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Arnd Bergmann arnd@arndb.de

---

include/linux/mmzone.h | 16 +++++++++ include/linux/page-isolation.h | 5 +++ mm/page_alloc.c | 67 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index a2760bb..862a834 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -1168,6 +1168,22 @@ static inline int memmap_valid_within(unsigned long pfn, } #endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */ +#if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) +/*

• • Both PFNs must be from the same zone! If this function returns
• • true, pfn_to_page(pfn1) + (pfn2 - pfn1) == pfn_to_page(pfn2).
• */

+static inline bool zone_pfn_same_memmap(unsigned long pfn1, unsigned long pfn2) +{

• return pfn_to_section_nr(pfn1) == pfn_to_section_nr(pfn2);

+}

Why do you care what section the page is in? The zone is important all right, but not the section. Also, offhand I'm unsure if being in the same section guarantees the same zone. sections are ordinarily fully populated (except on ARM but hey) but I can't remember anything enforcing that zones be section-aligned.

Later I think I see that the intention was to reduce the use of pfn_to_page(). You can do this in a more general fashion by checking the zone boundaries and resolving the pfn->page every MAX_ORDER_NR_PAGES. That will not be SPARSEMEM specific.

+#else

+#define zone_pfn_same_memmap(pfn1, pfn2) (true)

+#endif

#endif /* !__GENERATING_BOUNDS.H */ #endif /* !__ASSEMBLY__ */ #endif /* _LINUX_MMZONE_H */ diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h index 58cdbac..b9fc428 100644 --- a/include/linux/page-isolation.h +++ b/include/linux/page-isolation.h @@ -33,6 +33,11 @@ test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn); extern int set_migratetype_isolate(struct page *page); extern void unset_migratetype_isolate(struct page *page); +/* The below functions must be run on a range from a single zone. */ +extern unsigned long alloc_contig_freed_pages(unsigned long start,

• 			      unsigned long end, gfp_t flag);
  

+extern void free_contig_pages(unsigned long pfn, unsigned nr_pages);

/*

• For migration.

*/ diff --git a/mm/page_alloc.c b/mm/page_alloc.c index bf4399a..fbfb920 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -5706,6 +5706,73 @@ out: spin_unlock_irqrestore(&zone->lock, flags); } +unsigned long alloc_contig_freed_pages(unsigned long start, unsigned long end,

• 		       gfp_t flag)
  

+{

• unsigned long pfn = start, count;
• struct page *page;
• struct zone *zone;
• int order;
• VM_BUG_ON(!pfn_valid(start));

VM_BUG_ON seems very harsh here. WARN_ON_ONCE and returning 0 to the caller sees reasonable.

• page = pfn_to_page(start);
• zone = page_zone(page);
• spin_lock_irq(&zone->lock);
• for (;;) {

• VM_BUG_ON(page_count(page) || !PageBuddy(page) ||
  

• 	  page_zone(page) != zone);
  

Here you will VM_BUG_ON with the zone lock held leading to system halting very shortly.

• list_del(&page->lru);
  

• order = page_order(page);
  

• count = 1UL << order;
  

• zone->free_area[order].nr_free--;
  

• rmv_page_order(page);
  

• __mod_zone_page_state(zone, NR_FREE_PAGES, -(long)count);
  

The callers need to check in advance if watermarks are sufficient for this. In compaction, it happens in compaction_suitable() because it only needed to be checked once. Your requirements might be different.

• pfn += count;
  

• if (pfn >= end)
  

• 	break;
  

• VM_BUG_ON(!pfn_valid(pfn));
  

On ARM, it's possible to encounter invalid pages. VM_BUG_ON is serious overkill.

• if (zone_pfn_same_memmap(pfn - count, pfn))
  

• 	page += count;
  

• else
  

• 	page = pfn_to_page(pfn);
  

• }
• spin_unlock_irq(&zone->lock);
• /* After this, pages in the range can be freed one be one */
• count = pfn - start;
• pfn = start;
• for (page = pfn_to_page(pfn); count; --count) {

• prep_new_page(page, 0, flag);
  

• ++pfn;
  

• if (likely(zone_pfn_same_memmap(pfn - 1, pfn)))
  

• 	++page;
  

• else
  

• 	page = pfn_to_page(pfn);
  

• }

Here it looks like you have implemented something like split_free_page().

• return pfn;

+}

+void free_contig_pages(unsigned long pfn, unsigned nr_pages) +{

• struct page *page = pfn_to_page(pfn);
• while (nr_pages--) {

• __free_page(page);
  

• ++pfn;
  

• if (likely(zone_pfn_same_memmap(pfn - 1, pfn)))
  

• 	++page;
  

• else
  

• 	page = pfn_to_page(pfn);
  

• }

+}

#ifdef CONFIG_MEMORY_HOTREMOVE /*

• All pages in the range must be isolated before calling this.

On Thu, Oct 06, 2011 at 03:54:42PM +0200, Marek Szyprowski wrote:

...

This commit introduces alloc_contig_freed_pages() function which allocates (ie. removes from buddy system) free pages in range. Caller has to guarantee that all pages in range are in buddy system.

On Tue, 18 Oct 2011 05:21:09 -0700, Mel Gorman mel@csn.ul.ie wrote:

Straight away, I'm wondering why you didn't use mm/compaction.c#isolate_freepages()

Does the below look like a step in the right direction?

It basically moves isolate_freepages_block() to page_alloc.c (changing it name to isolate_freepages_range()) and changes it so that depending on arguments it treats holes (either invalid PFN or non-free page) as errors so that CMA can use it.

It also accepts a range rather then just assuming a single pageblock thus the change moves range calculation in compaction.c from isolate_freepages_block() up to isolate_freepages().

The change also modifies spilt_free_page() so that it does not try to change pageblock's migrate type if current migrate type is ISOLATE or CMA.

--- include/linux/mm.h | 1 - include/linux/page-isolation.h | 4 +- mm/compaction.c | 73 +++-------------------- mm/internal.h | 5 ++ mm/page_alloc.c | 128 +++++++++++++++++++++++++--------------- 5 files changed, 95 insertions(+), 116 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h index fd599f4..98c99c4 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -435,7 +435,6 @@ void put_page(struct page *page); void put_pages_list(struct list_head *pages);

void split_page(struct page *page, unsigned int order); -int split_free_page(struct page *page);

/* * Compound pages have a destructor function. Provide a diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h index 003c52f..6becc74 100644 --- a/include/linux/page-isolation.h +++ b/include/linux/page-isolation.h @@ -48,10 +48,8 @@ static inline void unset_migratetype_isolate(struct page *page) }

/* The below functions must be run on a range from a single zone. */ -extern unsigned long alloc_contig_freed_pages(unsigned long start, - unsigned long end, gfp_t flag); extern int alloc_contig_range(unsigned long start, unsigned long end, - gfp_t flags, unsigned migratetype); + unsigned migratetype); extern void free_contig_pages(unsigned long pfn, unsigned nr_pages);

/* diff --git a/mm/compaction.c b/mm/compaction.c index 9e5cc59..685a19e 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -58,77 +58,15 @@ static unsigned long release_freepages(struct list_head *freelist) return count; }

-/* Isolate free pages onto a private freelist. Must hold zone->lock */ -static unsigned long isolate_freepages_block(struct zone *zone, - unsigned long blockpfn, - struct list_head *freelist) -{ - unsigned long zone_end_pfn, end_pfn; - int nr_scanned = 0, total_isolated = 0; - struct page *cursor; - - /* Get the last PFN we should scan for free pages at */ - zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages; - end_pfn = min(blockpfn + pageblock_nr_pages, zone_end_pfn); - - /* Find the first usable PFN in the block to initialse page cursor */ - for (; blockpfn < end_pfn; blockpfn++) { - if (pfn_valid_within(blockpfn)) - break; - } - cursor = pfn_to_page(blockpfn); - - /* Isolate free pages. This assumes the block is valid */ - for (; blockpfn < end_pfn; blockpfn++, cursor++) { - int isolated, i; - struct page *page = cursor; - - if (!pfn_valid_within(blockpfn)) - continue; - nr_scanned++; - - if (!PageBuddy(page)) - continue; - - /* Found a free page, break it into order-0 pages */ - isolated = split_free_page(page); - total_isolated += isolated; - for (i = 0; i < isolated; i++) { - list_add(&page->lru, freelist); - page++; - } - - /* If a page was split, advance to the end of it */ - if (isolated) { - blockpfn += isolated - 1; - cursor += isolated - 1; - } - } - - trace_mm_compaction_isolate_freepages(nr_scanned, total_isolated); - return total_isolated; -} - /* Returns true if the page is within a block suitable for migration to */ 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) return false;

- /* Keep MIGRATE_CMA alone as well. */ - /* - * XXX Revisit. We currently cannot let compaction touch CMA - * pages since compaction insists on changing their migration - * type to MIGRATE_MOVABLE (see split_free_page() called from - * isolate_freepages_block() above). - */ - if (is_migrate_cma(migratetype)) - return false; - /* If the page is a large free page, then allow migration */ if (PageBuddy(page) && page_order(page) >= pageblock_order) return true; @@ -149,7 +87,7 @@ static void isolate_freepages(struct zone *zone, struct compact_control *cc) { struct page *page; - unsigned long high_pfn, low_pfn, pfn; + unsigned long high_pfn, low_pfn, pfn, zone_end_pfn, end_pfn; unsigned long flags; int nr_freepages = cc->nr_freepages; struct list_head *freelist = &cc->freepages; @@ -169,6 +107,8 @@ static void isolate_freepages(struct zone *zone, */ high_pfn = min(low_pfn, pfn);

+ zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages; + /* * Isolate free pages until enough are available to migrate the * pages on cc->migratepages. We stop searching if the migrate @@ -176,7 +116,7 @@ static void isolate_freepages(struct zone *zone, */ for (; pfn > low_pfn && cc->nr_migratepages > nr_freepages; pfn -= pageblock_nr_pages) { - unsigned long isolated; + unsigned isolated, scanned;

if (!pfn_valid(pfn)) continue; @@ -205,7 +145,10 @@ static void isolate_freepages(struct zone *zone, isolated = 0; spin_lock_irqsave(&zone->lock, flags); if (suitable_migration_target(page)) { - isolated = isolate_freepages_block(zone, pfn, freelist); + end_pfn = min(pfn + pageblock_nr_pages, zone_end_pfn); + isolated = isolate_freepages_range(zone, pfn, + end_pfn, freelist, &scanned); + trace_mm_compaction_isolate_freepages(scanned, isolated); nr_freepages += isolated; } spin_unlock_irqrestore(&zone->lock, flags); diff --git a/mm/internal.h b/mm/internal.h index d071d380..4a9bb3f 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -263,3 +263,8 @@ extern u64 hwpoison_filter_flags_mask; extern u64 hwpoison_filter_flags_value; extern u64 hwpoison_filter_memcg; extern u32 hwpoison_filter_enable; + +unsigned isolate_freepages_range(struct zone *zone, + unsigned long start, unsigned long end, + struct list_head *freelist, + unsigned *scannedp); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index df69706..adf3f34 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1300,10 +1300,11 @@ void split_page(struct page *page, unsigned int order) * Note: this is probably too low level an operation for use in drivers. * Please consult with lkml before using this in your driver. */ -int split_free_page(struct page *page) +static unsigned split_free_page(struct page *page) { unsigned int order; unsigned long watermark; + struct page *endpage; struct zone *zone;

BUG_ON(!PageBuddy(page)); @@ -1326,14 +1327,18 @@ int split_free_page(struct page *page) set_page_refcounted(page); split_page(page, order);

- if (order >= pageblock_order - 1) { - struct page *endpage = page + (1 << order) - 1; - for (; page < endpage; page += pageblock_nr_pages) - if (!is_pageblock_cma(page)) - set_pageblock_migratetype(page, - MIGRATE_MOVABLE); + if (order < pageblock_order - 1) + goto done; + + endpage = page + (1 << order) - 1; + for (; page < endpage; page += pageblock_nr_pages) { + int mt = get_pageblock_migratetype(page); + /* Don't change CMA nor ISOLATE */ + if (!is_migrate_cma(mt) && mt != MIGRATE_ISOLATE) + set_pageblock_migratetype(page, MIGRATE_MOVABLE); }

+done: return 1 << order; }

@@ -5723,57 +5728,76 @@ out: spin_unlock_irqrestore(&zone->lock, flags); }

-unsigned long alloc_contig_freed_pages(unsigned long start, unsigned long end, - gfp_t flag) +/** + * isolate_freepages_range() - isolate free pages, must hold zone->lock. + * @zone: Zone pages are in. + * @start: The first PFN to start isolating. + * @end: The one-past-last PFN. + * @freelist: A list to save isolated pages to. + * @scannedp: Optional pointer where to save number of scanned pages. + * + * If @freelist is not provided, holes in range (either non-free pages + * or invalid PFN) are considered an error and function undos its + * actions and returns zero. + * + * If @freelist is provided, function will simply skip non-free and + * missing pages and put only the ones isolated on the list. It will + * also call trace_mm_compaction_isolate_freepages() at the end. + * + * Returns number of isolated pages. This may be more then end-start + * if end fell in a middle of a free page. + */ +unsigned isolate_freepages_range(struct zone *zone, + unsigned long start, unsigned long end, + struct list_head *freelist, unsigned *scannedp) { - unsigned long pfn = start, count; + unsigned nr_scanned = 0, total_isolated = 0; + unsigned long pfn = start; struct page *page; - struct zone *zone; - int order;

VM_BUG_ON(!pfn_valid(start)); - page = pfn_to_page(start); - zone = page_zone(page);

- spin_lock_irq(&zone->lock); + /* Isolate free pages. This assumes the block is valid */ + page = pfn_to_page(pfn); + while (pfn < end) { + unsigned isolated = 1;

- for (;;) { - VM_BUG_ON(!page_count(page) || !PageBuddy(page) || - page_zone(page) != zone); + VM_BUG_ON(page_zone(page) != zone);

- list_del(&page->lru); - order = page_order(page); - count = 1UL << order; - zone->free_area[order].nr_free--; - rmv_page_order(page); - __mod_zone_page_state(zone, NR_FREE_PAGES, -(long)count); + if (!pfn_valid_within(blockpfn)) + goto skip; + ++nr_scanned;

- pfn += count; - if (pfn >= end) - break; - VM_BUG_ON(!pfn_valid(pfn)); - - if (zone_pfn_same_memmap(pfn - count, pfn)) - page += count; - else - page = pfn_to_page(pfn); - } + if (!PageBuddy(page)) { +skip: + if (freelist) + goto next; + for (; start < pfn; ++start) + __free_page(pfn_to_page(pfn)); + return 0; + }

- spin_unlock_irq(&zone->lock); + /* Found a free page, break it into order-0 pages */ + isolated = split_free_page(page); + total_isolated += isolated; + if (freelist) { + struct page *p = page; + unsigned i = isolated; + for (; i--; ++page) + list_add(&p->lru, freelist); + }

- /* After this, pages in the range can be freed one be one */ - count = pfn - start; - pfn = start; - for (page = pfn_to_page(pfn); count; --count) { - prep_new_page(page, 0, flag); - ++pfn; - if (likely(zone_pfn_same_memmap(pfn - 1, pfn))) - ++page; +next: /* Advance to the next page */ + pfn += isolated; + if (zone_pfn_same_memmap(pfn - isolated, pfn)) + page += isolated; else page = pfn_to_page(pfn); }

- return pfn; + if (scannedp) + *scannedp = nr_scanned; + return total_isolated; }

static unsigned long pfn_to_maxpage(unsigned long pfn) @@ -5837,7 +5861,6 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end) * alloc_contig_range() -- tries to allocate given range of pages * @start: start PFN to allocate * @end: one-past-the-last PFN to allocate - * @flags: flags passed to alloc_contig_freed_pages(). * @migratetype: migratetype of the underlaying pageblocks (either * #MIGRATE_MOVABLE or #MIGRATE_CMA). All pageblocks * in range must have the same migratetype and it must @@ -5853,9 +5876,10 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end) * need to be freed with free_contig_pages(). */ int alloc_contig_range(unsigned long start, unsigned long end, - gfp_t flags, unsigned migratetype) + unsigned migratetype) { unsigned long outer_start, outer_end; + struct zone *zone; int ret;

/* @@ -5910,7 +5934,17 @@ int alloc_contig_range(unsigned long start, unsigned long end, return -EINVAL;

outer_start = start & (~0UL << ret); - outer_end = alloc_contig_freed_pages(outer_start, end, flags); + + zone = page_zone(pfn_to_page(outer_start)); + spin_lock_irq(&zone->lock); + outer_end = isolate_freepages_range(zone, outer_start, end, NULL, NULL); + spin_unlock_irq(&zone->lock); + + if (!outer_end) { + ret = -EBUSY; + goto done; + } + outer_end += outer_start;

/* Free head and tail (if any) */ if (start != outer_start)

On Thu, Oct 06, 2011 at 03:54:43PM +0200, Marek Szyprowski wrote:

From: Michal Nazarewicz m.nazarewicz@samsung.com

This commit adds the alloc_contig_range() function which tries to allocate given range of pages. It tries to migrate all already allocated pages that fall in the range thus freeing them. Once all pages in the range are freed they are removed from the buddy system thus allocated for the caller to use.

Signed-off-by: Michal Nazarewicz m.nazarewicz@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com [m.szyprowski: renamed some variables for easier code reading] Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com CC: Michal Nazarewicz mina86@mina86.com Acked-by: Arnd Bergmann arnd@arndb.de

---

include/linux/page-isolation.h | 2 + mm/page_alloc.c | 148 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 0 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h index b9fc428..774ecec 100644 --- a/include/linux/page-isolation.h +++ b/include/linux/page-isolation.h @@ -36,6 +36,8 @@ extern void unset_migratetype_isolate(struct page *page); /* The below functions must be run on a range from a single zone. */ extern unsigned long alloc_contig_freed_pages(unsigned long start, unsigned long end, gfp_t flag); +extern int alloc_contig_range(unsigned long start, unsigned long end,

• 	      gfp_t flags);
  

extern void free_contig_pages(unsigned long pfn, unsigned nr_pages); /* diff --git a/mm/page_alloc.c b/mm/page_alloc.c index fbfb920..8010854 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -5773,6 +5773,154 @@ void free_contig_pages(unsigned long pfn, unsigned nr_pages) } } +static unsigned long pfn_to_maxpage(unsigned long pfn) +{

• return pfn & ~(MAX_ORDER_NR_PAGES - 1);

+}

pfn_to_maxpage is a very confusing name here. It would be preferable to create a MAX_ORDER_MASK that you apply directly.

Maybe something like SECTION_ALIGN_UP and SECTION_ALIGN_DOWN.

+static unsigned long pfn_to_maxpage_up(unsigned long pfn) +{

• return ALIGN(pfn, MAX_ORDER_NR_PAGES);

+}

+#define MIGRATION_RETRY 5 +static int __alloc_contig_migrate_range(unsigned long start, unsigned long end) +{

• int migration_failed = 0, ret;
• unsigned long pfn = start;
• /*

• * Some code "borrowed" from KAMEZAWA Hiroyuki's
  

• * __alloc_contig_pages().
  

• */
  

There is no need to put a comment like this here. Credit him in the changelog.

• /* drop all pages in pagevec and pcp list */
• lru_add_drain_all();
• drain_all_pages();

Very similar to migrate_prep(). drain_all_pages should not be required at this point.

• for (;;) {

• pfn = scan_lru_pages(pfn, end);
  

scan_lru_pages() is inefficient, this is going to be costly.

• if (!pfn || pfn >= end)
  

• 	break;
  

• ret = do_migrate_range(pfn, end);
  

• if (!ret) {
  

• 	migration_failed = 0;
  

• } else if (ret != -EBUSY
  

• 	|| ++migration_failed >= MIGRATION_RETRY) {
  

• 	return ret;
  

• } else {
  

• 	/* There are unstable pages.on pagevec. */
  

• 	lru_add_drain_all();
  

• 	/*
  

• 	 * there may be pages on pcplist before
  

• 	 * we mark the range as ISOLATED.
  

• 	 */
  

• 	drain_all_pages();
  

• }
  

• cond_resched();
  

• }
• if (!migration_failed) {

• /* drop all pages in pagevec and pcp list */
  

• lru_add_drain_all();
  

• drain_all_pages();
  

• }
• /* Make sure all pages are isolated */
• if (WARN_ON(test_pages_isolated(start, end)))

• return -EBUSY;
  

In some respects, this is very similar to mm/compaction#compact_zone(). They could have shared significant code if you reworked compact_zone to work on ranges of memory and express compact_zone to operate on zone->zone_start_pfn zone->zone_start_pfn+zone->spanned_pages . The compaction code is

• return 0;

+}

+/**

• • alloc_contig_range() -- tries to allocate given range of pages
• • @start: start PFN to allocate
• • @end: one-past-the-last PFN to allocate
• • @flags: flags passed to alloc_contig_freed_pages().
• • The PFN range does not have to be pageblock or MAX_ORDER_NR_PAGES
• • aligned, hovewer it's callers responsibility to guarantee that we
• • are the only thread that changes migrate type of pageblocks the
• • pages fall in.
• • Returns zero on success or negative error code. On success all
• • pages which PFN is in (start, end) are allocated for the caller and
• • need to be freed with free_contig_pages().
• */

+int alloc_contig_range(unsigned long start, unsigned long end,

•        gfp_t flags)
  

+{

• unsigned long outer_start, outer_end;
• int ret;
• /*

• * What we do here is we mark all pageblocks in range as
  

• * MIGRATE_ISOLATE.  Because of the way page allocator work, we
  

• * align the range to MAX_ORDER pages so that page allocator
  

• * won't try to merge buddies from different pageblocks and
  

• * change MIGRATE_ISOLATE to some other migration type.
  

• *
  

This part is new. compaction does not need to do this nor does it need to. I can see though why it would be important for CMA though and calling start_isolate_page_range() is reasonable.

There are alignment problems because it'll be on a pageblock boundary but that is not a significant problem.

• * Once the pageblocks are marked as MIGRATE_ISOLATE, we
  

• * migrate the pages from an unaligned range (ie. pages that
  

• * we are interested in).  This will put all the pages in
  

• * range back to page allocator as MIGRATE_ISOLATE.
  

• *
  

• * When this is done, we take the pages in range from page
  

• * allocator removing them from the buddy system.  This way
  

• * page allocator will never consider using them.
  

• *
  

• * This lets us mark the pageblocks back as
  

• * MIGRATE_CMA/MIGRATE_MOVABLE so that free pages in the
  

• * MAX_ORDER aligned range but not in the unaligned, original
  

• * range are put back to page allocator so that buddy can use
  

• * them.
  

• */
  

• ret = start_isolate_page_range(pfn_to_maxpage(start),

• 		       pfn_to_maxpage_up(end));
  

• if (ret)

• goto done;
  

• ret = __alloc_contig_migrate_range(start, end);
• if (ret)

• goto done;
  

• /*

• * Pages from [start, end) are within a MAX_ORDER_NR_PAGES
  

• * aligned blocks that are marked as MIGRATE_ISOLATE.  What's
  

• * more, all pages in [start, end) are free in page allocator.
  

• * What we are going to do is to allocate all pages from
  

• * [start, end) (that is remove them from page allocater).
  

• *
  

• * The only problem is that pages at the beginning and at the
  

• * end of interesting range may be not aligned with pages that
  

• * page allocator holds, ie. they can be part of higher order
  

• * pages.  Because of this, we reserve the bigger range and
  

• * once this is done free the pages we are not interested in.
  

• */
  

• ret = 0;
• while (!PageBuddy(pfn_to_page(start & (~0UL << ret))))

• if (WARN_ON(++ret >= MAX_ORDER))
  

• 	return -EINVAL;
  

• outer_start = start & (~0UL << ret);
• outer_end = alloc_contig_freed_pages(outer_start, end, flags);
• /* Free head and tail (if any) */
• if (start != outer_start)

• free_contig_pages(outer_start, start - outer_start);
  

• if (end != outer_end)

• free_contig_pages(end, outer_end - end);
  

• ret = 0;

+done:

• undo_isolate_page_range(pfn_to_maxpage(start), pfn_to_maxpage_up(end));
• return ret;

+}

#ifdef CONFIG_MEMORY_HOTREMOVE /*

• All pages in the range must be isolated before calling this.

On Thu, 06 Oct 2011 15:54:44 +0200 Marek Szyprowski m.szyprowski@samsung.com wrote:

From: Michal Nazarewicz m.nazarewicz@samsung.com

The MIGRATE_CMA migration type has two main characteristics: (i) only movable pages can be allocated from MIGRATE_CMA pageblocks and (ii) page allocator will never change migration type of MIGRATE_CMA pageblocks.

This guarantees that page in a MIGRATE_CMA page block can always be migrated somewhere else (unless there's no memory left in the system).

It is designed to be used with Contiguous Memory Allocator (CMA) for allocating big chunks (eg. 10MiB) of physically contiguous memory. Once driver requests contiguous memory, CMA will migrate pages from MIGRATE_CMA pageblocks.

To minimise number of migrations, MIGRATE_CMA migration type is the last type tried when page allocator falls back to other migration types then requested.

...

+#ifdef CONFIG_CMA_MIGRATE_TYPE +# define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA) +#else +# define is_migrate_cma(migratetype) false +#endif

Implement in C, please.

...

--- a/mm/compaction.c +++ b/mm/compaction.c @@ -115,6 +115,16 @@ static bool suitable_migration_target(struct page *page) if (migratetype == MIGRATE_ISOLATE || migratetype == MIGRATE_RESERVE) return false;

• /* Keep MIGRATE_CMA alone as well. */
• /*

• * XXX Revisit.  We currently cannot let compaction touch CMA
  

• * pages since compaction insists on changing their migration
  

• * type to MIGRATE_MOVABLE (see split_free_page() called from
  

• * isolate_freepages_block() above).
  

• */
  

Talk to us about this.

How serious is this shortcoming in practice? What would a fix look like? Is anyone working on an implementation, or planning to do so?

• if (is_migrate_cma(migratetype))

• return false;
  

• /* If the page is a large free page, then allow migration */ if (PageBuddy(page) && page_order(page) >= pageblock_order) return true;

...

+void __init init_cma_reserved_pageblock(struct page *page) +{

• struct page *p = page;
• unsigned i = pageblock_nr_pages;
• prefetchw(p);
• do {

• if (--i)
  

• 	prefetchw(p + 1);
  

• __ClearPageReserved(p);
  

• set_page_count(p, 0);
  

• } while (++p, i);
• set_page_refcounted(page);
• set_pageblock_migratetype(page, MIGRATE_CMA);
• __free_pages(page, pageblock_order);
• totalram_pages += pageblock_nr_pages;

+}

I wonder if the prefetches do any good. it doesn't seem very important in an __init function.

+#endif

...

On Thu, Oct 06, 2011 at 03:54:44PM +0200, Marek Szyprowski wrote:

...

The MIGRATE_CMA migration type has two main characteristics: (i) only movable pages can be allocated from MIGRATE_CMA pageblocks and (ii) page allocator will never change migration type of MIGRATE_CMA pageblocks.

This guarantees that page in a MIGRATE_CMA page block can always be migrated somewhere else (unless there's no memory left in the system).

On Tue, 18 Oct 2011 06:08:26 -0700, Mel Gorman mel@csn.ul.ie wrote:

Or the count is premanently elevated by a device driver for some reason or if the page is backed by a filesystem with a broken or unusable migrate_page() function. This is unavoidable, I'm just pointing out that you can stil have migration failures, particularly if GFP_MOVABLE has been improperly used.

CMA does not handle that well right now. I guess it's something to think about once the rest is nice and working.

...

It is designed to be used with Contiguous Memory Allocator (CMA) for allocating big chunks (eg. 10MiB) of physically contiguous memory. Once driver requests contiguous memory, CMA will migrate pages from MIGRATE_CMA pageblocks.

To minimise number of migrations, MIGRATE_CMA migration type is the last type tried when page allocator falls back to other migration types then requested.

It would be preferable if you could figure out how to reuse the MIGRATE_RESERVE type for just the bitmap.

I'm not entirely sure of what you mean here.

Like MIGRATE_CMA, it does not change type except when min_free_kbytes changes. However, it is something that could be done in the future to keep the size of the pageblock bitmap where it is now.

...

+enum {

• MIGRATE_UNMOVABLE,
• MIGRATE_RECLAIMABLE,
• MIGRATE_MOVABLE,
• MIGRATE_PCPTYPES, /* the number of types on the pcp lists */
• MIGRATE_RESERVE = MIGRATE_PCPTYPES,
• /*

• * MIGRATE_CMA migration type is designed to mimic the way
  

• * ZONE_MOVABLE works.  Only movable pages can be allocated
  

• * from MIGRATE_CMA pageblocks and page allocator never
  

• * implicitly change migration type of MIGRATE_CMA pageblock.
  

• *
  

• * The way to use it is to change migratetype of a range of
  

• * pageblocks to MIGRATE_CMA which can be done by
  

• * __free_pageblock_cma() function.  What is important though
  

• * is that a range of pageblocks must be aligned to
  

• * MAX_ORDER_NR_PAGES should biggest page be bigger then
  

• * a single pageblock.
  

• */
  

• MIGRATE_CMA,

This does mean that MIGRATE_CMA also does not have a per-cpu list. I don't know if that matters to you but all allocations using MIGRATE_CMA will take the zone lock.

This is sort of an artefact of my misunderstanding of pcp lists in the past. I'll have to re-evaluate the decision not to include CMA on pcp list.

Still, I think that CMA not being on pcp lists should not be a problem for us. At least we can try and get CMA running and then consider adding CMA to pcp lists.

I'm not sure this can be easily avoided because if there is a per-CPU list for MIGRATE_CMA, it might use a new cache line for it and incur a different set of performance problems.

...

• MIGRATE_ISOLATE, /* can't allocate from here */
• MIGRATE_TYPES

+};

...

diff --git a/mm/compaction.c b/mm/compaction.c index 97254e4..9cf6b2b 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -115,6 +115,16 @@ static bool suitable_migration_target(struct page *page) if (migratetype == MIGRATE_ISOLATE || migratetype == MIGRATE_RESERVE) return false;

• /* Keep MIGRATE_CMA alone as well. */
• /*

• * XXX Revisit.  We currently cannot let compaction touch CMA
  

• * pages since compaction insists on changing their migration
  

• * type to MIGRATE_MOVABLE (see split_free_page() called from
  

• * isolate_freepages_block() above).
  

• */
  

• if (is_migrate_cma(migratetype))

• return false;
  

This is another reason why CMA and compaction should be using almost identical code. It does mean that the compact_control may need to be renamed and get flags to control things like the setting of pageblock flags but it would be preferable to having two almost identical pieces of code.

I've addressed it in my other mail where I've changed the split_free_page() to not touch CMA and ISOLATE pageblocks. I think that this change should make the above comment no longer accurate and the check unnecessary.

...

/* If the page is a large free page, then allow migration */ if (PageBuddy(page) && page_order(page) >= pageblock_order) return true;

...

@@ -940,12 +963,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++) {
  

I don't see why this change is necessary.

It changes a sort of a magic number into a value that is calculated from the array. This makes it resistant to changes in the definition of the fallbacks array. I think this is a reasonable change.

...

	migratetype = fallbacks[start_migratetype][i];

	/* MIGRATE_RESERVE handled later if necessary */
	if (migratetype == MIGRATE_RESERVE)

• 		continue;
  

• 		break;
  
  area = &(zone->free_area[current_order]);
  if (list_empty(&area->free_list[migratetype]))
  

On Thu, Oct 06, 2011 at 03:54:46PM +0200, Marek Szyprowski wrote:

The Contiguous Memory Allocator is a set of helper functions for DMA mapping framework that improves allocations of contiguous memory chunks.

CMA grabs memory on system boot, marks it with CMA_MIGRATE_TYPE and gives back to the system. Kernel is allowed to allocate movable pages within CMA's managed memory so that it can be used for example for page cache when DMA mapping do not use it. On dma_alloc_from_contiguous() request such pages are migrated out of CMA area to free required contiguous block and fulfill the request. This allows to allocate large contiguous chunks of memory at any time assuming that there is enough free memory available in the system.

This code is heavily based on earlier works by Michal Nazarewicz.

Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com CC: Michal Nazarewicz mina86@mina86.com

---

arch/Kconfig | 3 + drivers/base/Kconfig | 79 +++++++ drivers/base/Makefile | 1 + drivers/base/dma-contiguous.c | 386 ++++++++++++++++++++++++++++++++++ include/asm-generic/dma-contiguous.h | 27 +++ include/linux/device.h | 4 + include/linux/dma-contiguous.h | 106 ++++++++++ 7 files changed, 606 insertions(+), 0 deletions(-) create mode 100644 drivers/base/dma-contiguous.c create mode 100644 include/asm-generic/dma-contiguous.h create mode 100644 include/linux/dma-contiguous.h

diff --git a/arch/Kconfig b/arch/Kconfig index 4b0669c..a3b39a2 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -124,6 +124,9 @@ config HAVE_ARCH_TRACEHOOK config HAVE_DMA_ATTRS bool +config HAVE_DMA_CONTIGUOUS

• bool

config USE_GENERIC_SMP_HELPERS bool diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 21cf46f..a5e6d75 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -174,4 +174,83 @@ config SYS_HYPERVISOR source "drivers/base/regmap/Kconfig" +config CMA

• bool "Contiguous Memory Allocator (EXPERIMENTAL)"
• depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK && EXPERIMENTAL
• select MIGRATION
• select CMA_MIGRATE_TYPE
• help

•  This enables the Contiguous Memory Allocator which allows drivers
  

•  to allocate big physically-contiguous blocks of memory for use with
  

•  hardware components that do not support I/O map nor scatter-gather.
  

•  For more information see <include/linux/dma-contiguous.h>.
  

•  If unsure, say "n".
  

+if CMA

+config CMA_DEBUG

• bool "CMA debug messages (DEVELOPEMENT)"

s/DEVELOPEMENT/DEVELOPMENT/

Should it be under DEBUG_KERNEL?

• help

•  Turns on debug messages in CMA.  This produces KERN_DEBUG
  

•  messages for every CMA call as well as various messages while
  

•  processing calls such as dma_alloc_from_contiguous().
  

•  This option does not affect warning and error messages.
  

+comment "Default contiguous memory area size:"

+config CMA_SIZE_ABSOLUTE

• int "Absolute size (in MiB)"
• depends on !CMA_SIZE_SEL_PERCENTAGE
• default 16
• help

•  Defines the size (in MiB) of the default memory area for Contiguous
  

•  Memory Allocator.
  

+config CMA_SIZE_PERCENTAGE

• int "Percentage of total memory"
• depends on !CMA_SIZE_SEL_ABSOLUTE
• default 10
• help

•  Defines the size of the default memory area for Contiguous Memory
  

•  Allocator as a percentage of the total memory in the system.
  

Why is this not a kernel parameter rather than a config option?

Better yet, why do drivers not register how much CMA memory they are interested in and then the drive core figure out if it can allocate that much or not?

+choice

• prompt "Selected region size"
• default CMA_SIZE_SEL_ABSOLUTE

+config CMA_SIZE_SEL_ABSOLUTE

• bool "Use absolute value only"

+config CMA_SIZE_SEL_PERCENTAGE

• bool "Use percentage value only"

+config CMA_SIZE_SEL_MIN

• bool "Use lower value (minimum)"

+config CMA_SIZE_SEL_MAX

• bool "Use higher value (maximum)"

+endchoice

+config CMA_ALIGNMENT

• int "Maximum PAGE_SIZE order of alignment for contiguous buffers"
• range 4 9
• default 8
• help

•  DMA mapping framework by default aligns all buffers to the smallest
  

•  PAGE_SIZE order which is greater than or equal to the requested buffer
  

•  size. This works well for buffers up to a few hundreds kilobytes, but
  

•  for larger buffers it just a memory waste. With this parameter you can
  

•  specify the maximum PAGE_SIZE order for contiguous buffers. Larger
  

•  buffers will be aligned only to this specified order. The order is
  

•  expressed as a power of two multiplied by the PAGE_SIZE.
  

•  For example, if your system defaults to 4KiB pages, the order value
  

•  of 8 means that the buffers will be aligned up to 1MiB only.
  

•  If unsure, leave the default value "8".
  

+endif

endmenu diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 99a375a..794546f 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -5,6 +5,7 @@ obj-y := core.o sys.o bus.o dd.o syscore.o \ cpu.o firmware.o init.o map.o devres.o \ attribute_container.o transport_class.o obj-$(CONFIG_DEVTMPFS) += devtmpfs.o +obj-$(CONFIG_CMA) += dma-contiguous.o obj-y += power/ obj-$(CONFIG_HAS_DMA) += dma-mapping.o obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c new file mode 100644 index 0000000..e54bb76 --- /dev/null +++ b/drivers/base/dma-contiguous.c @@ -0,0 +1,386 @@ +/*

• • Contiguous Memory Allocator for DMA mapping framework
• • Copyright (c) 2010-2011 by Samsung Electronics.
• • Written by:
• • Marek Szyprowski m.szyprowski@samsung.com
• • Michal Nazarewicz mina86@mina86.com
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of the
• • License or (at your optional) any later version of the license.
• */

+#define pr_fmt(fmt) "cma: " fmt

+#ifdef CONFIG_CMA_DEBUG +#ifndef DEBUG +# define DEBUG +#endif +#endif

+#include <asm/page.h> +#include <asm/dma-contiguous.h>

+#include <linux/memblock.h> +#include <linux/err.h> +#include <linux/mm.h> +#include <linux/mutex.h> +#include <linux/page-isolation.h> +#include <linux/slab.h> +#include <linux/swap.h> +#include <linux/mm_types.h> +#include <linux/dma-contiguous.h>

+#ifndef SZ_1M +#define SZ_1M (1 << 20) +#endif

+#ifdef phys_to_pfn +/* nothing to do */ +#elif defined __phys_to_pfn +# define phys_to_pfn __phys_to_pfn +#elif defined __va +# define phys_to_pfn(x) page_to_pfn(virt_to_page(__va(x))) +#else +# error phys_to_pfn implementation needed +#endif

Parts of this are assuming that there is a linear mapping of virtual to physical memory. I think this is always the case but it looks like something that should be defined in asm-generic with an option for architectures to override.

+struct cma {

• unsigned long base_pfn;
• unsigned long count;
• unsigned long *bitmap;

+};

+struct cma *dma_contiguous_default_area;

+#ifndef CONFIG_CMA_SIZE_ABSOLUTE +#define CONFIG_CMA_SIZE_ABSOLUTE 0 +#endif

+#ifndef CONFIG_CMA_SIZE_PERCENTAGE +#define CONFIG_CMA_SIZE_PERCENTAGE 0 +#endif

+static unsigned long size_abs = CONFIG_CMA_SIZE_ABSOLUTE * SZ_1M;

SIZE_ABSOLUTE is an odd name. It can't be a negative size. size_bytes maybe.

+static unsigned long size_percent = CONFIG_CMA_SIZE_PERCENTAGE; +static long size_cmdline = -1;

+static int __init early_cma(char *p) +{

• pr_debug("%s(%s)\n", __func__, p);
• size_cmdline = memparse(p, &p);
• return 0;

+} +early_param("cma", early_cma);

+static unsigned long __init __cma_early_get_total_pages(void) +{

• struct memblock_region *reg;
• unsigned long total_pages = 0;
• /*

• * We cannot use memblock_phys_mem_size() here, because
  

• * memblock_analyze() has not been called yet.
  

• */
  

• for_each_memblock(memory, reg)

• total_pages += memblock_region_memory_end_pfn(reg) -
  

• 	       memblock_region_memory_base_pfn(reg);
  

• return total_pages;

+}

Is this being called too early yet? What prevents you seeing up the CMA regions after the page allocator is brought up for example? I understand that there is a need for the memory to be coherent so maybe that is the obstacle.

+/**

• • dma_contiguous_reserve() - reserve area for contiguous memory handling
• • This funtion reserves memory from early allocator. It should be
• • called by arch specific code once the early allocator (memblock or bootmem)
• • has been activated and all other subsystems have already allocated/reserved
• • memory.
• */

+void __init dma_contiguous_reserve(phys_addr_t limit) +{

• unsigned long selected_size = 0;
• unsigned long total_pages;
• pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
• total_pages = __cma_early_get_total_pages();
• size_percent *= (total_pages << PAGE_SHIFT) / 100;
• pr_debug("%s: total available: %ld MiB, size absolute: %ld MiB, size percentage: %ld MiB\n",

•  __func__, (total_pages << PAGE_SHIFT) / SZ_1M,
  

• size_abs / SZ_1M, size_percent / SZ_1M);
  

+#ifdef CONFIG_CMA_SIZE_SEL_ABSOLUTE

• selected_size = size_abs;

+#elif defined(CONFIG_CMA_SIZE_SEL_PERCENTAGE)

• selected_size = size_percent;

+#elif defined(CONFIG_CMA_SIZE_SEL_MIN)

• selected_size = min(size_abs, size_percent);

+#elif defined(CONFIG_CMA_SIZE_SEL_MAX)

• selected_size = max(size_abs, size_percent);

+#endif

It seems very strange to do this at Kconfig time instead of via kernel parameters.

• if (size_cmdline != -1)

• selected_size = size_cmdline;
  

• if (!selected_size)

• return;
  

• pr_debug("%s: reserving %ld MiB for global area\n", __func__,

•  selected_size / SZ_1M);
  

• dma_declare_contiguous(NULL, selected_size, 0, limit);

+};

+static DEFINE_MUTEX(cma_mutex);

+static void __cma_activate_area(unsigned long base_pfn, unsigned long count) +{

• unsigned long pfn = base_pfn;
• unsigned i = count >> pageblock_order;
• struct zone *zone;
• VM_BUG_ON(!pfn_valid(pfn));

Again, VM_BUG_ON is an extreme reaction. WARN_ON_ONCE, return an error code and fail gracefully.

• zone = page_zone(pfn_to_page(pfn));
• do {

• unsigned j;
  

• base_pfn = pfn;
  

• for (j = pageblock_nr_pages; j; --j, pfn++) {
  

This is correct but does not look like any other PFN walker. There are plenty of examples of where we walk PFN ranges. There is no requirement to use the same pattern but it does make reviewing easier.

• 	VM_BUG_ON(!pfn_valid(pfn));
  

• 	VM_BUG_ON(page_zone(pfn_to_page(pfn)) != zone);
  

• }
  

In the field, this is a no-op as I would assume CONFIG_DEBUG_VM is not set. This should be checked unconditionally and fail gracefully if necessary.

• init_cma_reserved_pageblock(pfn_to_page(base_pfn));
  

• } while (--i);

+}

+static struct cma *__cma_create_area(unsigned long base_pfn,

• 		     unsigned long count)
  

+{

• int bitmap_size = BITS_TO_LONGS(count) * sizeof(long);
• struct cma *cma;
• pr_debug("%s(base %08lx, count %lx)\n", __func__, base_pfn, count);
• cma = kmalloc(sizeof *cma, GFP_KERNEL);
• if (!cma)

• return ERR_PTR(-ENOMEM);
  

• cma->base_pfn = base_pfn;
• cma->count = count;
• cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
• if (!cma->bitmap)

• goto no_mem;
  

• __cma_activate_area(base_pfn, count);
• pr_debug("%s: returned %p\n", __func__, (void *)cma);
• return cma;

+no_mem:

• kfree(cma);
• return ERR_PTR(-ENOMEM);

+}

+static struct cma_reserved {

• phys_addr_t start;
• unsigned long size;
• struct device *dev;

+} cma_reserved[MAX_CMA_AREAS] __initdata; +static unsigned cma_reserved_count __initdata;

+static int __init __cma_init_reserved_areas(void) +{

• struct cma_reserved *r = cma_reserved;
• unsigned i = cma_reserved_count;
• pr_debug("%s()\n", __func__);
• for (; i; --i, ++r) {

• struct cma *cma;
  

• cma = __cma_create_area(phys_to_pfn(r->start),
  

• 			r->size >> PAGE_SHIFT);
  

• if (!IS_ERR(cma)) {
  

• 	if (r->dev)
  

• 		set_dev_cma_area(r->dev, cma);
  

• 	else
  

• 		dma_contiguous_default_area = cma;
  

• }
  

• }
• return 0;

+} +core_initcall(__cma_init_reserved_areas);

+/**

• • dma_declare_contiguous() - reserve area for contiguous memory handling

• • 	      for particular device
    

• • @dev: Pointer to device structure.
• • @size: Size of the reserved memory.
• • @start: Start address of the reserved memory (optional, 0 for any).
• • @limit: End address of the reserved memory (optional, 0 for any).
• • This funtion reserves memory for specified device. It should be
• • called by board specific code when early allocator (memblock or bootmem)
• • is still activate.
• */

+int __init dma_declare_contiguous(struct device *dev, unsigned long size,

• 		  phys_addr_t base, phys_addr_t limit)
  

+{

• struct cma_reserved *r = &cma_reserved[cma_reserved_count];
• unsigned long alignment;
• pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,

•  (unsigned long)size, (unsigned long)base,
  

•  (unsigned long)limit);
  

• /* Sanity checks */
• if (cma_reserved_count == ARRAY_SIZE(cma_reserved))

• return -ENOSPC;
  

• if (!size)

• return -EINVAL;
  

• /* Sanitise input arguments */
• alignment = PAGE_SIZE << max(MAX_ORDER, pageblock_order);
• base = ALIGN(base, alignment);
• size = ALIGN(size, alignment);
• limit = ALIGN(limit, alignment);
• /* Reserve memory */
• if (base) {

• if (memblock_is_region_reserved(base, size) ||
  

•     memblock_reserve(base, size) < 0) {
  

• 	base = -EBUSY;
  

• 	goto err;
  

• }
  

• } else {

• /*
  

•  * Use __memblock_alloc_base() since
  

•  * memblock_alloc_base() panic()s.
  

•  */
  

• phys_addr_t addr = __memblock_alloc_base(size, alignment, limit);
  

• if (!addr) {
  

• 	base = -ENOMEM;
  

• 	goto err;
  

• } else if (addr + size > ~(unsigned long)0) {
  

• 	memblock_free(addr, size);
  

• 	base = -EOVERFLOW;
  

• 	goto err;
  

• } else {
  

• 	base = addr;
  

• }
  

• }
• /*

• * Each reserved area must be initialised later, when more kernel
  

• * subsystems (like slab allocator) are available.
  

• */
  

• r->start = base;
• r->size = size;
• r->dev = dev;
• cma_reserved_count++;
• printk(KERN_INFO "CMA: reserved %ld MiB at %08lx\n", size / SZ_1M,

•       (unsigned long)base);
  

• /*

• * Architecture specific contiguous memory fixup.
  

• */
  

• dma_contiguous_early_fixup(base, size);
• return 0;

+err:

• printk(KERN_ERR "CMA: failed to reserve %ld MiB\n", size / SZ_1M);
• return base;

+}

+/**

• • dma_alloc_from_contiguous() - allocate pages from contiguous area
• • @dev: Pointer to device for which the allocation is performed.
• • @count: Requested number of pages.
• • @align: Requested alignment of pages (in PAGE_SIZE order).
• • This funtion allocates memory buffer for specified device. It uses
• • device specific contiguous memory area if available or the default
• • global one. Requires architecture specific get_dev_cma_area() helper
• • function.
• */

+struct page *dma_alloc_from_contiguous(struct device *dev, int count,

• 		       unsigned int align)
  

+{

• struct cma *cma = get_dev_cma_area(dev);
• unsigned long pfn, pageno;
• int ret;
• if (!cma)

• return NULL;
  

• if (align > CONFIG_CMA_ALIGNMENT)

• align = CONFIG_CMA_ALIGNMENT;
  

• pr_debug("%s(cma %p, count %d, align %d)\n", __func__, (void *)cma,

•  count, align);
  

• if (!count)

• return NULL;
  

• mutex_lock(&cma_mutex);
• pageno = bitmap_find_next_zero_area(cma->bitmap, cma->count, 0, count,

• 			    (1 << align) - 1);
  

• if (pageno >= cma->count) {

• ret = -ENOMEM;
  

• goto error;
  

• }
• bitmap_set(cma->bitmap, pageno, count);
• pfn = cma->base_pfn + pageno;
• ret = alloc_contig_range(pfn, pfn + count, 0, MIGRATE_CMA);
• if (ret)

• goto free;
  

If alloc_contig_range returns failure, the bitmap is still set. It will never be freed so now the area cannot be used for CMA allocations any more.

• mutex_unlock(&cma_mutex);
• pr_debug("%s(): returned %p\n", __func__, pfn_to_page(pfn));
• return pfn_to_page(pfn);

+free:

• bitmap_clear(cma->bitmap, pageno, count);

+error:

• mutex_unlock(&cma_mutex);
• return NULL;

+}

+/**

• • dma_release_from_contiguous() - release allocated pages
• • @dev: Pointer to device for which the pages were allocated.
• • @pages: Allocated pages.
• • @count: Number of allocated pages.
• • This funtion releases memory allocated by dma_alloc_from_contiguous().
• • It return 0 when provided pages doen't belongs to contiguous area and
• • 1 on success.
• */

+int dma_release_from_contiguous(struct device *dev, struct page *pages,

• 		int count)
  

+{

• struct cma *cma = get_dev_cma_area(dev);
• unsigned long pfn;
• if (!cma || !pages)

• return 0;
  

• pr_debug("%s(page %p)\n", __func__, (void *)pages);
• pfn = page_to_pfn(pages);
• if (pfn < cma->base_pfn || pfn >= cma->base_pfn + cma->count)

• return 0;
  

• mutex_lock(&cma_mutex);
• bitmap_clear(cma->bitmap, pfn - cma->base_pfn, count);
• free_contig_pages(pfn, count);
• mutex_unlock(&cma_mutex);

It feels like the mutex could be a lot lighter here. If the bitmap is protected by a spinlock, it would only need to be held while the bitmap was being cleared. free the contig pages outside the spinlock and clear the bitmap afterwards.

It's not particularly important as the scalability of CMA is not something to be concerned with at this point.

• return 1;

+} diff --git a/include/asm-generic/dma-contiguous.h b/include/asm-generic/dma-contiguous.h new file mode 100644 index 0000000..8c76649 --- /dev/null +++ b/include/asm-generic/dma-contiguous.h @@ -0,0 +1,27 @@ +#ifndef ASM_DMA_CONTIGUOUS_H +#define ASM_DMA_CONTIGUOUS_H

+#ifdef __KERNEL__

+#include <linux/device.h> +#include <linux/dma-contiguous.h>

+#ifdef CONFIG_CMA

+static inline struct cma *get_dev_cma_area(struct device *dev) +{

• if (dev && dev->cma_area)

• return dev->cma_area;
  

• return dma_contiguous_default_area;

+}

+static inline void set_dev_cma_area(struct device *dev, struct cma *cma) +{

• if (dev)

• dev->cma_area = cma;
  

• dma_contiguous_default_area = cma;

+}

+#endif +#endif +#endif diff --git a/include/linux/device.h b/include/linux/device.h index 8bab5c4..cc1e7f0 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -592,6 +592,10 @@ struct device { struct dma_coherent_mem *dma_mem; /* internal for coherent mem override */ +#ifdef CONFIG_CMA

• struct cma *cma_area; /* contiguous memory area for dma

• 			   allocations */
  

+#endif /* arch specific additions */ struct dev_archdata archdata; diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h new file mode 100644 index 0000000..7ca81c9 --- /dev/null +++ b/include/linux/dma-contiguous.h @@ -0,0 +1,106 @@ +#ifndef __LINUX_CMA_H +#define __LINUX_CMA_H

+/*

• • Contiguous Memory Allocator for DMA mapping framework
• • Copyright (c) 2010-2011 by Samsung Electronics.
• • Written by:
• • Marek Szyprowski m.szyprowski@samsung.com
• • Michal Nazarewicz mina86@mina86.com
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of the
• • License or (at your optional) any later version of the license.
• */

+/*

• • Contiguous Memory Allocator
• • The Contiguous Memory Allocator (CMA) makes it possible to
• • allocate big contiguous chunks of memory after the system has
• • booted.
• • Why is it needed?
• • Various devices on embedded systems have no scatter-getter and/or
• • IO map support and require contiguous blocks of memory to
• • operate. They include devices such as cameras, hardware video
• • coders, etc.
• • Such devices often require big memory buffers (a full HD frame
• • is, for instance, more then 2 mega pixels large, i.e. more than 6
• • MB of memory), which makes mechanisms such as kmalloc() or
• • alloc_page() ineffective.
• • At the same time, a solution where a big memory region is
• • reserved for a device is suboptimal since often more memory is
• • reserved then strictly required and, moreover, the memory is
• • inaccessible to page system even if device drivers don't use it.
• • CMA tries to solve this issue by operating on memory regions
• • where only movable pages can be allocated from. This way, kernel
• • can use the memory for pagecache and when device driver requests
• • it, allocated pages can be migrated.
• • Driver usage
• • CMA should not be used by the device drivers directly. It is
• • only a helper framework for dma-mapping subsystem.
• • For more information, see kernel-docs in drivers/base/dma-contiguous.c
• */

+#ifdef __KERNEL__

+struct cma; +struct page; +struct device;

+#ifdef CONFIG_CMA

+#define MAX_CMA_AREAS (8)

+extern struct cma *dma_contiguous_default_area;

+void dma_contiguous_reserve(phys_addr_t addr_limit); +int dma_declare_contiguous(struct device *dev, unsigned long size,

• 	   phys_addr_t base, phys_addr_t limit);
  

+struct page *dma_alloc_from_contiguous(struct device *dev, int count,

• 		       unsigned int order);
  

+int dma_release_from_contiguous(struct device *dev, struct page *pages,

• 		int count);
  

+#else

+#define MAX_CMA_AREAS (0)

+static inline void dma_contiguous_reserve(phys_addr_t limit) { }

+static inline +int dma_declare_contiguous(struct device *dev, unsigned long size,

• 	   phys_addr_t base, phys_addr_t limit)
  

+{

• return -ENOSYS;

+}

+static inline +struct page *dma_alloc_from_contiguous(struct device *dev, int count,

• 		       unsigned int order)
  

+{

• return NULL;

+}

+static inline +int dma_release_from_contiguous(struct device *dev, struct page *pages,

• 		int count)
  

+{

• return 0;

+}

+#endif

+#endif

+#endif

1.7.1.569.g6f426

Hello,

On Tuesday, October 18, 2011 3:43 PM Mel Gorman wrote:

On Thu, Oct 06, 2011 at 03:54:46PM +0200, Marek Szyprowski wrote:

...

The Contiguous Memory Allocator is a set of helper functions for DMA mapping framework that improves allocations of contiguous memory chunks.

CMA grabs memory on system boot, marks it with CMA_MIGRATE_TYPE and gives back to the system. Kernel is allowed to allocate movable pages within CMA's managed memory so that it can be used for example for page cache when DMA mapping do not use it. On dma_alloc_from_contiguous() request such pages are migrated out of CMA area to free required contiguous block and fulfill the request. This allows to allocate large contiguous chunks of memory at any time assuming that there is enough free memory available in the system.

This code is heavily based on earlier works by Michal Nazarewicz.

Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com CC: Michal Nazarewicz mina86@mina86.com

---

arch/Kconfig | 3 + drivers/base/Kconfig | 79 +++++++ drivers/base/Makefile | 1 + drivers/base/dma-contiguous.c | 386 ++++++++++++++++++++++++++++++++++ include/asm-generic/dma-contiguous.h | 27 +++ include/linux/device.h | 4 + include/linux/dma-contiguous.h | 106 ++++++++++ 7 files changed, 606 insertions(+), 0 deletions(-) create mode 100644 drivers/base/dma-contiguous.c create mode 100644 include/asm-generic/dma-contiguous.h create mode 100644 include/linux/dma-contiguous.h

diff --git a/arch/Kconfig b/arch/Kconfig index 4b0669c..a3b39a2 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -124,6 +124,9 @@ config HAVE_ARCH_TRACEHOOK config HAVE_DMA_ATTRS bool

+config HAVE_DMA_CONTIGUOUS

• bool

config USE_GENERIC_SMP_HELPERS bool

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 21cf46f..a5e6d75 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -174,4 +174,83 @@ config SYS_HYPERVISOR

source "drivers/base/regmap/Kconfig"

+config CMA

• bool "Contiguous Memory Allocator (EXPERIMENTAL)"
• depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK && EXPERIMENTAL
• select MIGRATION
• select CMA_MIGRATE_TYPE
• help

•  This enables the Contiguous Memory Allocator which allows drivers
  

•  to allocate big physically-contiguous blocks of memory for use with
  

•  hardware components that do not support I/O map nor scatter-gather.
  

•  For more information see <include/linux/dma-contiguous.h>.
  

•  If unsure, say "n".
  

+if CMA

+config CMA_DEBUG

• bool "CMA debug messages (DEVELOPEMENT)"

s/DEVELOPEMENT/DEVELOPMENT/

Should it be under DEBUG_KERNEL?

...

• help

•  Turns on debug messages in CMA.  This produces KERN_DEBUG
  

•  messages for every CMA call as well as various messages while
  

•  processing calls such as dma_alloc_from_contiguous().
  

•  This option does not affect warning and error messages.
  

+comment "Default contiguous memory area size:"

+config CMA_SIZE_ABSOLUTE

• int "Absolute size (in MiB)"
• depends on !CMA_SIZE_SEL_PERCENTAGE
• default 16
• help

•  Defines the size (in MiB) of the default memory area for Contiguous
  

•  Memory Allocator.
  

+config CMA_SIZE_PERCENTAGE

• int "Percentage of total memory"
• depends on !CMA_SIZE_SEL_ABSOLUTE
• default 10
• help

•  Defines the size of the default memory area for Contiguous Memory
  

•  Allocator as a percentage of the total memory in the system.
  

Why is this not a kernel parameter rather than a config option?

There is also a kernel parameter for CMA area size which overrides the value from .config.

Better yet, why do drivers not register how much CMA memory they are interested in and then the drive core figure out if it can allocate that much or not?

CMA area is reserved very early during boot process, even before the buddy allocator gets initialized. That time no device driver has been probed yet. Such early reservation is required to be sure that enough contiguous memory can be gathered and to perform some MMU related fixups that are required on ARM to avoid page aliasing for dma_alloc_coherent() memory.

...

+choice

• prompt "Selected region size"
• default CMA_SIZE_SEL_ABSOLUTE

+config CMA_SIZE_SEL_ABSOLUTE

• bool "Use absolute value only"

+config CMA_SIZE_SEL_PERCENTAGE

• bool "Use percentage value only"

+config CMA_SIZE_SEL_MIN

• bool "Use lower value (minimum)"

+config CMA_SIZE_SEL_MAX

• bool "Use higher value (maximum)"

+endchoice

+config CMA_ALIGNMENT

• int "Maximum PAGE_SIZE order of alignment for contiguous buffers"
• range 4 9
• default 8
• help

•  DMA mapping framework by default aligns all buffers to the smallest
  

•  PAGE_SIZE order which is greater than or equal to the requested buffer
  

•  size. This works well for buffers up to a few hundreds kilobytes, but
  

•  for larger buffers it just a memory waste. With this parameter you can
  

•  specify the maximum PAGE_SIZE order for contiguous buffers. Larger
  

•  buffers will be aligned only to this specified order. The order is
  

•  expressed as a power of two multiplied by the PAGE_SIZE.
  

•  For example, if your system defaults to 4KiB pages, the order value
  

•  of 8 means that the buffers will be aligned up to 1MiB only.
  

•  If unsure, leave the default value "8".
  

+endif

endmenu diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 99a375a..794546f 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -5,6 +5,7 @@ obj-y := core.o sys.o bus.o dd.o syscore.o \ cpu.o firmware.o init.o map.o devres.o \ attribute_container.o transport_class.o obj-$(CONFIG_DEVTMPFS) += devtmpfs.o +obj-$(CONFIG_CMA) += dma-contiguous.o obj-y += power/ obj-$(CONFIG_HAS_DMA) += dma-mapping.o obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c new file mode 100644 index 0000000..e54bb76 --- /dev/null +++ b/drivers/base/dma-contiguous.c @@ -0,0 +1,386 @@ +/*

• • Contiguous Memory Allocator for DMA mapping framework
• • Copyright (c) 2010-2011 by Samsung Electronics.
• • Written by:
• • Marek Szyprowski m.szyprowski@samsung.com
• • Michal Nazarewicz mina86@mina86.com
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of the
• • License or (at your optional) any later version of the license.
• */

+#define pr_fmt(fmt) "cma: " fmt

+#ifdef CONFIG_CMA_DEBUG +#ifndef DEBUG +# define DEBUG +#endif +#endif

+#include <asm/page.h> +#include <asm/dma-contiguous.h>

+#include <linux/memblock.h> +#include <linux/err.h> +#include <linux/mm.h> +#include <linux/mutex.h> +#include <linux/page-isolation.h> +#include <linux/slab.h> +#include <linux/swap.h> +#include <linux/mm_types.h> +#include <linux/dma-contiguous.h>

+#ifndef SZ_1M +#define SZ_1M (1 << 20) +#endif

+#ifdef phys_to_pfn +/* nothing to do */ +#elif defined __phys_to_pfn +# define phys_to_pfn __phys_to_pfn +#elif defined __va +# define phys_to_pfn(x) page_to_pfn(virt_to_page(__va(x))) +#else +# error phys_to_pfn implementation needed +#endif

Parts of this are assuming that there is a linear mapping of virtual to physical memory. I think this is always the case but it looks like something that should be defined in asm-generic with an option for architectures to override.

...

+struct cma {

• unsigned long base_pfn;
• unsigned long count;
• unsigned long *bitmap;

+};

+struct cma *dma_contiguous_default_area;

+#ifndef CONFIG_CMA_SIZE_ABSOLUTE +#define CONFIG_CMA_SIZE_ABSOLUTE 0 +#endif

+#ifndef CONFIG_CMA_SIZE_PERCENTAGE +#define CONFIG_CMA_SIZE_PERCENTAGE 0 +#endif

+static unsigned long size_abs = CONFIG_CMA_SIZE_ABSOLUTE * SZ_1M;

SIZE_ABSOLUTE is an odd name. It can't be a negative size. size_bytes maybe.

...

+static unsigned long size_percent = CONFIG_CMA_SIZE_PERCENTAGE; +static long size_cmdline = -1;

+static int __init early_cma(char *p) +{

• pr_debug("%s(%s)\n", __func__, p);
• size_cmdline = memparse(p, &p);
• return 0;

+} +early_param("cma", early_cma);

+static unsigned long __init __cma_early_get_total_pages(void) +{

• struct memblock_region *reg;
• unsigned long total_pages = 0;
• /*

• * We cannot use memblock_phys_mem_size() here, because
  

• * memblock_analyze() has not been called yet.
  

• */
  

• for_each_memblock(memory, reg)

• total_pages += memblock_region_memory_end_pfn(reg) -
  

• 	       memblock_region_memory_base_pfn(reg);
  

• return total_pages;

+}

Is this being called too early yet? What prevents you seeing up the CMA regions after the page allocator is brought up for example? I understand that there is a need for the memory to be coherent so maybe that is the obstacle.

Right now we assume that CMA areas can be created only during early boot with memblock allocator. The code that converts memory on-fly into CMA region can be added later (if required).

...

+/**

• • dma_contiguous_reserve() - reserve area for contiguous memory handling
• • This funtion reserves memory from early allocator. It should be
• • called by arch specific code once the early allocator (memblock or bootmem)
• • has been activated and all other subsystems have already allocated/reserved
• • memory.
• */

+void __init dma_contiguous_reserve(phys_addr_t limit) +{

• unsigned long selected_size = 0;
• unsigned long total_pages;
• pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
• total_pages = __cma_early_get_total_pages();
• size_percent *= (total_pages << PAGE_SHIFT) / 100;
• pr_debug("%s: total available: %ld MiB, size absolute: %ld MiB, size percentage: %ld

MiB\n",

...

•  __func__, (total_pages << PAGE_SHIFT) / SZ_1M,
  

• size_abs / SZ_1M, size_percent / SZ_1M);
  

+#ifdef CONFIG_CMA_SIZE_SEL_ABSOLUTE

• selected_size = size_abs;

+#elif defined(CONFIG_CMA_SIZE_SEL_PERCENTAGE)

• selected_size = size_percent;

+#elif defined(CONFIG_CMA_SIZE_SEL_MIN)

• selected_size = min(size_abs, size_percent);

+#elif defined(CONFIG_CMA_SIZE_SEL_MAX)

• selected_size = max(size_abs, size_percent);

+#endif

It seems very strange to do this at Kconfig time instead of via kernel parameters.

...

• if (size_cmdline != -1)

• selected_size = size_cmdline;
  

• if (!selected_size)

• return;
  

• pr_debug("%s: reserving %ld MiB for global area\n", __func__,

•  selected_size / SZ_1M);
  

• dma_declare_contiguous(NULL, selected_size, 0, limit);

+};

+static DEFINE_MUTEX(cma_mutex);

+static void __cma_activate_area(unsigned long base_pfn, unsigned long count) +{

• unsigned long pfn = base_pfn;
• unsigned i = count >> pageblock_order;
• struct zone *zone;
• VM_BUG_ON(!pfn_valid(pfn));

Again, VM_BUG_ON is an extreme reaction. WARN_ON_ONCE, return an error code and fail gracefully.

...

• zone = page_zone(pfn_to_page(pfn));
• do {

• unsigned j;
  

• base_pfn = pfn;
  

• for (j = pageblock_nr_pages; j; --j, pfn++) {
  

This is correct but does not look like any other PFN walker. There are plenty of examples of where we walk PFN ranges. There is no requirement to use the same pattern but it does make reviewing easier.

...

• 	VM_BUG_ON(!pfn_valid(pfn));
  

• 	VM_BUG_ON(page_zone(pfn_to_page(pfn)) != zone);
  

• }
  

In the field, this is a no-op as I would assume CONFIG_DEBUG_VM is not set. This should be checked unconditionally and fail gracefully if necessary.

...

• init_cma_reserved_pageblock(pfn_to_page(base_pfn));
  

• } while (--i);

+}

+static struct cma *__cma_create_area(unsigned long base_pfn,

• 		     unsigned long count)
  

+{

• int bitmap_size = BITS_TO_LONGS(count) * sizeof(long);
• struct cma *cma;
• pr_debug("%s(base %08lx, count %lx)\n", __func__, base_pfn, count);
• cma = kmalloc(sizeof *cma, GFP_KERNEL);
• if (!cma)

• return ERR_PTR(-ENOMEM);
  

• cma->base_pfn = base_pfn;
• cma->count = count;
• cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
• if (!cma->bitmap)

• goto no_mem;
  

• __cma_activate_area(base_pfn, count);
• pr_debug("%s: returned %p\n", __func__, (void *)cma);
• return cma;

+no_mem:

• kfree(cma);
• return ERR_PTR(-ENOMEM);

+}

+static struct cma_reserved {

• phys_addr_t start;
• unsigned long size;
• struct device *dev;

+} cma_reserved[MAX_CMA_AREAS] __initdata; +static unsigned cma_reserved_count __initdata;

+static int __init __cma_init_reserved_areas(void) +{

• struct cma_reserved *r = cma_reserved;
• unsigned i = cma_reserved_count;
• pr_debug("%s()\n", __func__);
• for (; i; --i, ++r) {

• struct cma *cma;
  

• cma = __cma_create_area(phys_to_pfn(r->start),
  

• 			r->size >> PAGE_SHIFT);
  

• if (!IS_ERR(cma)) {
  

• 	if (r->dev)
  

• 		set_dev_cma_area(r->dev, cma);
  

• 	else
  

• 		dma_contiguous_default_area = cma;
  

• }
  

• }
• return 0;

+} +core_initcall(__cma_init_reserved_areas);

+/**

• • dma_declare_contiguous() - reserve area for contiguous memory handling

• • 	      for particular device
    

• • @dev: Pointer to device structure.
• • @size: Size of the reserved memory.
• • @start: Start address of the reserved memory (optional, 0 for any).
• • @limit: End address of the reserved memory (optional, 0 for any).
• • This funtion reserves memory for specified device. It should be
• • called by board specific code when early allocator (memblock or bootmem)
• • is still activate.
• */

+int __init dma_declare_contiguous(struct device *dev, unsigned long size,

• 		  phys_addr_t base, phys_addr_t limit)
  

+{

• struct cma_reserved *r = &cma_reserved[cma_reserved_count];
• unsigned long alignment;
• pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,

•  (unsigned long)size, (unsigned long)base,
  

•  (unsigned long)limit);
  

• /* Sanity checks */
• if (cma_reserved_count == ARRAY_SIZE(cma_reserved))

• return -ENOSPC;
  

• if (!size)

• return -EINVAL;
  

• /* Sanitise input arguments */
• alignment = PAGE_SIZE << max(MAX_ORDER, pageblock_order);
• base = ALIGN(base, alignment);
• size = ALIGN(size, alignment);
• limit = ALIGN(limit, alignment);
• /* Reserve memory */
• if (base) {

• if (memblock_is_region_reserved(base, size) ||
  

•     memblock_reserve(base, size) < 0) {
  

• 	base = -EBUSY;
  

• 	goto err;
  

• }
  

• } else {

• /*
  

•  * Use __memblock_alloc_base() since
  

•  * memblock_alloc_base() panic()s.
  

•  */
  

• phys_addr_t addr = __memblock_alloc_base(size, alignment, limit);
  

• if (!addr) {
  

• 	base = -ENOMEM;
  

• 	goto err;
  

• } else if (addr + size > ~(unsigned long)0) {
  

• 	memblock_free(addr, size);
  

• 	base = -EOVERFLOW;
  

• 	goto err;
  

• } else {
  

• 	base = addr;
  

• }
  

• }
• /*

• * Each reserved area must be initialised later, when more kernel
  

• * subsystems (like slab allocator) are available.
  

• */
  

• r->start = base;
• r->size = size;
• r->dev = dev;
• cma_reserved_count++;
• printk(KERN_INFO "CMA: reserved %ld MiB at %08lx\n", size / SZ_1M,

•       (unsigned long)base);
  

• /*

• * Architecture specific contiguous memory fixup.
  

• */
  

• dma_contiguous_early_fixup(base, size);
• return 0;

+err:

• printk(KERN_ERR "CMA: failed to reserve %ld MiB\n", size / SZ_1M);
• return base;

+}

+/**

• • dma_alloc_from_contiguous() - allocate pages from contiguous area
• • @dev: Pointer to device for which the allocation is performed.
• • @count: Requested number of pages.
• • @align: Requested alignment of pages (in PAGE_SIZE order).
• • This funtion allocates memory buffer for specified device. It uses
• • device specific contiguous memory area if available or the default
• • global one. Requires architecture specific get_dev_cma_area() helper
• • function.
• */

+struct page *dma_alloc_from_contiguous(struct device *dev, int count,

• 		       unsigned int align)
  

+{

• struct cma *cma = get_dev_cma_area(dev);
• unsigned long pfn, pageno;
• int ret;
• if (!cma)

• return NULL;
  

• if (align > CONFIG_CMA_ALIGNMENT)

• align = CONFIG_CMA_ALIGNMENT;
  

• pr_debug("%s(cma %p, count %d, align %d)\n", __func__, (void *)cma,

•  count, align);
  

• if (!count)

• return NULL;
  

• mutex_lock(&cma_mutex);
• pageno = bitmap_find_next_zero_area(cma->bitmap, cma->count, 0, count,

• 			    (1 << align) - 1);
  

• if (pageno >= cma->count) {

• ret = -ENOMEM;
  

• goto error;
  

• }
• bitmap_set(cma->bitmap, pageno, count);
• pfn = cma->base_pfn + pageno;
• ret = alloc_contig_range(pfn, pfn + count, 0, MIGRATE_CMA);
• if (ret)

• goto free;
  

If alloc_contig_range returns failure, the bitmap is still set. It will never be freed so now the area cannot be used for CMA allocations any more.

There is bitmap_clear() call just after free: label, so the bitmap is updated correctly.

...

• mutex_unlock(&cma_mutex);
• pr_debug("%s(): returned %p\n", __func__, pfn_to_page(pfn));
• return pfn_to_page(pfn);

+free:

• bitmap_clear(cma->bitmap, pageno, count);

+error:

• mutex_unlock(&cma_mutex);
• return NULL;

+}

+/**

• • dma_release_from_contiguous() - release allocated pages
• • @dev: Pointer to device for which the pages were allocated.
• • @pages: Allocated pages.
• • @count: Number of allocated pages.
• • This funtion releases memory allocated by dma_alloc_from_contiguous().
• • It return 0 when provided pages doen't belongs to contiguous area and
• • 1 on success.
• */

+int dma_release_from_contiguous(struct device *dev, struct page *pages,

• 		int count)
  

+{

• struct cma *cma = get_dev_cma_area(dev);
• unsigned long pfn;
• if (!cma || !pages)

• return 0;
  

• pr_debug("%s(page %p)\n", __func__, (void *)pages);
• pfn = page_to_pfn(pages);
• if (pfn < cma->base_pfn || pfn >= cma->base_pfn + cma->count)

• return 0;
  

• mutex_lock(&cma_mutex);
• bitmap_clear(cma->bitmap, pfn - cma->base_pfn, count);
• free_contig_pages(pfn, count);
• mutex_unlock(&cma_mutex);

It feels like the mutex could be a lot lighter here. If the bitmap is protected by a spinlock, it would only need to be held while the bitmap was being cleared. free the contig pages outside the spinlock and clear the bitmap afterwards.

It's not particularly important as the scalability of CMA is not something to be concerned with at this point.

This mutex also serializes cma allocations, so there is only one alloc_contig_range() call processed at once. This is done to serialize isolation of page blocks that is performed inside alloc_contig_range().

Best regards

Hi Marek,

As informed to you in private over IRC, below piece of code broke during booting EXYNOS4:SMDKV310 with ZONE_DMA enabled.

On 10/06/2011 07:24 PM, Marek Szyprowski wrote: ...

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index fbdd12e..9c27fbd 100644 --- a/arch/arm/mm/init.c +++ b/arch/arm/mm/init.c @@ -21,6 +21,7 @@ #include<linux/gfp.h> #include<linux/memblock.h> #include<linux/sort.h> +#include<linux/dma-contiguous.h>

#include<asm/mach-types.h> #include<asm/prom.h> @@ -371,6 +372,13 @@ void __init arm_memblock_init(struct meminfo *mi, struct machine_desc *mdesc) if (mdesc->reserve) mdesc->reserve();

• /* reserve memory for DMA contigouos allocations */

+#ifdef CONFIG_ZONE_DMA

• dma_contiguous_reserve(PHYS_OFFSET + mdesc->dma_zone_size - 1);

+#else

• dma_contiguous_reserve(0);

+#endif

• memblock_analyze(); memblock_dump_all(); }

Regards, Subash

Hello Marek,

On 10/12/2011 01:08 PM, Marek Szyprowski wrote:

Signed-off-by: Marek Szyprowskim.szyprowski@samsung.com

mm/page_alloc.c | 15 ++++++++++++--- 1 files changed, 12 insertions(+), 3 deletions(-)

Hello Maxime,

Please check if this patch fixes your lockup issue. It is a bit cruel, but it looks that in case of real low-memory situation page allocation is very complex task which usually ends in waiting for the io/fs and free pages that really don't arrive at all.

Thanks for the reactivity. We just tested it, we no more faced the lockup. Instead, the OOM Killer is triggered and contiguous allocation succeed. I'm not familiar enough with page_alloc.c to detect any side effects this patch could bring.

Best regards

Marek Szyprowski Samsung Poland R&D Center

diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 055aa4c..45473e9 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -5872,6 +5872,7 @@ int alloc_contig_range(unsigned long start, unsigned long end, gfp_t flags, unsigned migratetype) { unsigned long outer_start, outer_end;

• unsigned int count = end - start; int ret;

  /*

@@ -5900,7 +5901,10 @@ int alloc_contig_range(unsigned long start, unsigned long end, ret = __start_isolate_page_range(pfn_to_maxpage(start), pfn_to_maxpage_up(end), migratetype); if (ret)

• goto done;
  

• return ret;
  

• min_free_kbytes += count * PAGE_SIZE / 1024;

• setup_per_zone_wmarks();

  ret = __alloc_contig_migrate_range(start, end); if (ret)

@@ -5922,8 +5926,10 @@ int alloc_contig_range(unsigned long start, unsigned long end,

ret = 0; while (!PageBuddy(pfn_to_page(start& (~0UL<< ret))))

• if (WARN_ON(++ret>= MAX_ORDER))
  

• 	return -EINVAL;
  

• if (WARN_ON(++ret>= MAX_ORDER)) {
  

• 	ret = -EINVAL;
  

• 	goto done;
  

• }
  
  

  outer_start = start& (~0UL<< ret); outer_end = alloc_contig_freed_pages(outer_start, end, flags);

@@ -5936,6 +5942,9 @@ int alloc_contig_range(unsigned long start, unsigned long end,

ret = 0; done:

• min_free_kbytes -= count * PAGE_SIZE / 1024;
• setup_per_zone_wmarks();
• __undo_isolate_page_range(pfn_to_maxpage(start), pfn_to_maxpage_up(end), migratetype); return ret;

Best regards, Maxime