From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:10:16
Welcome everyone!
Yes, that's true. This is yet another release of the Contiguous Memory
Allocator patches. This version mainly includes code cleanups requested
by Mel Gorman and a few minor bug fixes.
ARM integration code has not been changed since v16. It provides
implementation of the ideas that has been discussed during Linaro Sprint
meeting in Cambourne, August 2011. Here are the details:
This version provides a solution for complete integration of CMA to
DMA mapping subsystem on ARM architecture. The issue caused by double
dma pages mapping and possible aliasing in coherent memory mapping has
been finally resolved, both for GFP_ATOMIC case (allocations comes from
coherent memory pool) and non-GFP_ATOMIC case (allocations comes from
CMA managed areas).
For coherent, nommu, ARMv4 and ARMv5 systems the current DMA-mapping
implementation has been kept.
For ARMv6+ systems, CMA has been enabled and a special pool of coherent
memory for atomic allocations has been created. The size of this pool
defaults to DEFAULT_CONSISTEN_DMA_SIZE/8, but can be changed with
coherent_pool kernel parameter (if really required).
All atomic allocations are served from this pool. I've did a little
simplification here, because there is no separate pool for writecombine
memory - such requests are also served from coherent pool. I don't
think that such simplification is a problem here - I found no driver
that use dma_alloc_writecombine with GFP_ATOMIC flags.
All non-atomic allocation are served from CMA area. Kernel mappings are
updated to reflect required memory attributes changes. This is possible
because during early boot, all CMA area are remapped with 4KiB pages in
kernel low-memory.
This version have been tested on Samsung S5PC110 based Goni machine and
Exynos4 UniversalC210 board with various V4L2 multimedia drivers.
Coherent atomic allocations has been tested by manually enabling the dma
bounce for the s3c-sdhci device.
All patches are prepared for Linux Kernel v3.3-rc1.
A few words for these who see CMA for the first time:
The Contiguous Memory Allocator (CMA) makes it possible for device
drivers to allocate big contiguous chunks of memory after the system
has booted.
The main difference from the similar frameworks is the fact that CMA
allows to transparently reuse memory region reserved for the big
chunk allocation as a system memory, so no memory is wasted when no
big chunk is allocated. Once the alloc request is issued, the
framework will migrate system pages to create a required big chunk of
physically contiguous memory.
For more information you can refer to nice LWN articles:
http://lwn.net/Articles/447405/ and http://lwn.net/Articles/450286/
as well as links to previous versions of the CMA framework.
The CMA framework has been initially developed by Michal Nazarewicz
at Samsung Poland R&D Center. Since version 9, I've taken over the
development, because Michal has left the company. Since version v17
Michal is working again on CMA patches and the current version is
the result of our joint open-source effort.
TODO (optional):
- implement support for contiguous memory areas placed in HIGHMEM zone
- resolve issue with movable pages with pending io operations
Best regards
Marek Szyprowski
Samsung Poland R&D Center
Links to previous versions of the patchset:
v18: <http://www.spinics.net/lists/linux-mm/msg28125.html>
v17: <http://www.spinics.net/lists/arm-kernel/msg148499.html>
v16: <http://www.spinics.net/lists/linux-mm/msg25066.html>
v15: <http://www.spinics.net/lists/linux-mm/msg23365.html>
v14: <http://www.spinics.net/lists/linux-media/msg36536.html>
v13: (internal, intentionally not released)
v12: <http://www.spinics.net/lists/linux-media/msg35674.html>
v11: <http://www.spinics.net/lists/linux-mm/msg21868.html>
v10: <http://www.spinics.net/lists/linux-mm/msg20761.html>
v9: <http://article.gmane.org/gmane.linux.kernel.mm/60787>
v8: <http://article.gmane.org/gmane.linux.kernel.mm/56855>
v7: <http://article.gmane.org/gmane.linux.kernel.mm/55626>
v6: <http://article.gmane.org/gmane.linux.kernel.mm/55626>
v5: (intentionally left out as CMA v5 was identical to CMA v4)
v4: <http://article.gmane.org/gmane.linux.kernel.mm/52010>
v3: <http://article.gmane.org/gmane.linux.kernel.mm/51573>
v2: <http://article.gmane.org/gmane.linux.kernel.mm/50986>
v1: <http://article.gmane.org/gmane.linux.kernel.mm/50669>
Changelog:
v19:
1. Addressed another set of comments and suggestions from Mel Gorman, mainly
related to breaking patches into smaller, single-feature related chunks
and rewriting already existing functions in memory compaction code.
2. Reworked completely page reclaim code, removed it from split_free_page()
and introduce direct call from alloc_contig_range().
3. Merged a fix from Mans Rullgard for correct cma area limit alignment.
4. Replaced broken "mm: page_alloc: set_migratetype_isolate: drain PCP prior
to isolating" patch with "mm: page_alloc: update migrate type of pages on
pcp when isolating" which is another attempt to solve this issue without
touching free_pcppages_bulk().
5. Rebased onto v3.3-rc1
v18:
1. Addressed comments and suggestions from Mel Gorman related to changes
in memory compaction code, most important points:
- removed "mm: page_alloc: handle MIGRATE_ISOLATE in free_pcppages_bulk()"
and moved all the logic to set_migratetype_isolate - see
"mm: page_alloc: set_migratetype_isolate: drain PCP prior to isolating"
patch
- code in "mm: compaction: introduce isolate_{free,migrate}pages_range()"
patch have been simplified and improved
- removed "mm: mmzone: introduce zone_pfn_same_memmap()" patch
2. Fixed crash on initialization if HIGHMEM is available on ARM platforms
3. Fixed problems with allocation of contiguous memory if all free pages
are occupied by page cache and reclaim is required.
4. Added a workaround for temporary migration failures (now CMA tries
to allocate different memory block in such case), what heavily increased
reliability of the CMA.
5. Minor cleanup here and there.
6. Rebased onto v3.2-rc7 kernel tree.
v17:
1. Replaced whole CMA core memory migration code to the new one kindly
provided by Michal Nazarewicz. The new code is based on memory
compaction framework not the memory hotplug, like it was before. This
change has been suggested by Mel Godman.
2. Addressed most of the comments from Andrew Morton and Mel Gorman in
the rest of the CMA code.
3. Fixed broken initialization on ARM systems with DMA zone enabled.
4. Rebased onto v3.2-rc2 kernel.
v16:
1. merged a fixup from Michal Nazarewicz to address comments from Dave
Hansen about checking if pfns belong to the same memory zone
2. merged a fix from Michal Nazarewicz for incorrect handling of pages
which belong to page block that is in MIGRATE_ISOLATE state, in very
rare cases the migrate type of page block might have been changed
from MIGRATE_CMA to MIGRATE_MOVABLE because of this bug
3. moved some common code to include/asm-generic
4. added support for x86 DMA-mapping framework for pci-dma hardware,
CMA can be now even more widely tested on KVM/QEMU and a lot of common
x86 boxes
5. rebased onto next-20111005 kernel tree, which includes changes in ARM
DMA-mapping subsystem (CONSISTENT_DMA_SIZE removal)
6. removed patch for CMA s5p-fimc device private regions (served only as
example) and provided the one that matches real life case - s5p-mfc
device
v15:
1. fixed calculation of the total memory after activating CMA area (was
broken from v12)
2. more code cleanup in drivers/base/dma-contiguous.c
3. added address limit for default CMA area
4. rewrote ARM DMA integration:
- removed "ARM: DMA: steal memory for DMA coherent mappings" patch
- kept current DMA mapping implementation for coherent, nommu and
ARMv4/ARMv5 systems
- enabled CMA for all ARMv6+ systems
- added separate, small pool for coherent atomic allocations, defaults
to CONSISTENT_DMA_SIZE/8, but can be changed with kernel parameter
coherent_pool=[size]
v14:
1. Merged with "ARM: DMA: steal memory for DMA coherent mappings"
patch, added support for GFP_ATOMIC allocations.
2. Added checks for NULL device pointer
v13: (internal, intentionally not released)
v12:
1. Fixed 2 nasty bugs in dma-contiguous allocator:
- alignment argument was not passed correctly
- range for dma_release_from_contiguous was not checked correctly
2. Added support for architecture specfic dma_contiguous_early_fixup()
function
3. CMA and DMA-mapping integration for ARM architechture has been
rewritten to take care of the memory aliasing issue that might
happen for newer ARM CPUs (mapping of the same pages with different
cache attributes is forbidden). TODO: add support for GFP_ATOMIC
allocations basing on the "ARM: DMA: steal memory for DMA coherent
mappings" patch and implement support for contiguous memory areas
that are placed in HIGHMEM zone
v11:
1. Removed genalloc usage and replaced it with direct calls to
bitmap_* functions, dropped patches that are not needed
anymore (genalloc extensions)
2. Moved all contiguous area management code from mm/cma.c
to drivers/base/dma-contiguous.c
3. Renamed cm_alloc/free to dma_alloc/release_from_contiguous
4. Introduced global, system wide (default) contiguous area
configured with kernel config and kernel cmdline parameters
5. Simplified initialization to just one function:
dma_declare_contiguous()
6. Added example of device private memory contiguous area
v10:
1. Rebased onto 3.0-rc2 and resolved all conflicts
2. Simplified CMA to be just a pure memory allocator, for use
with platfrom/bus specific subsystems, like dma-mapping.
Removed all device specific functions are calls.
3. Integrated with ARM DMA-mapping subsystem.
4. Code cleanup here and there.
5. Removed private context support.
v9: 1. Rebased onto 2.6.39-rc1 and resolved all conflicts
2. Fixed a bunch of nasty bugs that happened when the allocation
failed (mainly kernel oops due to NULL ptr dereference).
3. Introduced testing code: cma-regions compatibility layer and
videobuf2-cma memory allocator module.
v8: 1. The alloc_contig_range() function has now been separated from
CMA and put in page_allocator.c. This function tries to
migrate all LRU pages in specified range and then allocate the
range using alloc_contig_freed_pages().
2. Support for MIGRATE_CMA has been separated from the CMA code.
I have not tested if CMA works with ZONE_MOVABLE but I see no
reasons why it shouldn't.
3. I have added a @private argument when creating CMA contexts so
that one can reserve memory and not share it with the rest of
the system. This way, CMA acts only as allocation algorithm.
v7: 1. A lot of functionality that handled driver->allocator_context
mapping has been removed from the patchset. This is not to say
that this code is not needed, it's just not worth posting
everything in one patchset.
Currently, CMA is "just" an allocator. It uses it's own
migratetype (MIGRATE_CMA) for defining ranges of pageblokcs
which behave just like ZONE_MOVABLE but dispite the latter can
be put in arbitrary places.
2. The migration code that was introduced in the previous version
actually started working.
v6: 1. Most importantly, v6 introduces support for memory migration.
The implementation is not yet complete though.
Migration support means that when CMA is not using memory
reserved for it, page allocator can allocate pages from it.
When CMA wants to use the memory, the pages have to be moved
and/or evicted as to make room for CMA.
To make it possible it must be guaranteed that only movable and
reclaimable pages are allocated in CMA controlled regions.
This is done by introducing a MIGRATE_CMA migrate type that
guarantees exactly that.
Some of the migration code is "borrowed" from Kamezawa
Hiroyuki's alloc_contig_pages() implementation. The main
difference is that thanks to MIGRATE_CMA migrate type CMA
assumes that memory controlled by CMA are is always movable or
reclaimable so that it makes allocation decisions regardless of
the whether some pages are actually allocated and migrates them
if needed.
The most interesting patches from the patchset that implement
the functionality are:
09/13: mm: alloc_contig_free_pages() added
10/13: mm: MIGRATE_CMA migration type added
11/13: mm: MIGRATE_CMA isolation functions added
12/13: mm: cma: Migration support added [wip]
Currently, kernel panics in some situations which I am trying
to investigate.
2. cma_pin() and cma_unpin() functions has been added (after
a conversation with Johan Mossberg). The idea is that whenever
hardware does not use the memory (no transaction is on) the
chunk can be moved around. This would allow defragmentation to
be implemented if desired. No defragmentation algorithm is
provided at this time.
3. Sysfs support has been replaced with debugfs. I always felt
unsure about the sysfs interface and when Greg KH pointed it
out I finally got to rewrite it to debugfs.
v5: (intentionally left out as CMA v5 was identical to CMA v4)
v4: 1. The "asterisk" flag has been removed in favour of requiring
that platform will provide a "*=<regions>" rule in the map
attribute.
2. The terminology has been changed slightly renaming "kind" to
"type" of memory. In the previous revisions, the documentation
indicated that device drivers define memory kinds and now,
v3: 1. The command line parameters have been removed (and moved to
a separate patch, the fourth one). As a consequence, the
cma_set_defaults() function has been changed -- it no longer
accepts a string with list of regions but an array of regions.
2. The "asterisk" attribute has been removed. Now, each region
has an "asterisk" flag which lets one specify whether this
region should by considered "asterisk" region.
3. SysFS support has been moved to a separate patch (the third one
in the series) and now also includes list of regions.
v2: 1. The "cma_map" command line have been removed. In exchange,
a SysFS entry has been created under kernel/mm/contiguous.
The intended way of specifying the attributes is
a cma_set_defaults() function called by platform initialisation
code. "regions" attribute (the string specified by "cma"
command line parameter) can be overwritten with command line
parameter; the other attributes can be changed during run-time
using the SysFS entries.
2. The behaviour of the "map" attribute has been modified
slightly. Currently, if no rule matches given device it is
assigned regions specified by the "asterisk" attribute. It is
by default built from the region names given in "regions"
attribute.
3. Devices can register private regions as well as regions that
can be shared but are not reserved using standard CMA
mechanisms. A private region has no name and can be accessed
only by devices that have the pointer to it.
4. The way allocators are registered has changed. Currently,
a cma_allocator_register() function is used for that purpose.
Moreover, allocators are attached to regions the first time
memory is registered from the region or when allocator is
registered which means that allocators can be dynamic modules
that are loaded after the kernel booted (of course, it won't be
possible to allocate a chunk of memory from a region if
allocator is not loaded).
5. Index of new functions:
+static inline dma_addr_t __must_check
+cma_alloc_from(const char *regions, size_t size,
+ dma_addr_t alignment)
+static inline int
+cma_info_about(struct cma_info *info, const const char *regions)
+int __must_check cma_region_register(struct cma_region *reg);
+dma_addr_t __must_check
+cma_alloc_from_region(struct cma_region *reg,
+ size_t size, dma_addr_t alignment);
+static inline dma_addr_t __must_check
+cma_alloc_from(const char *regions,
+ size_t size, dma_addr_t alignment);
+int cma_allocator_register(struct cma_allocator *alloc);
Patches in this patchset:
Marek Szyprowski (6):
mm: extract reclaim code from __alloc_pages_direct_reclaim()
mm: trigger page reclaim in alloc_contig_range() to stabilize
watermarks
drivers: add Contiguous Memory Allocator
X86: integrate CMA with DMA-mapping subsystem
ARM: integrate CMA with DMA-mapping subsystem
ARM: Samsung: use CMA for 2 memory banks for s5p-mfc device
Michal Nazarewicz (9):
mm: page_alloc: remove trailing whitespace
mm: page_alloc: update migrate type of pages on pcp when isolating
mm: compaction: introduce isolate_migratepages_range().
mm: compaction: introduce isolate_freepages_range()
mm: compaction: export some of the functions
mm: page_alloc: introduce alloc_contig_range()
mm: page_alloc: change fallbacks array handling
mm: mmzone: MIGRATE_CMA migration type added
mm: page_isolation: MIGRATE_CMA isolation functions added
Documentation/kernel-parameters.txt | 9 +
arch/Kconfig | 3 +
arch/arm/Kconfig | 2 +
arch/arm/include/asm/dma-contiguous.h | 16 ++
arch/arm/include/asm/mach/map.h | 1 +
arch/arm/kernel/setup.c | 9 +-
arch/arm/mm/dma-mapping.c | 368 ++++++++++++++++++++++++------
arch/arm/mm/init.c | 22 ++-
arch/arm/mm/mm.h | 3 +
arch/arm/mm/mmu.c | 31 ++-
arch/arm/plat-s5p/dev-mfc.c | 51 +----
arch/x86/Kconfig | 1 +
arch/x86/include/asm/dma-contiguous.h | 13 +
arch/x86/include/asm/dma-mapping.h | 4 +
arch/x86/kernel/pci-dma.c | 18 ++-
arch/x86/kernel/pci-nommu.c | 8 +-
arch/x86/kernel/setup.c | 2 +
drivers/base/Kconfig | 89 +++++++
drivers/base/Makefile | 1 +
drivers/base/dma-contiguous.c | 404 ++++++++++++++++++++++++++++++++
include/asm-generic/dma-contiguous.h | 27 +++
include/linux/device.h | 4 +
include/linux/dma-contiguous.h | 110 +++++++++
include/linux/mmzone.h | 43 +++-
include/linux/page-isolation.h | 35 ++-
mm/Kconfig | 2 +-
mm/Makefile | 3 +-
mm/compaction.c | 414 +++++++++++++++++++++------------
mm/internal.h | 33 +++
mm/memory-failure.c | 2 +-
mm/memory_hotplug.c | 6 +-
mm/page_alloc.c | 355 +++++++++++++++++++++++++---
mm/page_isolation.c | 39 +++-
mm/vmstat.c | 3 +
34 files changed, 1770 insertions(+), 361 deletions(-)
create mode 100644 arch/arm/include/asm/dma-contiguous.h
create mode 100644 arch/x86/include/asm/dma-contiguous.h
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
--
1.7.1.569.g6f426
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:01:04
From: Michal Nazarewicz <redacted>
This commit introduces isolate_migratepages_range() function which
extracts functionality from isolate_migratepages() so that it can be
used on arbitrary PFN ranges.
isolate_migratepages() function is implemented as a simple wrapper
around isolate_migratepages_range().
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
mm/compaction.c | 77 +++++++++++++++++++++++++++++++++++++++---------------
1 files changed, 55 insertions(+), 22 deletions(-)
@@ -250,31 +250,34 @@ typedef enum {ISOLATE_SUCCESS,/* Pages isolated, migrate */}isolate_migrate_t;-/*-*Isolateallpagesthatcanbemigratedfromtheblockpointedtoby-*themigratescannerwithincompact_control.+/**+*isolate_migratepages_range()-isolateallmigrate-ablepagesinrange.+*@zone:Zonepagesarein.+*@cc:Compactioncontrolstructure.+*@low_pfn:ThefirstPFNoftherange.+*@end_pfn:Theone-past-the-lastPFNoftherange.+*+*Isolateallpagesthatcanbemigratedfromtherangespecifiedby+*[low_pfn,end_pfn).Returnszeroifthereisafatalsignal+*pending),otherwisePFNofthefirstpagethatwasnotscanned+*(whichmaybebothless,equaltoormorethenend_pfn).+*+*Assumesthatcc->migratepagesisemptyandcc->nr_migratepagesis+*zero.+*+*Apartfromcc->migratepagesandcc->nr_migratetypesthisfunction+*doesnotmodifyanycc'sfields,inparticularitdoesnotmodify+*(orreadforthatmatter)cc->migrate_pfn.*/-staticisolate_migrate_tisolate_migratepages(structzone*zone,-structcompact_control*cc)+staticunsignedlong+isolate_migratepages_range(structzone*zone,structcompact_control*cc,+unsignedlonglow_pfn,unsignedlongend_pfn){-unsignedlonglow_pfn,end_pfn;unsignedlonglast_pageblock_nr=0,pageblock_nr;unsignedlongnr_scanned=0,nr_isolated=0;structlist_head*migratelist=&cc->migratepages;isolate_mode_tmode=ISOLATE_ACTIVE|ISOLATE_INACTIVE;-/* Do not scan outside zone boundaries */-low_pfn=max(cc->migrate_pfn,zone->zone_start_pfn);--/* Only scan within a pageblock boundary */-end_pfn=ALIGN(low_pfn+pageblock_nr_pages,pageblock_nr_pages);--/* Do not cross the free scanner or scan within a memory hole */-if(end_pfn>cc->free_pfn||!pfn_valid(low_pfn)){-cc->migrate_pfn=end_pfn;-returnISOLATE_NONE;-}-/**EnsurethattherearenottoomanypagesisolatedfromtheLRU*listbyeitherparallelreclaimersorcompaction.Ifthereare,
@@ -283,12 +286,12 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,while(unlikely(too_many_isolated(zone))){/* async migration should just abort */if(!cc->sync)-returnISOLATE_ABORT;+return0;congestion_wait(BLK_RW_ASYNC,HZ/10);if(fatal_signal_pending(current))-returnISOLATE_ABORT;+return0;}/* Time to isolate some pages for migration */
@@ -313,7 +316,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,}elseif(!locked)spin_lock_irq(&zone->lru_lock);-if(!pfn_valid_within(low_pfn))+if(!pfn_valid(low_pfn))continue;nr_scanned++;
@@ -374,10 +377,40 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,acct_isolated(zone,cc);spin_unlock_irq(&zone->lru_lock);-cc->migrate_pfn=low_pfn;trace_mm_compaction_isolate_migratepages(nr_scanned,nr_isolated);+returnlow_pfn;+}++/*+*Isolateallpagesthatcanbemigratedfromtheblockpointedtoby+*themigratescannerwithincompact_control.+*/+staticisolate_migrate_tisolate_migratepages(structzone*zone,+structcompact_control*cc)+{+unsignedlonglow_pfn,end_pfn;++/* Do not scan outside zone boundaries */+low_pfn=max(cc->migrate_pfn,zone->zone_start_pfn);++/* Only scan within a pageblock boundary */+end_pfn=ALIGN(low_pfn+pageblock_nr_pages,pageblock_nr_pages);++/* Do not cross the free scanner or scan within a memory hole */+if(end_pfn>cc->free_pfn||!pfn_valid(low_pfn)){+cc->migrate_pfn=end_pfn;+returnISOLATE_NONE;+}++/* Perform the isolation */+low_pfn=isolate_migratepages_range(zone,cc,low_pfn,end_pfn);+if(!low_pfn)+returnISOLATE_ABORT;++cc->migrate_pfn=low_pfn;+returnISOLATE_SUCCESS;}
On Thu, Jan 26, 2012 at 10:00:45AM +0100, Marek Szyprowski wrote:
From: Michal Nazarewicz <redacted>
This commit introduces isolate_migratepages_range() function which
extracts functionality from isolate_migratepages() so that it can be
used on arbitrary PFN ranges.
isolate_migratepages() function is implemented as a simple wrapper
around isolate_migratepages_range().
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Super, this is much easier to read. I have just one nit below but once
that is fixed;
Acked-by: Mel Gorman <redacted>
quoted hunk
@@ -313,7 +316,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone, } else if (!locked) spin_lock_irq(&zone->lru_lock);- if (!pfn_valid_within(low_pfn))+ if (!pfn_valid(low_pfn)) continue; nr_scanned++;
This chunk looks unrelated to the rest of the patch.
I think what you are doing is patching around a bug that CMA exposed
which is very similar to the bug report at
http://www.spinics.net/lists/linux-mm/msg29260.html . Is this true?
If so, I posted a fix that only calls pfn_valid() when necessary. Can
you check if that works for you and if so, drop this hunk please? If
the patch does not work for you, then this hunk still needs to be
in a separate patch and handled separately as it would also be a fix
for -stable.
--
Mel Gorman
SUSE Labs
From: Michal Nazarewicz <hidden> Date: 2012-01-30 12:42:57
On Thu, Jan 26, 2012 at 10:00:45AM +0100, Marek Szyprowski wrote:
quoted
From: Michal Nazarewicz <redacted>
@@ -313,7 +316,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone, } else if (!locked) spin_lock_irq(&zone->lru_lock);- if (!pfn_valid_within(low_pfn))+ if (!pfn_valid(low_pfn)) continue; nr_scanned++;
On Mon, 30 Jan 2012 12:24:28 +0100, Mel Gorman [off-list ref] wrote:
This chunk looks unrelated to the rest of the patch.
I think what you are doing is patching around a bug that CMA exposed
which is very similar to the bug report at
http://www.spinics.net/lists/linux-mm/msg29260.html . Is this true?
If so, I posted a fix that only calls pfn_valid() when necessary. Can
you check if that works for you and if so, drop this hunk please? If
the patch does not work for you, then this hunk still needs to be
in a separate patch and handled separately as it would also be a fix
for -stable.
I'll actually never encountered this bug myself and CMA is unlikely to
expose it, since it always operates on continuous memory regions with
no holes.
I've made this change because looking at the code it seemed like this
may cause problems in some cases. The crash that you linked to looks
like the kind of problem I was thinking about.
I'll drop this hunk and let you resolve this independently of CMA.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Micha? ?mina86? Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
On Mon, Jan 30, 2012 at 01:42:50PM +0100, Michal Nazarewicz wrote:
quoted
On Thu, Jan 26, 2012 at 10:00:45AM +0100, Marek Szyprowski wrote:
quoted
From: Michal Nazarewicz <redacted>
@@ -313,7 +316,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,
} else if (!locked)
spin_lock_irq(&zone->lru_lock);
- if (!pfn_valid_within(low_pfn))
+ if (!pfn_valid(low_pfn))
continue;
nr_scanned++;
On Mon, 30 Jan 2012 12:24:28 +0100, Mel Gorman [off-list ref] wrote:
quoted
This chunk looks unrelated to the rest of the patch.
I think what you are doing is patching around a bug that CMA exposed
which is very similar to the bug report at
http://www.spinics.net/lists/linux-mm/msg29260.html . Is this true?
If so, I posted a fix that only calls pfn_valid() when necessary. Can
you check if that works for you and if so, drop this hunk please? If
the patch does not work for you, then this hunk still needs to be
in a separate patch and handled separately as it would also be a fix
for -stable.
I'll actually never encountered this bug myself and CMA is unlikely to
expose it, since it always operates on continuous memory regions with
no holes.
I've made this change because looking at the code it seemed like this
may cause problems in some cases. The crash that you linked to looks
like the kind of problem I was thinking about.
I'll drop this hunk and let you resolve this independently of CMA.
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:01:06
From: Michal Nazarewicz <redacted>
This commit introduces isolate_freepages_range() function which
generalises isolate_freepages_block() so that it can be used on
arbitrary PFN ranges.
isolate_freepages_block() is left with only minor changes.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
mm/compaction.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 100 insertions(+), 18 deletions(-)
@@ -54,24 +54,20 @@ static unsigned long release_freepages(struct list_head *freelist)returncount;}-/* Isolate free pages onto a private freelist. Must hold zone->lock */-staticunsignedlongisolate_freepages_block(structzone*zone,-unsignedlongblockpfn,-structlist_head*freelist)+/*+*Isolatefreepagesontoaprivatefreelist.Callermustholdzone->lock.+*If@strictistrue,willabortreturning0onanyinvalidPFNsornon-free+*pagesinsideofthepageblock(eventhoughitmaystillendupisolating+*somepages).+*/+staticunsignedlongisolate_freepages_block(unsignedlongblockpfn,+unsignedlongend_pfn,+structlist_head*freelist,+boolstrict){-unsignedlongzone_end_pfn,end_pfn;intnr_scanned=0,total_isolated=0;structpage*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 */
@@ -79,15 +75,23 @@ static unsigned long isolate_freepages_block(struct zone *zone,intisolated,i;structpage*page=cursor;-if(!pfn_valid_within(blockpfn))+if(!pfn_valid_within(blockpfn)){+if(strict)+return0;continue;+}nr_scanned++;-if(!PageBuddy(page))+if(!PageBuddy(page)){+if(strict)+return0;continue;+}/* Found a free page, break it into order-0 pages */isolated=split_free_page(page);+if(!isolated&&strict)+return0;total_isolated+=isolated;for(i=0;i<isolated;i++){list_add(&page->lru,freelist);
@@ -105,6 +109,80 @@ static unsigned long isolate_freepages_block(struct zone *zone,returntotal_isolated;}+/**+*isolate_freepages_range()-isolatefreepages.+*@start_pfn:ThefirstPFNtostartisolating.+*@end_pfn:Theone-past-lastPFN.+*+*Non-freepages,invalidPFNs,orzoneboundarieswithinthe+*[start_pfn,end_pfn)rangeareconsiderederrors,causefunctionto+*undoitsactionsandreturnzero.+*+*Otherwise,functionreturnsone-past-the-lastPFNofisolatedpage+*(whichmaybegreaterthenend_pfnifendfellinamiddleof+*afreepage).+*/+staticunsignedlong+isolate_freepages_range(unsignedlongstart_pfn,unsignedlongend_pfn)+{+unsignedlongisolated,pfn,block_end_pfn,flags;+structzone*zone=NULL;+LIST_HEAD(freelist);+structpage*page;++for(pfn=start_pfn;pfn<end_pfn;pfn+=isolated){+if(!pfn_valid(pfn))+break;++if(!zone)+zone=page_zone(pfn_to_page(pfn));+elseif(zone!=page_zone(pfn_to_page(pfn)))+break;++/*+*Onsubsequentiterationsround_down()isactuallynot+*needed,butwekeepitthatwenottocomplicatethecode.+*/+block_end_pfn=round_down(pfn,pageblock_nr_pages)++pageblock_nr_pages;+block_end_pfn=min(block_end_pfn,end_pfn);++spin_lock_irqsave(&zone->lock,flags);+isolated=isolate_freepages_block(pfn,block_end_pfn,+&freelist,true);+spin_unlock_irqrestore(&zone->lock,flags);++/*+*Instrictmode,isolate_freepages_block()returns0if+*thereareanyholesintheblock(ie.invalidPFNsor+*non-freepages).+*/+if(!isolated)+break;++/*+*Ifwemanagedtoisolatepages,itisalways(1<<n)*+*pageblock_nr_pagesforsomenon-negativen.(Maxorder+*pagemayspantwopageblocks).+*/+}++/* split_free_page does not map the pages */+list_for_each_entry(page,&freelist,lru){+arch_alloc_page(page,0);+kernel_map_pages(page,1,1);+}++if(pfn<end_pfn){+/* Loop terminated early, cleanup. */+release_freepages(&freelist);+return0;+}++/* We don't use freelists for anything. */+returnpfn;+}+/* Returns true if the page is within a block suitable for migration to */staticboolsuitable_migration_target(structpage*page){
@@ -135,7 +213,7 @@ static void isolate_freepages(struct zone *zone,structcompact_control*cc){structpage*page;-unsignedlonghigh_pfn,low_pfn,pfn;+unsignedlonghigh_pfn,low_pfn,pfn,zone_end_pfn,end_pfn;unsignedlongflags;intnr_freepages=cc->nr_freepages;structlist_head*freelist=&cc->freepages;
@@ -155,6 +233,8 @@ static void isolate_freepages(struct zone *zone,*/high_pfn=min(low_pfn,pfn);+zone_end_pfn=zone->zone_start_pfn+zone->spanned_pages;+/**Isolatefreepagesuntilenoughareavailabletomigratethe*pagesoncc->migratepages.Westopsearchingifthemigrate
@@ -191,7 +271,9 @@ 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_block(pfn,end_pfn,+freelist,false);nr_freepages+=isolated;}spin_unlock_irqrestore(&zone->lock,flags);
On Thu, Jan 26, 2012 at 10:00:46AM +0100, Marek Szyprowski wrote:
From: Michal Nazarewicz <redacted>
This commit introduces isolate_freepages_range() function which
generalises isolate_freepages_block() so that it can be used on
arbitrary PFN ranges.
isolate_freepages_block() is left with only minor changes.
The minor changes to isolate_freepages_block() look fine in
terms of how current compaction works. I have a minor comment on
isolate_freepages_range() but it is up to you whether to address them
or not. Whether you alter isolate_freepages_range() or not;
Acked-by: Mel Gorman <redacted>
quoted hunk
<SNIP>
@@ -105,6 +109,80 @@ static unsigned long isolate_freepages_block(struct zone *zone, return total_isolated; }+/**+ * isolate_freepages_range() - isolate free pages.+ * @start_pfn: The first PFN to start isolating.+ * @end_pfn: The one-past-last PFN.+ *+ * Non-free pages, invalid PFNs, or zone boundaries within the+ * [start_pfn, end_pfn) range are considered errors, cause function to+ * undo its actions and return zero.+ *+ * Otherwise, function returns one-past-the-last PFN of isolated page+ * (which may be greater then end_pfn if end fell in a middle of+ * a free page).+ */+static unsigned long+isolate_freepages_range(unsigned long start_pfn, unsigned long end_pfn)+{+ unsigned long isolated, pfn, block_end_pfn, flags;+ struct zone *zone = NULL;+ LIST_HEAD(freelist);+ struct page *page;++ for (pfn = start_pfn; pfn < end_pfn; pfn += isolated) {+ if (!pfn_valid(pfn))+ break;++ if (!zone)+ zone = page_zone(pfn_to_page(pfn));+ else if (zone != page_zone(pfn_to_page(pfn)))+ break;+
So what you are checking for here is if you straddle zones.
You could just initialise zone outside of the for loop. You can
then check outside the loop if end_pfn is in a different zone to
start_pfn. If it is, either adjust end_pfn accordingly or bail the
entire operation avoiding the need for release_freepages() later. This
will be a little cheaper.
+ /*
+ * On subsequent iterations round_down() is actually not
+ * needed, but we keep it that we not to complicate the code.
+ */
+ block_end_pfn = round_down(pfn, pageblock_nr_pages)
+ + pageblock_nr_pages;
Seems a little more involved than it needs to be. Something like
this might suit and be a bit nicer?
block_end_pfn = ALIGN(pfn+1, pageblock_nr_pages);
+ block_end_pfn = min(block_end_pfn, end_pfn);
+
+ spin_lock_irqsave(&zone->lock, flags);
+ isolated = isolate_freepages_block(pfn, block_end_pfn,
+ &freelist, true);
+ spin_unlock_irqrestore(&zone->lock, flags);
+
+ /*
+ * In strict mode, isolate_freepages_block() returns 0 if
+ * there are any holes in the block (ie. invalid PFNs or
+ * non-free pages).
+ */
+ if (!isolated)
+ break;
+
+ /*
+ * If we managed to isolate pages, it is always (1 << n) *
+ * pageblock_nr_pages for some non-negative n. (Max order
+ * page may span two pageblocks).
+ */
+ }
+
+ /* split_free_page does not map the pages */
+ list_for_each_entry(page, &freelist, lru) {
+ arch_alloc_page(page, 0);
+ kernel_map_pages(page, 1, 1);
+ }
+
This block is copied in two places - isolate_freepages and
isolate_freepages_range() so sharing a common helper would be nice. I
suspect you didn't because it would interfere with existing code more
than was strictly necessary which I complained about previously as
it made review harder. If that was your thinking, then just create
this helper in a separate patch. It's not critical though.
+ if (pfn < end_pfn) {
+ /* Loop terminated early, cleanup. */
+ release_freepages(&freelist);
+ return 0;
+ }
+
+ /* We don't use freelists for anything. */
+ return pfn;
+}
+
/* Returns true if the page is within a block suitable for migration to */
static bool suitable_migration_target(struct page *page)
{
On Mon, Jan 30, 2012 at 11:48:20AM +0000, Mel Gorman wrote:
quoted
+ if (!zone)
+ zone = page_zone(pfn_to_page(pfn));
+ else if (zone != page_zone(pfn_to_page(pfn)))
+ break;
+
So what you are checking for here is if you straddle zones.
You could just initialise zone outside of the for loop. You can
then check outside the loop if end_pfn is in a different zone to
start_pfn. If it is, either adjust end_pfn accordingly or bail the
entire operation avoiding the need for release_freepages() later. This
will be a little cheaper.
Whoops, silly me! You are watching for overlapping zones which can
happen in some rare configurations and for that checking page_zone()
like this is necessary. You can still initialise zone outside the loop
but the page_zone() check is still necessary.
My bad.
--
Mel Gorman
SUSE Labs
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:01:10
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 <redacted>
---
Documentation/kernel-parameters.txt | 5 +
arch/Kconfig | 3 +
drivers/base/Kconfig | 89 ++++++++
drivers/base/Makefile | 1 +
drivers/base/dma-contiguous.c | 404 ++++++++++++++++++++++++++++++++++
include/asm-generic/dma-contiguous.h | 27 +++
include/linux/device.h | 4 +
include/linux/dma-contiguous.h | 110 +++++++++
8 files changed, 643 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
@@ -508,6 +508,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted. Also note the kernel might malfunction if you disable some critical bits.+ cma=nn[MG] [ARM,KNL]+ Sets the size of kernel global memory area for contiguous+ memory allocations. For more information, see+ include/linux/dma-contiguous.h+ cmo_free_hint= [PPC] Format: { yes | no } Specify whether pages are marked as being inactive when they are freed. This is used in CMO environments
@@ -189,4 +189,93 @@ config DMA_SHARED_BUFFERAPIsextension;thefile'sdescriptorcanthenbepassedontootherdriver.+configCMA+bool"Contiguous Memory Allocator (EXPERIMENTAL)"+depends onHAVE_DMA_CONTIGUOUS&&HAVE_MEMBLOCK&&EXPERIMENTAL+selectMIGRATION+help+ThisenablestheContiguousMemoryAllocatorwhichallowsdrivers+toallocatebigphysically-contiguousblocksofmemoryforusewith+hardwarecomponentsthatdonotsupportI/Omapnorscatter-gather.++Formoreinformationsee<include/linux/dma-contiguous.h>.+Ifunsure,say"n".++ifCMA++configCMA_DEBUG+bool"CMA debug messages (DEVELOPMENT)"+depends onDEBUG_KERNEL+help+TurnsondebugmessagesinCMA.ThisproducesKERN_DEBUG+messagesforeveryCMAcallaswellasvariousmessageswhile+processingcallssuchasdma_alloc_from_contiguous().+Thisoptiondoesnotaffectwarninganderrormessages.++comment"Default contiguous memory area size:"++configCMA_SIZE_MBYTES+int"Size in Mega Bytes"+depends on!CMA_SIZE_SEL_PERCENTAGE+default16+help+Definesthesize(inMiB)ofthedefaultmemoryareaforContiguous+MemoryAllocator.++configCMA_SIZE_PERCENTAGE+int"Percentage of total memory"+depends on!CMA_SIZE_SEL_MBYTES+default10+help+DefinesthesizeofthedefaultmemoryareaforContiguousMemory+Allocatorasapercentageofthetotalmemoryinthesystem.++choice+prompt"Selected region size"+defaultCMA_SIZE_SEL_ABSOLUTE++configCMA_SIZE_SEL_MBYTES+bool"Use mega bytes value only"++configCMA_SIZE_SEL_PERCENTAGE+bool"Use percentage value only"++configCMA_SIZE_SEL_MIN+bool"Use lower value (minimum)"++configCMA_SIZE_SEL_MAX+bool"Use higher value (maximum)"++endchoice++configCMA_ALIGNMENT+int"Maximum PAGE_SIZE order of alignment for contiguous buffers"+range49+default8+help+DMAmappingframeworkbydefaultalignsallbufferstothesmallest+PAGE_SIZEorderwhichisgreaterthanorequaltotherequestedbuffer+size.Thisworkswellforbuffersuptoafewhundredskilobytes,but+forlargerbuffersitjustamemorywaste.Withthisparameteryoucan+specifythemaximumPAGE_SIZEorderforcontiguousbuffers.Larger+bufferswillbealignedonlytothisspecifiedorder.Theorderis+expressedasapoweroftwomultipliedbythePAGE_SIZE.++Forexample,ifyoursystemdefaultsto4KiBpages,theordervalue+of8meansthatthebufferswillbealignedupto1MiBonly.++Ifunsure,leavethedefaultvalue"8".++configCMA_AREAS+int"Maximum count of the CMA device-private areas"+default7+help+CMAallowstocreateCMAareasforparticulardevices.Thisparameter+setsthemaximumnumberofsuchdeviceprivateCMAareasinthe+system.++Ifunsure,leavethedefaultvalue"7".++endif+endmenu
Hi Marek,
With v19, I can't seem to allocate big regions anymore (e.g. 101MiB).
In particular, this seems to fail:
On Thu, Jan 26, 2012 at 11:00 AM, Marek Szyprowski
[off-list ref] wrote:
The above WARN_ON_ONCE is triggered, and then the conditional is
asserted (page_zone() retuns a "Movable" zone, whereas zone is
"Normal") and the function fails.
This happens to me on OMAP4 with your 3.3-rc1-cma-v19 branch (and a
bunch of remoteproc/rpmsg patches).
Do big allocations work for you ?
Thanks,
Ohad.
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-27 10:53:38
Hi Ohad,
On Friday, January 27, 2012 10:44 AM Ohad Ben-Cohen wrote:
With v19, I can't seem to allocate big regions anymore (e.g. 101MiB).
In particular, this seems to fail:
On Thu, Jan 26, 2012 at 11:00 AM, Marek Szyprowski
[off-list ref] wrote:
The above WARN_ON_ONCE is triggered, and then the conditional is
asserted (page_zone() retuns a "Movable" zone, whereas zone is
"Normal") and the function fails.
This happens to me on OMAP4 with your 3.3-rc1-cma-v19 branch (and a
bunch of remoteproc/rpmsg patches).
Do big allocations work for you ?
I've tested it with 256MiB on Exynos4 platform. Could you check if the
problem also appears on 3.2-cma-v19 branch (I've uploaded it a few hours
ago) and 3.2-cma-v18? Both are available on our public repo:
git://git.infradead.org/users/kmpark/linux-samsung/
The above code has not been changed since v16, so I'm really surprised
that it causes problems. Maybe the memory configuration or layout has
been changed in 3.3-rc1 for OMAP4?
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
From: Clark, Rob <hidden> Date: 2012-01-27 14:27:36
2012/1/27 Marek Szyprowski [off-list ref]:
Hi Ohad,
On Friday, January 27, 2012 10:44 AM Ohad Ben-Cohen wrote:
quoted
With v19, I can't seem to allocate big regions anymore (e.g. 101MiB).
In particular, this seems to fail:
On Thu, Jan 26, 2012 at 11:00 AM, Marek Szyprowski
[off-list ref] wrote:
The above WARN_ON_ONCE is triggered, and then the conditional is
asserted (page_zone() retuns a "Movable" zone, whereas zone is
"Normal") and the function fails.
This happens to me on OMAP4 with your 3.3-rc1-cma-v19 branch (and a
bunch of remoteproc/rpmsg patches).
Do big allocations work for you ?
I've tested it with 256MiB on Exynos4 platform. Could you check if the
problem also appears on 3.2-cma-v19 branch (I've uploaded it a few hours
ago) and 3.2-cma-v18? Both are available on our public repo:
git://git.infradead.org/users/kmpark/linux-samsung/
The above code has not been changed since v16, so I'm really surprised
that it causes problems. Maybe the memory configuration or layout has
been changed in 3.3-rc1 for OMAP4?
is highmem still an issue? I remember hitting this WARN_ON_ONCE() but
went away after I switched to a 2g/2g vm split (which avoids highmem)
BR,
-R
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
_______________________________________________
Linaro-mm-sig mailing list
Linaro-mm-sig at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-27 14:51:16
Hello,
On Friday, January 27, 2012 3:28 PM Clark, Rob wrote:
2012/1/27 Marek Szyprowski [off-list ref]:
quoted
Hi Ohad,
On Friday, January 27, 2012 10:44 AM Ohad Ben-Cohen wrote:
quoted
With v19, I can't seem to allocate big regions anymore (e.g. 101MiB).
In particular, this seems to fail:
On Thu, Jan 26, 2012 at 11:00 AM, Marek Szyprowski
[off-list ref] wrote:
The above WARN_ON_ONCE is triggered, and then the conditional is
asserted (page_zone() retuns a "Movable" zone, whereas zone is
"Normal") and the function fails.
This happens to me on OMAP4 with your 3.3-rc1-cma-v19 branch (and a
bunch of remoteproc/rpmsg patches).
Do big allocations work for you ?
I've tested it with 256MiB on Exynos4 platform. Could you check if the
problem also appears on 3.2-cma-v19 branch (I've uploaded it a few hours
ago) and 3.2-cma-v18? Both are available on our public repo:
git://git.infradead.org/users/kmpark/linux-samsung/
The above code has not been changed since v16, so I'm really surprised
that it causes problems. Maybe the memory configuration or layout has
been changed in 3.3-rc1 for OMAP4?
is highmem still an issue? I remember hitting this WARN_ON_ONCE() but
went away after I switched to a 2g/2g vm split (which avoids highmem)
No, it shouldn't be an issue. I've tested CMA v19 on a system with 1GiB of
the memory and general purpose (global) cma region was allocated correctly
at the end of low memory. For device private regions you should take care
of correct placement by yourself, so maybe this is an issue in this case?
Ohad, could you tell a bit more about your issue? Does this 'large region'
is a device private region (declared with dma_declare_contiguous()) or is it
a global one (defined in Kconfig or cma= kernel boot parameter)?
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
There have been some vmalloc layout changes merged to v3.3-rc1. Please check
if the hardcoded OMAP_RPROC_CMA_BASE+CONFIG_OMAP_DUCATI_CMA_SIZE fits into kernel
low-memory. Some hints you can find after the "Virtual kernel memory layout:"
message during boot and using "cat /proc/iomem".
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
Hi Marek,
On Fri, Jan 27, 2012 at 5:17 PM, Marek Szyprowski
[off-list ref] wrote:
There have been some vmalloc layout changes merged to v3.3-rc1.
That was dead-on, thanks a lot!
I did then bump into a different allocation failure which happened
because dma_alloc_from_contiguous() computes 'mask' before capping the
'align' argument.
The early 'mask' computation was added in v18 (and therefore exists in
v19 too) and I was actually testing v17 previously, so I didn't notice
it before.
You may want to squash something like this:
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-30 07:44:04
Hello,
On Saturday, January 28, 2012 7:57 PM Ohad Ben-Cohen wrote:
On Fri, Jan 27, 2012 at 5:17 PM, Marek Szyprowski
[off-list ref] wrote:
quoted
There have been some vmalloc layout changes merged to v3.3-rc1.
That was dead-on, thanks a lot!
Did you managed to fix this issue?
I did then bump into a different allocation failure which happened
because dma_alloc_from_contiguous() computes 'mask' before capping the
'align' argument.
The early 'mask' computation was added in v18 (and therefore exists in
v19 too) and I was actually testing v17 previously, so I didn't notice
it before.
Right, thanks for spotting it, I will squash it to the next release.
Hi Marek,
On Mon, Jan 30, 2012 at 9:43 AM, Marek Szyprowski
[off-list ref] wrote:
Did you managed to fix this issue?
Yes -- the recent increase in the vmalloc region triggered a bigger
truncation in the system RAM than we had before, and therefore
conflicted with the previous hardcoded region we were using.
Long term, our plan is to get rid of those hardcoded values, but for
the moment our remote RTOS still needs to know the physical address in
advance.
Right, thanks for spotting it, I will squash it to the next release.
Thanks. With that hunk squashed in, feel free to add my Tested-by tag
to the patches.
Thanks!
Ohad.
I've tested it with 256MiB on Exynos4 platform. Could you check if the
problem also appears on 3.2-cma-v19 branch (I've uploaded it a few hours
ago)
Exactly what I needed, thanks :)
Both v18 and v19 seem to work fine with 3.2.
The above code has not been changed since v16, so I'm really surprised
that it causes problems. Maybe the memory configuration or layout has
been changed in 3.3-rc1 for OMAP4?
Not sure what the culprit is, but it is only triggered with 3.3-rc1.
I'll tell you if I find anything.
Thanks!
Ohad.
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:01:14
This patch adds support for CMA to dma-mapping subsystem for ARM
architecture. By default a global CMA area is used, but specific devices
are allowed to have their private memory areas if required (they can be
created with dma_declare_contiguous() function during board
initialization).
Contiguous memory areas reserved for DMA are remapped with 2-level page
tables on boot. Once a buffer is requested, a low memory kernel mapping
is updated to to match requested memory access type.
GFP_ATOMIC allocations are performed from special pool which is created
early during boot. This way remapping page attributes is not needed on
allocation time.
CMA has been enabled unconditionally for ARMv6+ systems.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Michal Nazarewicz <redacted>
---
Documentation/kernel-parameters.txt | 4 +
arch/arm/Kconfig | 2 +
arch/arm/include/asm/dma-contiguous.h | 16 ++
arch/arm/include/asm/mach/map.h | 1 +
arch/arm/kernel/setup.c | 9 +-
arch/arm/mm/dma-mapping.c | 368 +++++++++++++++++++++++++++------
arch/arm/mm/init.c | 22 ++-
arch/arm/mm/mm.h | 3 +
arch/arm/mm/mmu.c | 31 ++-
9 files changed, 368 insertions(+), 88 deletions(-)
create mode 100644 arch/arm/include/asm/dma-contiguous.h
@@ -520,6 +520,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted. a hypervisor. Default: yes+ coherent_pool=nn[KMG] [ARM,KNL]+ Sets the size of memory pool for coherent, atomic dma+ allocations if Contiguous Memory Allocator (CMA) is used.+ code_bytes [X86] How many bytes of object code to print in an oops report. Range: 0 - 8192
@@ -170,6 +163,9 @@ static int __init consistent_init(void)unsignedlongbase=consistent_base;unsignedlongnum_ptes=(CONSISTENT_END-base)>>PMD_SHIFT;+if(cpu_architecture()>=CPU_ARCH_ARMv6)+return0;+consistent_pte=kmalloc(num_ptes*sizeof(pte_t),GFP_KERNEL);if(!consistent_pte){pr_err("%s: no memory\n",__func__);
@@ -210,9 +206,101 @@ static int __init consistent_init(void)returnret;}-core_initcall(consistent_init);+staticvoid*__alloc_from_contiguous(structdevice*dev,size_tsize,+pgprot_tprot,structpage**ret_page);++staticstructarm_vmregion_headcoherent_head={+.vm_lock=__SPIN_LOCK_UNLOCKED(&coherent_head.vm_lock),+.vm_list=LIST_HEAD_INIT(coherent_head.vm_list),+};++size_tcoherent_pool_size=DEFAULT_CONSISTENT_DMA_SIZE/8;++staticint__initearly_coherent_pool(char*p)+{+coherent_pool_size=memparse(p,&p);+return0;+}+early_param("coherent_pool",early_coherent_pool);++/*+*Initialisethecoherentpoolforatomicallocations.+*/+staticint__initcoherent_init(void)+{+pgprot_tprot=pgprot_dmacoherent(pgprot_kernel);+size_tsize=coherent_pool_size;+structpage*page;+void*ptr;++if(cpu_architecture()<CPU_ARCH_ARMv6)+return0;++ptr=__alloc_from_contiguous(NULL,size,prot,&page);+if(ptr){+coherent_head.vm_start=(unsignedlong)ptr;+coherent_head.vm_end=(unsignedlong)ptr+size;+printk(KERN_INFO"DMA: preallocated %u KiB pool for atomic coherent allocations\n",+(unsigned)size/1024);+return0;+}+printk(KERN_ERR"DMA: failed to allocate %u KiB pool for atomic coherent allocation\n",+(unsigned)size/1024);+return-ENOMEM;+}+/*+*CMAisactivatedbycore_initcall,sowemustbecalledafterit+*/+postcore_initcall(coherent_init);++structdma_contig_early_reserve{+phys_addr_tbase;+unsignedlongsize;+};++staticstructdma_contig_early_reservedma_mmu_remap[MAX_CMA_AREAS]__initdata;++staticintdma_mmu_remap_num__initdata;++void__initdma_contiguous_early_fixup(phys_addr_tbase,unsignedlongsize)+{+dma_mmu_remap[dma_mmu_remap_num].base=base;+dma_mmu_remap[dma_mmu_remap_num].size=size;+dma_mmu_remap_num++;+}++void__initdma_contiguous_remap(void)+{+inti;+for(i=0;i<dma_mmu_remap_num;i++){+phys_addr_tstart=dma_mmu_remap[i].base;+phys_addr_tend=start+dma_mmu_remap[i].size;+structmap_descmap;+unsignedlongaddr;++if(end>arm_lowmem_limit)+end=arm_lowmem_limit;+if(start>=end)+return;++map.pfn=__phys_to_pfn(start);+map.virtual=__phys_to_virt(start);+map.length=end-start;+map.type=MT_MEMORY_DMA_READY;++/*+*Clearpreviouslow-memorymapping+*/+for(addr=__phys_to_virt(start);addr<__phys_to_virt(end);+addr+=PGDIR_SIZE)+pmd_clear(pmd_off_k(addr));++iotable_init(&map,1);+}+}+staticvoid*__dma_alloc_remap(structpage*page,size_tsize,gfp_tgfp,pgprot_tprot){
@@ -318,20 +406,172 @@ static void __dma_free_remap(void *cpu_addr, size_t size)arm_vmregion_free(&consistent_head,c);}+staticint__dma_update_pte(pte_t*pte,pgtable_ttoken,unsignedlongaddr,+void*data)+{+structpage*page=virt_to_page(addr);+pgprot_tprot=*(pgprot_t*)data;++set_pte_ext(pte,mk_pte(page,prot),0);+return0;+}++staticvoid__dma_remap(structpage*page,size_tsize,pgprot_tprot)+{+unsignedlongstart=(unsignedlong)page_address(page);+unsignedend=start+size;++apply_to_page_range(&init_mm,start,size,__dma_update_pte,&prot);+dsb();+flush_tlb_kernel_range(start,end);+}++staticvoid*__alloc_remap_buffer(structdevice*dev,size_tsize,gfp_tgfp,+pgprot_tprot,structpage**ret_page)+{+structpage*page;+void*ptr;+page=__dma_alloc_buffer(dev,size,gfp);+if(!page)+returnNULL;++ptr=__dma_alloc_remap(page,size,gfp,prot);+if(!ptr){+__dma_free_buffer(page,size);+returnNULL;+}++*ret_page=page;+returnptr;+}++staticvoid*__alloc_from_pool(structdevice*dev,size_tsize,+structpage**ret_page)+{+structarm_vmregion*c;+size_talign;++if(!coherent_head.vm_start){+printk(KERN_ERR"%s: coherent pool not initialised!\n",+__func__);+dump_stack();+returnNULL;+}++/*+*Aligntheregionallocation-allocationsfrompoolarerather+*small,soalignthemtotheirorderinpages,minimumisapage+*size.ThishelpsreducefragmentationoftheDMAspace.+*/+align=PAGE_SIZE<<get_order(size);+c=arm_vmregion_alloc(&coherent_head,align,size,0);+if(c){+void*ptr=(void*)c->vm_start;+structpage*page=virt_to_page(ptr);+*ret_page=page;+returnptr;+}+returnNULL;+}++staticint__free_from_pool(void*cpu_addr,size_tsize)+{+unsignedlongstart=(unsignedlong)cpu_addr;+unsignedlongend=start+size;+structarm_vmregion*c;++if(start<coherent_head.vm_start||end>coherent_head.vm_end)+return0;++c=arm_vmregion_find_remove(&coherent_head,(unsignedlong)start);++if((c->vm_end-c->vm_start)!=size){+printk(KERN_ERR"%s: freeing wrong coherent size (%ld != %d)\n",+__func__,c->vm_end-c->vm_start,size);+dump_stack();+size=c->vm_end-c->vm_start;+}++arm_vmregion_free(&coherent_head,c);+return1;+}++staticvoid*__alloc_from_contiguous(structdevice*dev,size_tsize,+pgprot_tprot,structpage**ret_page)+{+unsignedlongorder=get_order(size);+size_tcount=size>>PAGE_SHIFT;+structpage*page;++page=dma_alloc_from_contiguous(dev,count,order);+if(!page)+returnNULL;++__dma_clear_buffer(page,size);+__dma_remap(page,size,prot);++*ret_page=page;+returnpage_address(page);+}++staticvoid__free_from_contiguous(structdevice*dev,structpage*page,+size_tsize)+{+__dma_remap(page,size,pgprot_kernel);+dma_release_from_contiguous(dev,page,size>>PAGE_SHIFT);+}++#define nommu() 0+#else /* !CONFIG_MMU */-#define __dma_alloc_remap(page, size, gfp, prot) page_address(page)-#define __dma_free_remap(addr, size) do { } while (0)+#define nommu() 1++#define __alloc_remap_buffer(dev, size, gfp, prot, ret) NULL+#define __alloc_from_pool(dev, size, ret_page) NULL+#define __alloc_from_contiguous(dev, size, prot, ret) NULL+#define __free_from_pool(cpu_addr, size) 0+#define __free_from_contiguous(dev, page, size) do { } while (0)+#define __dma_free_remap(cpu_addr, size) do { } while (0)#endif /* CONFIG_MMU */-staticvoid*-__dma_alloc(structdevice*dev,size_tsize,dma_addr_t*handle,gfp_tgfp,-pgprot_tprot)+staticvoid*__alloc_simple_buffer(structdevice*dev,size_tsize,gfp_tgfp,+structpage**ret_page){structpage*page;+page=__dma_alloc_buffer(dev,size,gfp);+if(!page)+returnNULL;++*ret_page=page;+returnpage_address(page);+}++++staticvoid*__dma_alloc(structdevice*dev,size_tsize,dma_addr_t*handle,+gfp_tgfp,pgprot_tprot)+{+u64mask=get_coherent_dma_mask(dev);+structpage*page;void*addr;+#ifdef CONFIG_DMA_API_DEBUG+u64limit=(mask+1)&~mask;+if(limit&&size>=limit){+dev_warn(dev,"coherent allocation too big (requested %#x mask %#llx)\n",+size,mask);+returnNULL;+}+#endif++if(!mask)+returnNULL;++if(mask<0xffffffffULL)+gfp|=GFP_DMA;+/**Followingisawork-around(a.k.a.hack)topreventpages*with__GFP_COMPbeingpassedtosplit_page()whichcannot
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:01:39
This patch adds support for CMA to dma-mapping subsystem for x86
architecture that uses common pci-dma/pci-nommu implementation. This
allows to test CMA on KVM/QEMU and a lot of common x86 boxes.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Michal Nazarewicz <redacted>
---
arch/x86/Kconfig | 1 +
arch/x86/include/asm/dma-contiguous.h | 13 +++++++++++++
arch/x86/include/asm/dma-mapping.h | 4 ++++
arch/x86/kernel/pci-dma.c | 18 ++++++++++++++++--
arch/x86/kernel/pci-nommu.c | 8 +-------
arch/x86/kernel/setup.c | 2 ++
6 files changed, 37 insertions(+), 9 deletions(-)
create mode 100644 arch/x86/include/asm/dma-contiguous.h
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:01:58
From: Michal Nazarewicz <redacted>
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 (to some degree) 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 for allocating big chunks (eg. 10MiB)
of physically contiguous memory. Once driver requests
contiguous memory, pages from MIGRATE_CMA pageblocks may be
migrated away to create a contiguous block.
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.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
include/linux/mmzone.h | 43 +++++++++++++++++++++----
include/linux/page-isolation.h | 3 ++
mm/Kconfig | 2 +-
mm/compaction.c | 11 +++++--
mm/page_alloc.c | 68 +++++++++++++++++++++++++++++++++-------
mm/vmstat.c | 3 ++
6 files changed, 107 insertions(+), 23 deletions(-)
@@ -35,13 +35,37 @@*/#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,+#ifdef CONFIG_CMA+/*+*MIGRATE_CMAmigrationtypeisdesignedtomimictheway+*ZONE_MOVABLEworks.Onlymovablepagescanbeallocated+*fromMIGRATE_CMApageblocksandpageallocatornever+*implicitlychangemigrationtypeofMIGRATE_CMApageblock.+*+*Thewaytouseitistochangemigratetypeofarangeof+*pageblockstoMIGRATE_CMAwhichcanbedoneby+*__free_pageblock_cma()function.Whatisimportantthough+*isthatarangeofpageblocksmustbealignedto+*MAX_ORDER_NR_PAGESshouldbiggestpagebebiggerthen+*asinglepageblock.+*/+MIGRATE_CMA,+#endif+MIGRATE_ISOLATE,/* can't allocate from here */+MIGRATE_TYPES+};++#ifdef CONFIG_CMA+# define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)+#else+# define is_migrate_cma(migratetype) false+#endif#define for_each_migratetype_order(order, type) \for(order=0;order<MAX_ORDER;order++)\
@@ -54,6 +78,11 @@ static inline int get_pageblock_migratetype(struct page *page)returnget_pageblock_flags_group(page,PB_migrate,PB_migrate_end);}+staticinlineboolis_pageblock_cma(structpage*page)+{+returnis_migrate_cma(get_pageblock_migratetype(page));+}+structfree_area{structlist_headfree_list[MIGRATE_TYPES];unsignedlongnr_free;
@@ -35,6 +35,11 @@ static unsigned long release_freepages(struct list_head *freelist)returncount;}+staticinlineboolmigrate_async_suitable(intmigratetype)+{+returnis_migrate_cma(migratetype)||migratetype==MIGRATE_MOVABLE;+}+/**Isolatefreepagesontoaprivatefreelist.Callermustholdzone->lock.*If@strictistrue,willabortreturning0onanyinvalidPFNsornon-free
@@ -274,7 +279,7 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,*/pageblock_nr=low_pfn>>pageblock_order;if(!cc->sync&&last_pageblock_nr!=pageblock_nr&&-get_pageblock_migratetype(page)!=MIGRATE_MOVABLE){+migrate_async_suitable(get_pageblock_migratetype(page))){low_pfn+=pageblock_nr_pages;low_pfn=ALIGN(low_pfn,pageblock_nr_pages)-1;last_pageblock_nr=pageblock_nr;
@@ -342,8 +347,8 @@ static bool suitable_migration_target(struct page *page)if(PageBuddy(page)&&page_order(page)>=pageblock_order)returntrue;-/* If the block is MIGRATE_MOVABLE, allow migration */-if(migratetype==MIGRATE_MOVABLE)+/* If the block is MIGRATE_MOVABLE or MIGRATE_CMA, allow migration */+if(migrate_async_suitable(migratetype))returntrue;/* Otherwise skip the block */
@@ -875,10 +895,15 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,*Thisarraydescribestheorderlistsarefallenbacktowhen*thefreelistsforthedesirablemigratetypearedepleted*/-staticintfallbacks[MIGRATE_TYPES][3]={+staticintfallbacks[MIGRATE_TYPES][4]={[MIGRATE_UNMOVABLE]={MIGRATE_RECLAIMABLE,MIGRATE_MOVABLE,MIGRATE_RESERVE},[MIGRATE_RECLAIMABLE]={MIGRATE_UNMOVABLE,MIGRATE_MOVABLE,MIGRATE_RESERVE},+#ifdef CONFIG_CMA+[MIGRATE_MOVABLE]={MIGRATE_RECLAIMABLE,MIGRATE_UNMOVABLE,MIGRATE_CMA,MIGRATE_RESERVE},+[MIGRATE_CMA]={MIGRATE_RESERVE},/* Never used */+#else[MIGRATE_MOVABLE]={MIGRATE_RECLAIMABLE,MIGRATE_UNMOVABLE,MIGRATE_RESERVE},+#endif[MIGRATE_RESERVE]={MIGRATE_RESERVE},/* Never used */[MIGRATE_ISOLATE]={MIGRATE_RESERVE},/* Never used */};
@@ -995,11 +1020,18 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)*pagestothepreferredallocationlist.Iffalling*backforareclaimablekernelallocation,bemore*aggressiveabouttakingownershipoffreepages+*+*Ontheotherhand,neverchangemigration+*typeofMIGRATE_CMApageblocksnormoveCMA+*pagesondifferentfreelists.Wedon't+*wantunmovablepagestobeallocatedfrom+*MIGRATE_CMAareas.*/-if(unlikely(current_order>=(pageblock_order>>1))||-start_migratetype==MIGRATE_RECLAIMABLE||-page_group_by_mobility_disabled){-unsignedlongpages;+if(!is_pageblock_cma(page)&&+(unlikely(current_order>=pageblock_order/2)||+start_migratetype==MIGRATE_RECLAIMABLE||+page_group_by_mobility_disabled)){+intpages;pages=move_freepages_block(zone,page,start_migratetype);
@@ -1017,11 +1049,14 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)rmv_page_order(page);/* Take ownership for orders >= pageblock_order */-if(current_order>=pageblock_order)+if(current_order>=pageblock_order&&+!is_pageblock_cma(page))change_pageblock_range(page,current_order,start_migratetype);-expand(zone,page,order,current_order,area,migratetype);+expand(zone,page,order,current_order,area,+is_migrate_cma(start_migratetype)+?start_migratetype:migratetype);trace_mm_page_alloc_extfrag(page,order,current_order,start_migratetype,migratetype);
@@ -1093,7 +1128,12 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,list_add(&page->lru,list);elselist_add_tail(&page->lru,list);-set_page_private(page,migratetype);+#ifdef CONFIG_CMA+if(is_pageblock_cma(page))+set_page_private(page,MIGRATE_CMA);+else+#endif+set_page_private(page,migratetype);list=&page->lru;}__mod_zone_page_state(zone,NR_FREE_PAGES,-(i<<order));
@@ -1337,8 +1377,12 @@ int split_free_page(struct page *page)if(order>=pageblock_order-1){structpage*endpage=page+(1<<order)-1;-for(;page<endpage;page+=pageblock_nr_pages)-set_pageblock_migratetype(page,MIGRATE_MOVABLE);+for(;page<endpage;page+=pageblock_nr_pages){+intmt=get_pageblock_migratetype(page);+if(mt!=MIGRATE_ISOLATE&&!is_migrate_cma(mt))+set_pageblock_migratetype(page,+MIGRATE_MOVABLE);+}}return1<<order;
@@ -5375,8 +5419,8 @@ __count_immobile_pages(struct zone *zone, struct page *page, int count)*/if(zone_idx(zone)==ZONE_MOVABLE)returntrue;--if(get_pageblock_migratetype(page)==MIGRATE_MOVABLE)+if(get_pageblock_migratetype(page)==MIGRATE_MOVABLE||+is_pageblock_cma(page))returntrue;pfn=page_to_pfn(page);
On Thu, Jan 26, 2012 at 10:00:50AM +0100, Marek Szyprowski wrote:
quoted hunk
From: Michal Nazarewicz <redacted>
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 (to some degree) 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 for allocating big chunks (eg. 10MiB)
of physically contiguous memory. Once driver requests
contiguous memory, pages from MIGRATE_CMA pageblocks may be
migrated away to create a contiguous block.
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.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
include/linux/mmzone.h | 43 +++++++++++++++++++++----
include/linux/page-isolation.h | 3 ++
mm/Kconfig | 2 +-
mm/compaction.c | 11 +++++--
mm/page_alloc.c | 68 +++++++++++++++++++++++++++++++++-------
mm/vmstat.c | 3 ++
6 files changed, 107 insertions(+), 23 deletions(-)
@@ -35,13 +35,37 @@*/#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,+#ifdef CONFIG_CMA+/*+*MIGRATE_CMAmigrationtypeisdesignedtomimictheway+*ZONE_MOVABLEworks.Onlymovablepagescanbeallocated+*fromMIGRATE_CMApageblocksandpageallocatornever+*implicitlychangemigrationtypeofMIGRATE_CMApageblock.+*+*Thewaytouseitistochangemigratetypeofarangeof+*pageblockstoMIGRATE_CMAwhichcanbedoneby+*__free_pageblock_cma()function.Whatisimportantthough+*isthatarangeofpageblocksmustbealignedto+*MAX_ORDER_NR_PAGESshouldbiggestpagebebiggerthen+*asinglepageblock.+*/+MIGRATE_CMA,+#endif+MIGRATE_ISOLATE,/* can't allocate from here */+MIGRATE_TYPES+};++#ifdef CONFIG_CMA+# define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)+#else+# define is_migrate_cma(migratetype) false+#endif#define for_each_migratetype_order(order, type) \for(order=0;order<MAX_ORDER;order++)\
@@ -54,6 +78,11 @@ static inline int get_pageblock_migratetype(struct page *page)returnget_pageblock_flags_group(page,PB_migrate,PB_migrate_end);}+staticinlineboolis_pageblock_cma(structpage*page)+{+returnis_migrate_cma(get_pageblock_migratetype(page));+}+structfree_area{structlist_headfree_list[MIGRATE_TYPES];unsignedlongnr_free;
@@ -35,6 +35,11 @@ static unsigned long release_freepages(struct list_head *freelist)returncount;}+staticinlineboolmigrate_async_suitable(intmigratetype)+{+returnis_migrate_cma(migratetype)||migratetype==MIGRATE_MOVABLE;+}+/**Isolatefreepagesontoaprivatefreelist.Callermustholdzone->lock.*If@strictistrue,willabortreturning0onanyinvalidPFNsornon-free
@@ -274,7 +279,7 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,*/pageblock_nr=low_pfn>>pageblock_order;if(!cc->sync&&last_pageblock_nr!=pageblock_nr&&-get_pageblock_migratetype(page)!=MIGRATE_MOVABLE){+migrate_async_suitable(get_pageblock_migratetype(page))){low_pfn+=pageblock_nr_pages;low_pfn=ALIGN(low_pfn,pageblock_nr_pages)-1;last_pageblock_nr=pageblock_nr;
@@ -342,8 +347,8 @@ static bool suitable_migration_target(struct page *page)if(PageBuddy(page)&&page_order(page)>=pageblock_order)returntrue;-/* If the block is MIGRATE_MOVABLE, allow migration */-if(migratetype==MIGRATE_MOVABLE)+/* If the block is MIGRATE_MOVABLE or MIGRATE_CMA, allow migration */+if(migrate_async_suitable(migratetype))returntrue;/* Otherwise skip the block */
@@ -875,10 +895,15 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,*Thisarraydescribestheorderlistsarefallenbacktowhen*thefreelistsforthedesirablemigratetypearedepleted*/-staticintfallbacks[MIGRATE_TYPES][3]={+staticintfallbacks[MIGRATE_TYPES][4]={[MIGRATE_UNMOVABLE]={MIGRATE_RECLAIMABLE,MIGRATE_MOVABLE,MIGRATE_RESERVE},[MIGRATE_RECLAIMABLE]={MIGRATE_UNMOVABLE,MIGRATE_MOVABLE,MIGRATE_RESERVE},+#ifdef CONFIG_CMA+[MIGRATE_MOVABLE]={MIGRATE_RECLAIMABLE,MIGRATE_UNMOVABLE,MIGRATE_CMA,MIGRATE_RESERVE},
This is a curious choice. MIGRATE_CMA is allowed to contain movable
pages. By using MIGRATE_RECLAIMABLE and MIGRATE_UNMOVABLE for movable
pages instead of MIGRATE_CMA, you increase the changes that unmovable
pages will need to use MIGRATE_MOVABLE in the future which impacts
fragmentation avoidance. I would recommend that you change this to
{ MIGRATE_CMA, MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE }
+ [MIGRATE_CMA] = { MIGRATE_RESERVE }, /* Never used */
+#else
[MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE },
+#endif
[MIGRATE_RESERVE] = { MIGRATE_RESERVE }, /* Never used */
[MIGRATE_ISOLATE] = { MIGRATE_RESERVE }, /* Never used */
};
You should also be aware that you may have problems with zone
balancing. If MIGRATE_CMA is large and it is the only free memory
then UNMOVABLE and RECLAIMABLE allocations will fail. kswapd will
not necessarily help because it is checking the watermarks and the
watermarks may be fine. It's actually the reason ZONE_MOVABLE was
created originally.
quoted hunk
@@ -995,11 +1020,18 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype) * pages to the preferred allocation list. If falling * back for a reclaimable kernel allocation, be more * aggressive about taking ownership of free pages+ *+ * On the other hand, never change migration+ * type of MIGRATE_CMA pageblocks nor move CMA+ * pages on different free lists. We don't+ * want unmovable pages to be allocated from+ * MIGRATE_CMA areas. */- if (unlikely(current_order >= (pageblock_order >> 1)) ||- start_migratetype == MIGRATE_RECLAIMABLE ||- page_group_by_mobility_disabled) {- unsigned long pages;+ if (!is_pageblock_cma(page) &&+ (unlikely(current_order >= pageblock_order / 2) ||+ start_migratetype == MIGRATE_RECLAIMABLE ||+ page_group_by_mobility_disabled)) {+ int pages;
You call is_pageblock_cma(page) here which in turn calls
get_pageblock_migratetype(). get_pageblock_migratetype() should be
avoided where possible and it is unecessary in this context because
we know what the migratetype of page. Use that information instead of
calling get_pageblock_migratetype().
@@ -1017,11 +1049,14 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype) rmv_page_order(page); /* Take ownership for orders >= pageblock_order */- if (current_order >= pageblock_order)+ if (current_order >= pageblock_order &&+ !is_pageblock_cma(page))
Same, the get_pageblock_migratetype() call can be avoided.
What is this check meant to be doing?
start_migratetype is determined by allocflags_to_migratetype() and
that never will be MIGRATE_CMA so is_migrate_cma(start_migratetype)
should always be false.
From: Michal Nazarewicz <hidden> Date: 2012-01-30 13:06:54
On Thu, Jan 26, 2012 at 10:00:50AM +0100, Marek Szyprowski wrote:
quoted
From: Michal Nazarewicz <redacted>
@@ -875,10 +895,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][3] = {+static int fallbacks[MIGRATE_TYPES][4] = { [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE, MIGRATE_RESERVE }, [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE, MIGRATE_RESERVE },+#ifdef CONFIG_CMA+ [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_CMA , MIGRATE_RESERVE },
On Mon, 30 Jan 2012 13:35:42 +0100, Mel Gorman [off-list ref] wrote:
This is a curious choice. MIGRATE_CMA is allowed to contain movable
pages. By using MIGRATE_RECLAIMABLE and MIGRATE_UNMOVABLE for movable
pages instead of MIGRATE_CMA, you increase the changes that unmovable
pages will need to use MIGRATE_MOVABLE in the future which impacts
fragmentation avoidance. I would recommend that you change this to
{ MIGRATE_CMA, MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE }
At the beginning the idea was to try hard not to get pages from MIGRATE_CMA
allocated at all, thus it was put at the end of the fallbacks list, but on
a busy system this probably won't help anyway, so I'll change it per your
suggestion.
quoted
@@ -1017,11 +1049,14 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype) rmv_page_order(page); /* Take ownership for orders >= pageblock_order */- if (current_order >= pageblock_order)+ if (current_order >= pageblock_order &&+ !is_pageblock_cma(page)) change_pageblock_range(page, current_order, start_migratetype);- expand(zone, page, order, current_order, area, migratetype);+ expand(zone, page, order, current_order, area,+ is_migrate_cma(start_migratetype)+ ? start_migratetype : migratetype);
What is this check meant to be doing?
start_migratetype is determined by allocflags_to_migratetype() and
that never will be MIGRATE_CMA so is_migrate_cma(start_migratetype)
should always be false.
Right, thanks! This should be the other way around, ie.:
+ expand(zone, page, order, current_order, area,
+ is_migrate_cma(migratetype)
+ ? migratetype : start_migratetype);
I'll fix this and the calls to is_pageblock_cma().
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Micha? ?mina86? Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
What is this check meant to be doing?
start_migratetype is determined by allocflags_to_migratetype() and
that never will be MIGRATE_CMA so is_migrate_cma(start_migratetype)
should always be false.
Right, thanks! This should be the other way around, ie.:
+ expand(zone, page, order, current_order, area,
+ is_migrate_cma(migratetype)
+ ? migratetype : start_migratetype);
I'll fix this and the calls to is_pageblock_cma().
That makes a lot more sense. Thanks.
I have a vague recollection that there was a problem with finding
unmovable pages in MIGRATE_CMA regions. This might have been part of
the problem.
--
Mel Gorman
SUSE Labs
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:02:00
Replace custom memory bank initialization using memblock_reserve and
dma_declare_coherent with a single call to CMA's dma_declare_contiguous.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
arch/arm/plat-s5p/dev-mfc.c | 51 ++++++-------------------------------------
1 files changed, 7 insertions(+), 44 deletions(-)
@@ -22,52 +23,14 @@#include<plat/irqs.h>#include<plat/mfc.h>-structs5p_mfc_reserved_mem{-phys_addr_tbase;-unsignedlongsize;-structdevice*dev;-};--staticstructs5p_mfc_reserved_mems5p_mfc_mem[2]__initdata;-void__inits5p_mfc_reserve_mem(phys_addr_trbase,unsignedintrsize,phys_addr_tlbase,unsignedintlsize){-inti;--s5p_mfc_mem[0].dev=&s5p_device_mfc_r.dev;-s5p_mfc_mem[0].base=rbase;-s5p_mfc_mem[0].size=rsize;--s5p_mfc_mem[1].dev=&s5p_device_mfc_l.dev;-s5p_mfc_mem[1].base=lbase;-s5p_mfc_mem[1].size=lsize;--for(i=0;i<ARRAY_SIZE(s5p_mfc_mem);i++){-structs5p_mfc_reserved_mem*area=&s5p_mfc_mem[i];-if(memblock_remove(area->base,area->size)){-printk(KERN_ERR"Failed to reserve memory for MFC device (%ld bytes at 0x%08lx)\n",-area->size,(unsignedlong)area->base);-area->base=0;-}-}-}--staticint__inits5p_mfc_memory_init(void)-{-inti;--for(i=0;i<ARRAY_SIZE(s5p_mfc_mem);i++){-structs5p_mfc_reserved_mem*area=&s5p_mfc_mem[i];-if(!area->base)-continue;+if(dma_declare_contiguous(&s5p_device_mfc_r.dev,rsize,rbase,0))+printk(KERN_ERR"Failed to reserve memory for MFC device (%u bytes at 0x%08lx)\n",+rsize,(unsignedlong)rbase);-if(dma_declare_coherent_memory(area->dev,area->base,-area->base,area->size,-DMA_MEMORY_MAP|DMA_MEMORY_EXCLUSIVE)==0)-printk(KERN_ERR"Failed to declare coherent memory for MFC device (%ld bytes at 0x%08lx)\n",-area->size,(unsignedlong)area->base);-}-return0;+if(dma_declare_contiguous(&s5p_device_mfc_l.dev,lsize,lbase,0))+printk(KERN_ERR"Failed to reserve memory for MFC device (%u bytes at 0x%08lx)\n",+rsize,(unsignedlong)rbase);}-device_initcall(s5p_mfc_memory_init);
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:03:41
This patch extracts common reclaim code from __alloc_pages_direct_reclaim()
function to separate function: __perform_reclaim() which can be later used
by alloc_contig_range().
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Michal Nazarewicz <redacted>
---
mm/page_alloc.c | 30 +++++++++++++++++++++---------
1 files changed, 21 insertions(+), 9 deletions(-)
@@ -2094,16 +2094,13 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,}#endif /* CONFIG_COMPACTION */-/* The really slow allocator path where we enter direct reclaim */-staticinlinestructpage*-__alloc_pages_direct_reclaim(gfp_tgfp_mask,unsignedintorder,-structzonelist*zonelist,enumzone_typehigh_zoneidx,-nodemask_t*nodemask,intalloc_flags,structzone*preferred_zone,-intmigratetype,unsignedlong*did_some_progress)+/* Perform direct synchronous page reclaim */+staticinlineint+__perform_reclaim(gfp_tgfp_mask,unsignedintorder,structzonelist*zonelist,+nodemask_t*nodemask){-structpage*page=NULL;structreclaim_statereclaim_state;-booldrained=false;+intprogress;cond_resched();
@@ -2114,7 +2111,7 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,reclaim_state.reclaimed_slab=0;current->reclaim_state=&reclaim_state;-*did_some_progress=try_to_free_pages(zonelist,order,gfp_mask,nodemask);+progress=try_to_free_pages(zonelist,order,gfp_mask,nodemask);current->reclaim_state=NULL;lockdep_clear_current_reclaim_state();
@@ -2122,6 +2119,21 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,cond_resched();+returnprogress;+}++/* The really slow allocator path where we enter direct reclaim */+staticinlinestructpage*+__alloc_pages_direct_reclaim(gfp_tgfp_mask,unsignedintorder,+structzonelist*zonelist,enumzone_typehigh_zoneidx,+nodemask_t*nodemask,intalloc_flags,structzone*preferred_zone,+intmigratetype,unsignedlong*did_some_progress)+{+structpage*page=NULL;+booldrained=false;++*did_some_progress=__perform_reclaim(gfp_mask,order,zonelist,+nodemask);if(unlikely(!(*did_some_progress)))returnNULL;
On Thu, Jan 26, 2012 at 10:00:52AM +0100, Marek Szyprowski wrote:
quoted hunk
This patch extracts common reclaim code from __alloc_pages_direct_reclaim()
function to separate function: __perform_reclaim() which can be later used
by alloc_contig_range().
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Michal Nazarewicz <redacted>
---
mm/page_alloc.c | 30 +++++++++++++++++++++---------
1 files changed, 21 insertions(+), 9 deletions(-)
@@ -2094,16 +2094,13 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,}#endif /* CONFIG_COMPACTION */-/* The really slow allocator path where we enter direct reclaim */-staticinlinestructpage*-__alloc_pages_direct_reclaim(gfp_tgfp_mask,unsignedintorder,-structzonelist*zonelist,enumzone_typehigh_zoneidx,-nodemask_t*nodemask,intalloc_flags,structzone*preferred_zone,-intmigratetype,unsignedlong*did_some_progress)+/* Perform direct synchronous page reclaim */+staticinlineint+__perform_reclaim(gfp_tgfp_mask,unsignedintorder,structzonelist*zonelist,+nodemask_t*nodemask)
This function is too large to be inlined. Make it a static int. Once
that is fixed add a
Acked-by: Mel Gorman <redacted>
--
Mel Gorman
SUSE Labs
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:04:17
alloc_contig_range() performs memory allocation so it also should keep
track on keeping the correct level of memory watermarks. This commit adds
a call to *_slowpath style reclaim to grab enough pages to make sure that
the final collection of contiguous pages from freelists will not starve
the system.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Michal Nazarewicz <redacted>
---
mm/page_alloc.c | 36 ++++++++++++++++++++++++++++++++++++
1 files changed, 36 insertions(+), 0 deletions(-)
@@ -5613,6 +5613,34 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)returnret;}+/*+*Triggermemorypressurebumptoreclaimsomepagesinordertobeableto+*allocate'count'pagesinsinglepageunits.Doessimilarworkas+*__alloc_pages_slowpath()function.+*/+staticint__reclaim_pages(structzone*zone,gfp_tgfp_mask,intcount)+{+enumzone_typehigh_zoneidx=gfp_zone(gfp_mask);+structzonelist*zonelist=node_zonelist(0,gfp_mask);+intdid_some_progress=0;+intorder=1;+unsignedlongwatermark;++/* Obey watermarks as if the page was being allocated */+watermark=low_wmark_pages(zone)+count;+while(!zone_watermark_ok(zone,0,watermark,0,0)){+wake_all_kswapd(order,zonelist,high_zoneidx,zone_idx(zone));++did_some_progress=__perform_reclaim(gfp_mask,order,zonelist,+NULL);+if(!did_some_progress){+/* Exhausted what can be done so it's blamo time */+out_of_memory(zonelist,gfp_mask,order,NULL);+}+}+returncount;+}+/***alloc_contig_range()--triestoallocategivenrangeofpages*@start:startPFNtoallocate
@@ -5707,6 +5735,14 @@ int alloc_contig_range(unsigned long start, unsigned long end,gotodone;}+/*+*Reclaimenoughpagestomakesurethatcontiguousallocation+*willnotstarvethesystem.+*/+__reclaim_pages(page_zone(pfn_to_page(outer_start)),+GFP_HIGHUSER_MOVABLE,end-start);++/* Grab isolated pages from freelists. */outer_end=isolate_freepages_range(outer_start,end);if(!outer_end){ret=-EBUSY;
On Thu, Jan 26, 2012 at 10:00:53AM +0100, Marek Szyprowski wrote:
quoted hunk
alloc_contig_range() performs memory allocation so it also should keep
track on keeping the correct level of memory watermarks. This commit adds
a call to *_slowpath style reclaim to grab enough pages to make sure that
the final collection of contiguous pages from freelists will not starve
the system.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Michal Nazarewicz <redacted>
---
mm/page_alloc.c | 36 ++++++++++++++++++++++++++++++++++++
1 files changed, 36 insertions(+), 0 deletions(-)
@@ -5613,6 +5613,34 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)returnret;}+/*+*Triggermemorypressurebumptoreclaimsomepagesinordertobeableto+*allocate'count'pagesinsinglepageunits.Doessimilarworkas+*__alloc_pages_slowpath()function.+*/+staticint__reclaim_pages(structzone*zone,gfp_tgfp_mask,intcount)+{+enumzone_typehigh_zoneidx=gfp_zone(gfp_mask);+structzonelist*zonelist=node_zonelist(0,gfp_mask);+intdid_some_progress=0;+intorder=1;+unsignedlongwatermark;++/* Obey watermarks as if the page was being allocated */+watermark=low_wmark_pages(zone)+count;+while(!zone_watermark_ok(zone,0,watermark,0,0)){+wake_all_kswapd(order,zonelist,high_zoneidx,zone_idx(zone));++did_some_progress=__perform_reclaim(gfp_mask,order,zonelist,+NULL);+if(!did_some_progress){+/* Exhausted what can be done so it's blamo time */+out_of_memory(zonelist,gfp_mask,order,NULL);+}
There are three problems here
1. CMA can trigger the OOM killer.
That seems like overkill to me but as I do not know the consequences
of CMA failing, it's your call.
2. You cannot guarantee that try_to_free_pages will free pages from the
zone you care about or that kswapd will do anything
You check the watermarks and take into account the size of the pending
CMA allocation. kswapd in vmscan.c on the other hand will simply check
the watermarks and probably go back to sleep. You should be aware of
this in case you ever get bugs that CMA takes too long and that it
appears to be stuck in this loop with kswapd staying asleep.
3. You reclaim from zones other than your target zone
try_to_free_pages is not necessarily going to free pages in the
zone you are checking for. It'll work on ARM in many cases because
there will be only one zone but on other arches, this logic will
be problematic and will potentially livelock. You need to pass in
a zonelist that only contains the zone that CMA cares about. If it
cannot reclaim, did_some_progress == 0 and it'll exit. Otherwise
there is a possibility that this will loop forever reclaiming pages
from the wrong zones.
I won't ack this particular patch but I am not going to insist that
you fix these prior to merging either. If you leave problem 3 as it
is, I would really like to see a comment explaning the problem for
future users of CMA on other arches (if they exist).
--
Mel Gorman
SUSE Labs
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-31 17:15:12
Hello,
On Monday, January 30, 2012 2:06 PM Mel Gorman wrote:
On Thu, Jan 26, 2012 at 10:00:53AM +0100, Marek Szyprowski wrote:
quoted
alloc_contig_range() performs memory allocation so it also should keep
track on keeping the correct level of memory watermarks. This commit adds
a call to *_slowpath style reclaim to grab enough pages to make sure that
the final collection of contiguous pages from freelists will not starve
the system.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Michal Nazarewicz <redacted>
---
mm/page_alloc.c | 36 ++++++++++++++++++++++++++++++++++++
1 files changed, 36 insertions(+), 0 deletions(-)
@@ -5613,6 +5613,34 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned
long end)
quoted
return ret;
}
+/*
+ * Trigger memory pressure bump to reclaim some pages in order to be able to
+ * allocate 'count' pages in single page units. Does similar work as
+ *__alloc_pages_slowpath() function.
+ */
+static int __reclaim_pages(struct zone *zone, gfp_t gfp_mask, int count)
+{
+ enum zone_type high_zoneidx = gfp_zone(gfp_mask);
+ struct zonelist *zonelist = node_zonelist(0, gfp_mask);
+ int did_some_progress = 0;
+ int order = 1;
+ unsigned long watermark;
+
+ /* Obey watermarks as if the page was being allocated */
+ watermark = low_wmark_pages(zone) + count;
+ while (!zone_watermark_ok(zone, 0, watermark, 0, 0)) {
+ wake_all_kswapd(order, zonelist, high_zoneidx, zone_idx(zone));
+
+ did_some_progress = __perform_reclaim(gfp_mask, order, zonelist,
+ NULL);
+ if (!did_some_progress) {
+ /* Exhausted what can be done so it's blamo time */
+ out_of_memory(zonelist, gfp_mask, order, NULL);
+ }
There are three problems here
1. CMA can trigger the OOM killer.
That seems like overkill to me but as I do not know the consequences
of CMA failing, it's your call.
This behavior is intended, we agreed that the contiguous allocations should
have higher priority than others.
2. You cannot guarantee that try_to_free_pages will free pages from the
zone you care about or that kswapd will do anything
You check the watermarks and take into account the size of the pending
CMA allocation. kswapd in vmscan.c on the other hand will simply check
the watermarks and probably go back to sleep. You should be aware of
this in case you ever get bugs that CMA takes too long and that it
appears to be stuck in this loop with kswapd staying asleep.
Right, I experienced this problem today. The simplest workaround I've
found is to adjust watermark before calling kswapd, but I'm not sure
that increasing min_free_kbytes and calling setup_per_zone_wmarks() is
the nicest approach for it.
3. You reclaim from zones other than your target zone
try_to_free_pages is not necessarily going to free pages in the
zone you are checking for. It'll work on ARM in many cases because
there will be only one zone but on other arches, this logic will
be problematic and will potentially livelock. You need to pass in
a zonelist that only contains the zone that CMA cares about. If it
cannot reclaim, did_some_progress == 0 and it'll exit. Otherwise
there is a possibility that this will loop forever reclaiming pages
from the wrong zones.
Right. I tested it on a system with only one zone, so I never experienced
such problem. For the first version I think we might assume that the buffer
allocated by alloc_contig_range() must fit the single zone. I will add some
comments about it. Later we can extend it for more advanced cases.
I won't ack this particular patch but I am not going to insist that
you fix these prior to merging either. If you leave problem 3 as it
is, I would really like to see a comment explaning the problem for
future users of CMA on other arches (if they exist).
I will add more comments about the issues You have pointed out to make
the life easier for other arch developers.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:04:20
From: Michal Nazarewicz <redacted>
This commit adds a row for MIGRATE_ISOLATE type to the fallbacks array
which was missing from it. It also, changes the array traversal logic
a little making MIGRATE_RESERVE an end marker. The letter change,
removes the implicit MIGRATE_UNMOVABLE from the end of each row which
was read by __rmqueue_fallback() function.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
mm/page_alloc.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
@@ -875,11 +875,12 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,*Thisarraydescribestheorderlistsarefallenbacktowhen*thefreelistsforthedesirablemigratetypearedepleted*/-staticintfallbacks[MIGRATE_TYPES][MIGRATE_TYPES-1]={+staticintfallbacks[MIGRATE_TYPES][3]={[MIGRATE_UNMOVABLE]={MIGRATE_RECLAIMABLE,MIGRATE_MOVABLE,MIGRATE_RESERVE},[MIGRATE_RECLAIMABLE]={MIGRATE_UNMOVABLE,MIGRATE_MOVABLE,MIGRATE_RESERVE},[MIGRATE_MOVABLE]={MIGRATE_RECLAIMABLE,MIGRATE_UNMOVABLE,MIGRATE_RESERVE},-[MIGRATE_RESERVE]={MIGRATE_RESERVE,MIGRATE_RESERVE,MIGRATE_RESERVE},/* Never used */+[MIGRATE_RESERVE]={MIGRATE_RESERVE},/* Never used */+[MIGRATE_ISOLATE]={MIGRATE_RESERVE},/* Never used */};/*
@@ -974,12 +975,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++){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, Jan 26, 2012 at 10:00:49AM +0100, Marek Szyprowski wrote:
From: Michal Nazarewicz <redacted>
This commit adds a row for MIGRATE_ISOLATE type to the fallbacks array
which was missing from it. It also, changes the array traversal logic
a little making MIGRATE_RESERVE an end marker. The letter change,
removes the implicit MIGRATE_UNMOVABLE from the end of each row which
was read by __rmqueue_fallback() function.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:06:34
From: Michal Nazarewicz <redacted>
This commit changes various functions that change pages and
pageblocks migrate type between MIGRATE_ISOLATE and
MIGRATE_MOVABLE in such a way as to allow to work with
MIGRATE_CMA migrate type.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/page-isolation.h | 21 +++++++++++----------
mm/memory-failure.c | 2 +-
mm/memory_hotplug.c | 6 +++---
mm/page_alloc.c | 18 ++++++++++++------
mm/page_isolation.c | 15 ++++++++-------
5 files changed, 35 insertions(+), 27 deletions(-)
@@ -33,16 +34,16 @@ test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn);externvoidupdate_pcp_isolate_block(unsignedlongpfn);/*-*Internalfuncs.Changespageblock'smigratetype.-*Pleaseusemake_pagetype_isolated()/make_pagetype_movable().+*Internalfunctions.Changespageblock'smigratetype.*/externintset_migratetype_isolate(structpage*page);-externvoidunset_migratetype_isolate(structpage*page);+externvoidunset_migratetype_isolate(structpage*page,unsignedmigratetype);#ifdef CONFIG_CMA/* The below functions must be run on a range from a single zone. */-externintalloc_contig_range(unsignedlongstart,unsignedlongend);+externintalloc_contig_range(unsignedlongstart,unsignedlongend,+unsignedmigratetype);externvoidfree_contig_range(unsignedlongpfn,unsignednr_pages);/* CMA stuff */
@@ -1400,7 +1400,7 @@ static int get_any_page(struct page *p, unsigned long pfn, int flags)/* Not a free page */ret=1;}-unset_migratetype_isolate(p);+unset_migratetype_isolate(p,MIGRATE_MOVABLE);unlock_memory_hotplug();returnret;}
@@ -891,7 +891,7 @@ static int __ref offline_pages(unsigned long start_pfn,nr_pages=end_pfn-start_pfn;/* set above range as isolated */-ret=start_isolate_page_range(start_pfn,end_pfn);+ret=start_isolate_page_range(start_pfn,end_pfn,MIGRATE_MOVABLE);if(ret)gotoout;
@@ -956,7 +956,7 @@ repeat:Wecannotdorollbackatthispoint.*/offline_isolated_pages(start_pfn,end_pfn);/* reset pagetype flags and makes migrate type to be MOVABLE */-undo_isolate_page_range(start_pfn,end_pfn);+undo_isolate_page_range(start_pfn,end_pfn,MIGRATE_MOVABLE);/* removal success */zone->present_pages-=offlined_pages;zone->zone_pgdat->node_present_pages-=offlined_pages;
@@ -981,7 +981,7 @@ failed_removal:start_pfn,end_pfn);memory_notify(MEM_CANCEL_OFFLINE,&arg);/* pushback to free area */-undo_isolate_page_range(start_pfn,end_pfn);+undo_isolate_page_range(start_pfn,end_pfn,MIGRATE_MOVABLE);out:unlock_memory_hotplug();
@@ -5605,6 +5605,10 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)*alloc_contig_range()--triestoallocategivenrangeofpages*@start:startPFNtoallocate*@end:one-past-the-lastPFNtoallocate+*@migratetype:migratetypeoftheunderlayingpageblocks(either+*#MIGRATE_MOVABLEor#MIGRATE_CMA).Allpageblocks+*inrangemusthavethesamemigratetypeanditmust+*beeitherofthetwo.**ThePFNrangedoesnothavetobepageblockorMAX_ORDER_NR_PAGES*aligned,howeverit'sthecaller'sresponsibilitytoguaranteethat
@@ -5617,7 +5621,8 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)*pageswhichPFNisin[start,end)areallocatedforthecallerand*needtobefreedwithfree_contig_range().*/-intalloc_contig_range(unsignedlongstart,unsignedlongend)+intalloc_contig_range(unsignedlongstart,unsignedlongend,+unsignedmigratetype){unsignedlongouter_start,outer_end;intret=0,order;
@@ -5646,7 +5651,8 @@ int alloc_contig_range(unsigned long start, unsigned long end)*/ret=start_isolate_page_range(pfn_align_to_maxpage_down(start),-pfn_align_to_maxpage_up(end));+pfn_align_to_maxpage_up(end),+migratetype);if(ret)gotodone;
@@ -5703,7 +5709,7 @@ int alloc_contig_range(unsigned long start, unsigned long end)done:undo_isolate_page_range(pfn_align_to_maxpage_down(start),-pfn_align_to_maxpage_up(end));+pfn_align_to_maxpage_up(end),migratetype);returnret;}
@@ -24,6 +24,7 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)*tobeMIGRATE_ISOLATE.*@start_pfn:ThelowerPFNoftherangetobeisolated.*@end_pfn:TheupperPFNoftherangetobeisolated.+*@migratetype:migratetypetosetinerrorrecovery.**Makingpage-allocation-typetobeMIGRATE_ISOLATEmeansfreepagesin*therangewillneverbeallocated.Anyfreepagesandpagesfreedinthe
@@ -32,8 +33,8 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)*start_pfn/end_pfnmustbealignedtopageblock_order.*Returns0onsuccessand-EBUSYifanypartofrangecannotbeisolated.*/-int-start_isolate_page_range(unsignedlongstart_pfn,unsignedlongend_pfn)+intstart_isolate_page_range(unsignedlongstart_pfn,unsignedlongend_pfn,+unsignedmigratetype){unsignedlongpfn;unsignedlongundo_pfn;
@@ -77,7 +78,7 @@ undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn)page=__first_valid_page(pfn,pageblock_nr_pages);if(!page||get_pageblock_migratetype(page)!=MIGRATE_ISOLATE)continue;-unset_migratetype_isolate(page);+unset_migratetype_isolate(page,migratetype);}return0;}
@@ -86,7 +87,7 @@ undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn)*allpagesin[start_pfn...end_pfn)mustbeinthesamezone.*zone->lockmustbeheldbeforecallthis.*-*Returns1ifallpagesintherangeisisolated.+*Returns1ifallpagesintherangeareisolated.*/staticint__test_page_isolated_in_pageblock(unsignedlongpfn,unsignedlongend_pfn)
On Thu, Jan 26, 2012 at 10:00:43AM +0100, Marek Szyprowski wrote:
From: Michal Nazarewicz <redacted>
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Ordinarily, I do not like these sort of patches because they can
interfere with git blame but as it is comments that are affected;
Acked-by: Mel Gorman <redacted>
Thanks
--
Mel Gorman
SUSE Labs
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:06:39
From: Michal Nazarewicz <redacted>
This commit exports some of the functions from compaction.c file
outside of it adding their declaration into internal.h header
file so that other mm related code can use them.
This forced compaction.c to always be compiled (as opposed to being
compiled only if CONFIG_COMPACTION is defined) but as to avoid
introducing code that user did not ask for, part of the compaction.c
is now wrapped in on #ifdef.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
mm/Makefile | 3 +-
mm/compaction.c | 314 ++++++++++++++++++++++++++-----------------------------
mm/internal.h | 33 ++++++
3 files changed, 184 insertions(+), 166 deletions(-)
@@ -16,30 +16,11 @@#include<linux/sysfs.h>#include"internal.h"+#if defined CONFIG_COMPACTION || defined CONFIG_CMA+#define CREATE_TRACE_POINTS#include<trace/events/compaction.h>-/*-*compact_controlisusedtotrackpagesbeingmigratedandthefreepages-*theyarebeingmigratedtoduringmemorycompaction.Thefree_pfnstarts-*attheendofazoneandmigrate_pfnbeginsatthestart.Movablepages-*aremovedtotheendofazoneduringacompactionrunandtherun-*completeswhenfree_pfn<=migrate_pfn-*/-structcompact_control{-structlist_headfreepages;/* List of free pages to migrate to */-structlist_headmigratepages;/* List of pages being migrated */-unsignedlongnr_freepages;/* Number of isolated free pages */-unsignedlongnr_migratepages;/* Number of pages to migrate */-unsignedlongfree_pfn;/* isolate_freepages search base */-unsignedlongmigrate_pfn;/* isolate_migratepages search base */-boolsync;/* Synchronous migration */--unsignedintorder;/* order a direct compactor needs */-intmigratetype;/* MOVABLE, RECLAIMABLE etc */-structzone*zone;-};-staticunsignedlongrelease_freepages(structlist_head*freelist){structpage*page,*next;
@@ -122,7 +103,7 @@ static unsigned long isolate_freepages_block(unsigned long blockpfn,*(whichmaybegreaterthenend_pfnifendfellinamiddleof*afreepage).*/-staticunsignedlong+unsignedlongisolate_freepages_range(unsignedlongstart_pfn,unsignedlongend_pfn){unsignedlongisolated,pfn,block_end_pfn,flags;
@@ -183,120 +164,6 @@ isolate_freepages_range(unsigned long start_pfn, unsigned long end_pfn)returnpfn;}-/* Returns true if the page is within a block suitable for migration to */-staticboolsuitable_migration_target(structpage*page)-{--intmigratetype=get_pageblock_migratetype(page);--/* Don't interfere with memory hot-remove or the min_free_kbytes blocks */-if(migratetype==MIGRATE_ISOLATE||migratetype==MIGRATE_RESERVE)-returnfalse;--/* If the page is a large free page, then allow migration */-if(PageBuddy(page)&&page_order(page)>=pageblock_order)-returntrue;--/* If the block is MIGRATE_MOVABLE, allow migration */-if(migratetype==MIGRATE_MOVABLE)-returntrue;--/* Otherwise skip the block */-returnfalse;-}--/*-*Basedoninformationinthecurrentcompact_control,findblocks-*suitableforisolatingfreepagesfromandthenisolatethem.-*/-staticvoidisolate_freepages(structzone*zone,-structcompact_control*cc)-{-structpage*page;-unsignedlonghigh_pfn,low_pfn,pfn,zone_end_pfn,end_pfn;-unsignedlongflags;-intnr_freepages=cc->nr_freepages;-structlist_head*freelist=&cc->freepages;--/*-*Initialisethefreescanner.Thestartingpointiswherewelast-*scannedfrom(ortheendofthezoneifstarting).Thelowpoint-*istheendofthepageblockthemigrationscannerisusing.-*/-pfn=cc->free_pfn;-low_pfn=cc->migrate_pfn+pageblock_nr_pages;--/*-*Takecarethatifthemigrationscannerisattheendofthezone-*thatthefreescannerdoesnotaccidentallymovetothenextzone-*inthenextisolationcycle.-*/-high_pfn=min(low_pfn,pfn);--zone_end_pfn=zone->zone_start_pfn+zone->spanned_pages;--/*-*Isolatefreepagesuntilenoughareavailabletomigratethe-*pagesoncc->migratepages.Westopsearchingifthemigrate-*andfreepagescannersmeetorenoughfreepagesareisolated.-*/-for(;pfn>low_pfn&&cc->nr_migratepages>nr_freepages;-pfn-=pageblock_nr_pages){-unsignedlongisolated;--if(!pfn_valid(pfn))-continue;--/*-*Checkforoverlappingnodes/zones.It'spossibleonsome-*configurationstohaveasetuplike-*node0node1node0-*i.e.it'spossiblethatallpageswithinazonesrangeof-*pagesdonotbelongtoasinglezone.-*/-page=pfn_to_page(pfn);-if(page_zone(page)!=zone)-continue;--/* Check the block is suitable for migration */-if(!suitable_migration_target(page))-continue;--/*-*Foundablocksuitableforisolatingfreepagesfrom.Now-*wedisabledinterrupts,doublecheckthingsareokand-*isolatethepages.ThisistominimisethetimeIRQs-*aredisabled-*/-isolated=0;-spin_lock_irqsave(&zone->lock,flags);-if(suitable_migration_target(page)){-end_pfn=min(pfn+pageblock_nr_pages,zone_end_pfn);-isolated=isolate_freepages_block(pfn,end_pfn,-freelist,false);-nr_freepages+=isolated;-}-spin_unlock_irqrestore(&zone->lock,flags);--/*-*RecordthehighestPFNweisolatedpagesfrom.Whennext-*lookingforfreepages,thesearchwillrestarthereas-*pagemigrationmayhavereturnedsomepagestotheallocator-*/-if(isolated)-high_pfn=max(high_pfn,pfn);-}--/* split_free_page does not map the pages */-list_for_each_entry(page,freelist,lru){-arch_alloc_page(page,0);-kernel_map_pages(page,1,1);-}--cc->free_pfn=high_pfn;-cc->nr_freepages=nr_freepages;-}-/* Update the number of anon and file isolated pages in the zone */staticvoidacct_isolated(structzone*zone,structcompact_control*cc){
@@ -325,13 +192,6 @@ static bool too_many_isolated(struct zone *zone)returnisolated>(inactive+active)/2;}-/* possible outcome of isolate_migratepages */-typedefenum{-ISOLATE_ABORT,/* Abort compaction now */-ISOLATE_NONE,/* No pages isolated, continue scanning */-ISOLATE_SUCCESS,/* Pages isolated, migrate */-}isolate_migrate_t;-/***isolate_migratepages_range()-isolateallmigrate-ablepagesinrange.*@zone:Zonepagesarein.
@@ -465,35 +325,121 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,returnlow_pfn;}+#endif /* CONFIG_COMPACTION || CONFIG_CMA */+#ifdef CONFIG_COMPACTION++/* Returns true if the page is within a block suitable for migration to */+staticboolsuitable_migration_target(structpage*page)+{++intmigratetype=get_pageblock_migratetype(page);++/* Don't interfere with memory hot-remove or the min_free_kbytes blocks */+if(migratetype==MIGRATE_ISOLATE||migratetype==MIGRATE_RESERVE)+returnfalse;++/* If the page is a large free page, then allow migration */+if(PageBuddy(page)&&page_order(page)>=pageblock_order)+returntrue;++/* If the block is MIGRATE_MOVABLE, allow migration */+if(migratetype==MIGRATE_MOVABLE)+returntrue;++/* Otherwise skip the block */+returnfalse;+}+/*-*Isolateallpagesthatcanbemigratedfromtheblockpointedtoby-*themigratescannerwithincompact_control.+*Basedoninformationinthecurrentcompact_control,findblocks+*suitableforisolatingfreepagesfromandthenisolatethem.*/-staticisolate_migrate_tisolate_migratepages(structzone*zone,-structcompact_control*cc)+staticvoidisolate_freepages(structzone*zone,+structcompact_control*cc){-unsignedlonglow_pfn,end_pfn;+structpage*page;+unsignedlonghigh_pfn,low_pfn,pfn,zone_end_pfn,end_pfn;+unsignedlongflags;+intnr_freepages=cc->nr_freepages;+structlist_head*freelist=&cc->freepages;-/* Do not scan outside zone boundaries */-low_pfn=max(cc->migrate_pfn,zone->zone_start_pfn);+/*+*Initialisethefreescanner.Thestartingpointiswherewelast+*scannedfrom(ortheendofthezoneifstarting).Thelowpoint+*istheendofthepageblockthemigrationscannerisusing.+*/+pfn=cc->free_pfn;+low_pfn=cc->migrate_pfn+pageblock_nr_pages;-/* Only scan within a pageblock boundary */-end_pfn=ALIGN(low_pfn+pageblock_nr_pages,pageblock_nr_pages);+/*+*Takecarethatifthemigrationscannerisattheendofthezone+*thatthefreescannerdoesnotaccidentallymovetothenextzone+*inthenextisolationcycle.+*/+high_pfn=min(low_pfn,pfn);-/* Do not cross the free scanner or scan within a memory hole */-if(end_pfn>cc->free_pfn||!pfn_valid(low_pfn)){-cc->migrate_pfn=end_pfn;-returnISOLATE_NONE;-}+zone_end_pfn=zone->zone_start_pfn+zone->spanned_pages;-/* Perform the isolation */-low_pfn=isolate_migratepages_range(zone,cc,low_pfn,end_pfn);-if(!low_pfn)-returnISOLATE_ABORT;+/*+*Isolatefreepagesuntilenoughareavailabletomigratethe+*pagesoncc->migratepages.Westopsearchingifthemigrate+*andfreepagescannersmeetorenoughfreepagesareisolated.+*/+for(;pfn>low_pfn&&cc->nr_migratepages>nr_freepages;+pfn-=pageblock_nr_pages){+unsignedlongisolated;-cc->migrate_pfn=low_pfn;+if(!pfn_valid(pfn))+continue;-returnISOLATE_SUCCESS;+/*+*Checkforoverlappingnodes/zones.It'spossibleonsome+*configurationstohaveasetuplike+*node0node1node0+*i.e.it'spossiblethatallpageswithinazonesrangeof+*pagesdonotbelongtoasinglezone.+*/+page=pfn_to_page(pfn);+if(page_zone(page)!=zone)+continue;++/* Check the block is suitable for migration */+if(!suitable_migration_target(page))+continue;++/*+*Foundablocksuitableforisolatingfreepagesfrom.Now+*wedisabledinterrupts,doublecheckthingsareokand+*isolatethepages.ThisistominimisethetimeIRQs+*aredisabled+*/+isolated=0;+spin_lock_irqsave(&zone->lock,flags);+if(suitable_migration_target(page)){+end_pfn=min(pfn+pageblock_nr_pages,zone_end_pfn);+isolated=isolate_freepages_block(pfn,end_pfn,+freelist,false);+nr_freepages+=isolated;+}+spin_unlock_irqrestore(&zone->lock,flags);++/*+*RecordthehighestPFNweisolatedpagesfrom.Whennext+*lookingforfreepages,thesearchwillrestarthereas+*pagemigrationmayhavereturnedsomepagestotheallocator+*/+if(isolated)+high_pfn=max(high_pfn,pfn);+}++/* split_free_page does not map the pages */+list_for_each_entry(page,freelist,lru){+arch_alloc_page(page,0);+kernel_map_pages(page,1,1);+}++cc->free_pfn=high_pfn;+cc->nr_freepages=nr_freepages;}/*
@@ -542,6 +488,44 @@ static void update_nr_listpages(struct compact_control *cc)cc->nr_freepages=nr_freepages;}+/* possible outcome of isolate_migratepages */+typedefenum{+ISOLATE_ABORT,/* Abort compaction now */+ISOLATE_NONE,/* No pages isolated, continue scanning */+ISOLATE_SUCCESS,/* Pages isolated, migrate */+}isolate_migrate_t;++/*+*Isolateallpagesthatcanbemigratedfromtheblockpointedtoby+*themigratescannerwithincompact_control.+*/+staticisolate_migrate_tisolate_migratepages(structzone*zone,+structcompact_control*cc)+{+unsignedlonglow_pfn,end_pfn;++/* Do not scan outside zone boundaries */+low_pfn=max(cc->migrate_pfn,zone->zone_start_pfn);++/* Only scan within a pageblock boundary */+end_pfn=ALIGN(low_pfn+pageblock_nr_pages,pageblock_nr_pages);++/* Do not cross the free scanner or scan within a memory hole */+if(end_pfn>cc->free_pfn||!pfn_valid(low_pfn)){+cc->migrate_pfn=end_pfn;+returnISOLATE_NONE;+}++/* Perform the isolation */+low_pfn=isolate_migratepages_range(zone,cc,low_pfn,end_pfn);+if(!low_pfn)+returnISOLATE_ABORT;++cc->migrate_pfn=low_pfn;++returnISOLATE_SUCCESS;+}+staticintcompact_finished(structzone*zone,structcompact_control*cc){
@@ -100,6 +100,39 @@ extern void prep_compound_page(struct page *page, unsigned long order);externboolis_free_buddy_page(structpage*page);#endif+#if defined CONFIG_COMPACTION || defined CONFIG_CMA++/*+*inmm/compaction.c+*/+/*+*compact_controlisusedtotrackpagesbeingmigratedandthefreepages+*theyarebeingmigratedtoduringmemorycompaction.Thefree_pfnstarts+*attheendofazoneandmigrate_pfnbeginsatthestart.Movablepages+*aremovedtotheendofazoneduringacompactionrunandtherun+*completeswhenfree_pfn<=migrate_pfn+*/+structcompact_control{+structlist_headfreepages;/* List of free pages to migrate to */+structlist_headmigratepages;/* List of pages being migrated */+unsignedlongnr_freepages;/* Number of isolated free pages */+unsignedlongnr_migratepages;/* Number of pages to migrate */+unsignedlongfree_pfn;/* isolate_freepages search base */+unsignedlongmigrate_pfn;/* isolate_migratepages search base */+boolsync;/* Synchronous migration */++unsignedintorder;/* order a direct compactor needs */+intmigratetype;/* MOVABLE, RECLAIMABLE etc */+structzone*zone;+};++unsignedlong+isolate_freepages_range(unsignedlongstart_pfn,unsignedlongend_pfn);+unsignedlong+isolate_migratepages_range(structzone*zone,structcompact_control*cc,+unsignedlonglow_pfn,unsignedlongend_pfn);++#endif/**functionfordealingwithpage'sorderinbuddysystem.
On Thu, Jan 26, 2012 at 10:00:47AM +0100, Marek Szyprowski wrote:
quoted hunk
From: Michal Nazarewicz <redacted>
This commit exports some of the functions from compaction.c file
outside of it adding their declaration into internal.h header
file so that other mm related code can use them.
This forced compaction.c to always be compiled (as opposed to being
compiled only if CONFIG_COMPACTION is defined) but as to avoid
introducing code that user did not ask for, part of the compaction.c
is now wrapped in on #ifdef.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
mm/Makefile | 3 +-
mm/compaction.c | 314 ++++++++++++++++++++++++++-----------------------------
mm/internal.h | 33 ++++++
3 files changed, 184 insertions(+), 166 deletions(-)
@@ -16,30 +16,11 @@#include<linux/sysfs.h>#include"internal.h"+#if defined CONFIG_COMPACTION || defined CONFIG_CMA+
This is pedantic but you reference CONFIG_CMA before the patch that
declares it. The only time this really matters is when it breaks
bisection but I do not think that is the case here.
Whether you fix this or not by moving the CONFIG_CMA check to the same
patch that declares it in Kconfig
Acked-by: Mel Gorman <redacted>
--
Mel Gorman
SUSE Labs
From: Michal Nazarewicz <hidden> Date: 2012-01-30 12:33:12
On Thu, Jan 26, 2012 at 10:00:47AM +0100, Marek Szyprowski wrote:
quoted
From: Michal Nazarewicz <redacted>
--- a/mm/compaction.c+++ b/mm/compaction.c
@@ -16,30 +16,11 @@#include<linux/sysfs.h>#include"internal.h"+#if defined CONFIG_COMPACTION || defined CONFIG_CMA+
On Mon, 30 Jan 2012 12:57:26 +0100, Mel Gorman [off-list ref] wrote:
This is pedantic but you reference CONFIG_CMA before the patch that
declares it. The only time this really matters is when it breaks
bisection but I do not think that is the case here.
I think I'll choose to be lazy on this one. ;) I actually tried to move
some commits around to resolve this future-reference, but this resulted
in quite a few conflicts during rebase and after several minutes I decided
that it's not worth the effort.
Whether you fix this or not by moving the CONFIG_CMA check to the same
patch that declares it in Kconfig
Acked-by: Mel Gorman <redacted>
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Micha? ?mina86? Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:09:17
From: Michal Nazarewicz <redacted>
This commit changes set_migratetype_isolate() so that it updates
migrate type of pages on pcp list which is saved in their
page_private.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/page-isolation.h | 6 ++++++
mm/page_alloc.c | 1 +
mm/page_isolation.c | 24 ++++++++++++++++++++++++
3 files changed, 31 insertions(+), 0 deletions(-)
@@ -139,3 +139,27 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn)spin_unlock_irqrestore(&zone->lock,flags);returnret?0:-EBUSY;}++/* must hold zone->lock */+voidupdate_pcp_isolate_block(unsignedlongpfn)+{+unsignedlongend_pfn=pfn+pageblock_nr_pages;+structpage*page;++while(pfn<end_pfn){+if(!pfn_valid_within(pfn)){+++pfn;+continue;+}++page=pfn_to_page(pfn);+if(PageBuddy(page)){+pfn+=1<<page_order(page);+}elseif(page_count(page)==0){+set_page_private(page,MIGRATE_ISOLATE);+++pfn;+}else{+++pfn;+}+}+}
On Thu, Jan 26, 2012 at 10:00:44AM +0100, Marek Szyprowski wrote:
quoted hunk
From: Michal Nazarewicz <redacted>
This commit changes set_migratetype_isolate() so that it updates
migrate type of pages on pcp list which is saved in their
page_private.
Signed-off-by: Michal Nazarewicz <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/page-isolation.h | 6 ++++++
mm/page_alloc.c | 1 +
mm/page_isolation.c | 24 ++++++++++++++++++++++++
3 files changed, 31 insertions(+), 0 deletions(-)
@@ -139,3 +139,27 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn)spin_unlock_irqrestore(&zone->lock,flags);returnret?0:-EBUSY;}++/* must hold zone->lock */+voidupdate_pcp_isolate_block(unsignedlongpfn)+{+unsignedlongend_pfn=pfn+pageblock_nr_pages;+structpage*page;++while(pfn<end_pfn){+if(!pfn_valid_within(pfn)){+++pfn;+continue;+}+
There is a potential problem here that you need to be aware of.
set_pageblock_migratetype() is called from start_isolate_page_range().
I do not think there is a guarantee that pfn + pageblock_nr_pages is
not in a different block of MAX_ORDER_NR_PAGES. If that is right then
your options are to add a check like this;
if ((pfn & (MAX_ORDER_NR_PAGES - 1)) == 0 && !pfn_valid(pfn))
break;
or else ensure that end_pfn is always MAX_ORDER_NR_PAGES aligned and in
the same block as pfn and relying on the caller to have called
pfn_valid.
This is dangerous for two reasons. If the page_count is 0, it could
be because the page is in the process of being freed and is not
necessarily on the per-cpu lists yet and you cannot be sure if the
contents of page->private are important. Second, there is nothing to
prevent another CPU allocating this page from its per-cpu list while
the private field is getting updated from here which might lead to
some interesting races.
I recognise that what you are trying to do is respond to Gilad's
request that you really check if an IPI here is necessary. I think what
you need to do is check if a page with a count of 0 is encountered
and if it is, then a draining of the per-cpu lists is necessary. To
address Gilad's concerns, be sure to only this this once per attempt at
CMA rather than for every page encountered with a count of 0 to avoid a
storm of IPIs.
From: Michal Nazarewicz <hidden> Date: 2012-01-30 15:41:28
On Mon, 30 Jan 2012 12:15:22 +0100, Mel Gorman [off-list ref] wrote:
On Thu, Jan 26, 2012 at 10:00:44AM +0100, Marek Szyprowski wrote:
quoted
From: Michal Nazarewicz <redacted>
@@ -139,3 +139,27 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn) spin_unlock_irqrestore(&zone->lock, flags); return ret ? 0 : -EBUSY; }++/* must hold zone->lock */+void update_pcp_isolate_block(unsigned long pfn)+{+ unsigned long end_pfn = pfn + pageblock_nr_pages;+ struct page *page;++ while (pfn < end_pfn) {+ if (!pfn_valid_within(pfn)) {+ ++pfn;+ continue;+ }+
On Mon, 30 Jan 2012 12:15:22 +0100, Mel Gorman [off-list ref] wrote:
There is a potential problem here that you need to be aware of.
set_pageblock_migratetype() is called from start_isolate_page_range().
I do not think there is a guarantee that pfn + pageblock_nr_pages is
not in a different block of MAX_ORDER_NR_PAGES. If that is right then
your options are to add a check like this;
if ((pfn & (MAX_ORDER_NR_PAGES - 1)) == 0 && !pfn_valid(pfn))
break;
or else ensure that end_pfn is always MAX_ORDER_NR_PAGES aligned and in
the same block as pfn and relying on the caller to have called
pfn_valid.
pfn = round_down(pfn, pageblock_nr_pages);
end_pfn = pfn + pageblock_nr_pages;
should do the trick as well, right? move_freepages_block() seem to be
doing the same thing.
This is dangerous for two reasons. If the page_count is 0, it could
be because the page is in the process of being freed and is not
necessarily on the per-cpu lists yet and you cannot be sure if the
contents of page->private are important. Second, there is nothing to
prevent another CPU allocating this page from its per-cpu list while
the private field is getting updated from here which might lead to
some interesting races.
I recognise that what you are trying to do is respond to Gilad's
request that you really check if an IPI here is necessary. I think what
you need to do is check if a page with a count of 0 is encountered
and if it is, then a draining of the per-cpu lists is necessary. To
address Gilad's concerns, be sure to only this this once per attempt at
CMA rather than for every page encountered with a count of 0 to avoid a
storm of IPIs.
It's actually more then that.
This is the same issue that I first fixed with a change to free_pcppages_bulk()
function[1]. At the time of positing, you said you'd like me to try and find
a different solution which would not involve paying the price of calling
get_pageblock_migratetype(). Later I also realised that this solution is
not enough.
[1] http://article.gmane.org/gmane.linux.kernel.mm/70314
My next attempt was to run drain PCP list while holding zone->lock[2], but that
quickly proven to be broken approach when Marek started testing it on an SMP
system.
[2] http://article.gmane.org/gmane.linux.kernel.mm/72016
This patch is yet another attempt of solving this old issue. Even though it has
a potential race condition we came to conclusion that the actual chances of
causing any problems are slim. Various stress tests did not, in fact, show
the race to be an issue.
The problem is that if a page is on a PCP list, and it's underlaying pageblocks'
migrate type is changed to MIGRATE_ISOLATE, the page (i) will still remain on PCP
list and thus someone can allocate it, and (ii) when removed from PCP list, the
page will be put on freelist of migrate type it had prior to change.
(i) is actually not such a big issue since the next thing that happens after
isolation is migration so all the pages will get freed. (ii) is actual problem
and if [1] is not an acceptable solution I really don't have a good fix for that.
One things that comes to mind is calling drain_all_pages() prior to acquiring
zone->lock in set_migratetype_isolate(). This is however prone to races since
after the drain and before the zone->lock is acquired, pages might get moved
back to PCP list.
Draining PCP list after acquiring zone->lock is not possible because
smp_call_function_many() cannot be called with interrupts disabled, and changing
spin_lock_irqsave() to spin_lock() followed by local_irq_save() causes a dead
lock (that's what [2] attempted to do).
Any suggestions are welcome!
quoted
+ } else {
+ ++pfn;
+ }
+ }
+}
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Micha? ?mina86? Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
On Mon, Jan 30, 2012 at 04:41:22PM +0100, Michal Nazarewicz wrote:
On Mon, 30 Jan 2012 12:15:22 +0100, Mel Gorman [off-list ref] wrote:
quoted
On Thu, Jan 26, 2012 at 10:00:44AM +0100, Marek Szyprowski wrote:
quoted
From: Michal Nazarewicz <redacted>
@@ -139,3 +139,27 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn)
spin_unlock_irqrestore(&zone->lock, flags);
return ret ? 0 : -EBUSY;
}
+
+/* must hold zone->lock */
+void update_pcp_isolate_block(unsigned long pfn)
+{
+ unsigned long end_pfn = pfn + pageblock_nr_pages;
+ struct page *page;
+
+ while (pfn < end_pfn) {
+ if (!pfn_valid_within(pfn)) {
+ ++pfn;
+ continue;
+ }
+
On Mon, 30 Jan 2012 12:15:22 +0100, Mel Gorman [off-list ref] wrote:
quoted
There is a potential problem here that you need to be aware of.
set_pageblock_migratetype() is called from start_isolate_page_range().
I do not think there is a guarantee that pfn + pageblock_nr_pages is
not in a different block of MAX_ORDER_NR_PAGES. If that is right then
your options are to add a check like this;
if ((pfn & (MAX_ORDER_NR_PAGES - 1)) == 0 && !pfn_valid(pfn))
break;
or else ensure that end_pfn is always MAX_ORDER_NR_PAGES aligned and in
the same block as pfn and relying on the caller to have called
pfn_valid.
pfn = round_down(pfn, pageblock_nr_pages);
end_pfn = pfn + pageblock_nr_pages;
should do the trick as well, right? move_freepages_block() seem to be
doing the same thing.
This is dangerous for two reasons. If the page_count is 0, it could
be because the page is in the process of being freed and is not
necessarily on the per-cpu lists yet and you cannot be sure if the
contents of page->private are important. Second, there is nothing to
prevent another CPU allocating this page from its per-cpu list while
the private field is getting updated from here which might lead to
some interesting races.
I recognise that what you are trying to do is respond to Gilad's
request that you really check if an IPI here is necessary. I think what
you need to do is check if a page with a count of 0 is encountered
and if it is, then a draining of the per-cpu lists is necessary. To
address Gilad's concerns, be sure to only this this once per attempt at
CMA rather than for every page encountered with a count of 0 to avoid a
storm of IPIs.
It's actually more then that.
This is the same issue that I first fixed with a change to free_pcppages_bulk()
function[1]. At the time of positing, you said you'd like me to try and find
a different solution which would not involve paying the price of calling
get_pageblock_migratetype(). Later I also realised that this solution is
not enough.
[1] http://article.gmane.org/gmane.linux.kernel.mm/70314
Yes. I had forgotten the history but looking at that patch again,
I would reach the conclusion that this was adding a new call to
get_pageblock_migratetype() in the bulk free path. That would affect
everybody whether they were using CMA or not.
My next attempt was to run drain PCP list while holding zone->lock[2], but that
quickly proven to be broken approach when Marek started testing it on an SMP
system.
[2] http://article.gmane.org/gmane.linux.kernel.mm/72016
This patch is yet another attempt of solving this old issue. Even though it has
a potential race condition we came to conclusion that the actual chances of
causing any problems are slim. Various stress tests did not, in fact, show
the race to be an issue.
It is a really small race. To cause a problem CPU 1 must find a page
with count 0, CPU 2 must then allocate the page and set page->private
before CPU 1 overwrites that value but it's there.
The problem is that if a page is on a PCP list, and it's underlaying pageblocks'
migrate type is changed to MIGRATE_ISOLATE, the page (i) will still remain on PCP
list and thus someone can allocate it, and (ii) when removed from PCP list, the
page will be put on freelist of migrate type it had prior to change.
(i) is actually not such a big issue since the next thing that happens after
isolation is migration so all the pages will get freed. (ii) is actual problem
and if [1] is not an acceptable solution I really don't have a good fix for that.
One things that comes to mind is calling drain_all_pages() prior to acquiring
zone->lock in set_migratetype_isolate(). This is however prone to races since
after the drain and before the zone->lock is acquired, pages might get moved
back to PCP list.
Draining PCP list after acquiring zone->lock is not possible because
smp_call_function_many() cannot be called with interrupts disabled, and changing
spin_lock_irqsave() to spin_lock() followed by local_irq_save() causes a dead
lock (that's what [2] attempted to do).
Any suggestions are welcome!
[1] is still not preferred as I'd still like to keep the impact
of CMA to the normal paths to be as close to 0 as possible. In
update_pcp_isolate_block() how about something like this?
if (page_count(page) == 0) {
spin_unlock_irqrestore(zone->lock, flags);
drain_all_pages()
spin_lock_irqsave(zone->lock, flags);
if (PageBuddy(page)) {
order = page_order(page);
list_del(&page->lru);
list_add_tail(&page->lru, &zone->free_area[order].free_list[MIGRATE_ISOLATE]);
set_page_private(page, MIGRATE_ISOLATE);
}
}
If the page is !PageBuddy, it does not matter as alloc_contig_range()
is just about to migrate it.
quoted
quoted
+ } else {
+ ++pfn;
+ }
+ }
+}
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Micha?? ???mina86??? Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
This is dangerous for two reasons. If the page_count is 0, it could
be because the page is in the process of being freed and is not
necessarily on the per-cpu lists yet and you cannot be sure if the
contents of page->private are important. Second, there is nothing to
prevent another CPU allocating this page from its per-cpu list while
the private field is getting updated from here which might lead to
some interesting races.
I recognise that what you are trying to do is respond to Gilad's
request that you really check if an IPI here is necessary. I think what
you need to do is check if a page with a count of 0 is encountered
and if it is, then a draining of the per-cpu lists is necessary. To
address Gilad's concerns, be sure to only this this once per attempt at
CMA rather than for every page encountered with a count of 0 to avoid a
storm of IPIs.
It's actually more then that.
This is the same issue that I first fixed with a change to free_pcppages_bulk()
function[1]. At the time of positing, you said you'd like me to try and find
a different solution which would not involve paying the price of calling
get_pageblock_migratetype(). Later I also realised that this solution is
not enough.
[1] http://article.gmane.org/gmane.linux.kernel.mm/70314
Yes. I had forgotten the history but looking at that patch again,
I would reach the conclusion that this was adding a new call to
get_pageblock_migratetype() in the bulk free path. That would affect
everybody whether they were using CMA or not.
This will be a bit ugly, but we can also use that code and compile it conditionally
when CMA has been enabled. Pages, which have incorrect migrate type on free finally
causes pageblock migration type change from MIGRATE_CMA to MIGRATE_MOVABLE. This is
not a problem for non-CMA case where only pageblocks with MIGRATE_MOVABLE migration
type are being isolated.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
This is dangerous for two reasons. If the page_count is 0, it could
be because the page is in the process of being freed and is not
necessarily on the per-cpu lists yet and you cannot be sure if the
contents of page->private are important. Second, there is nothing to
prevent another CPU allocating this page from its per-cpu list while
the private field is getting updated from here which might lead to
some interesting races.
I recognise that what you are trying to do is respond to Gilad's
request that you really check if an IPI here is necessary. I think what
you need to do is check if a page with a count of 0 is encountered
and if it is, then a draining of the per-cpu lists is necessary. To
address Gilad's concerns, be sure to only this this once per attempt at
CMA rather than for every page encountered with a count of 0 to avoid a
storm of IPIs.
It's actually more then that.
This is the same issue that I first fixed with a change to free_pcppages_bulk()
function[1]. At the time of positing, you said you'd like me to try and find
a different solution which would not involve paying the price of calling
get_pageblock_migratetype(). Later I also realised that this solution is
not enough.
[1] http://article.gmane.org/gmane.linux.kernel.mm/70314
Yes. I had forgotten the history but looking at that patch again,
I would reach the conclusion that this was adding a new call to
get_pageblock_migratetype() in the bulk free path. That would affect
everybody whether they were using CMA or not.
This will be a bit ugly, but we can also use that code and compile it conditionally
when CMA has been enabled.
That would also be very unfortunate because it means enabling CMA incurs
a performance cost to everyone whether they use CMA or not. For ARM,
this may not be a problem but it would be for other arches if they
wanted to use CMA or if it ever became part of a distro contig.
Pages, which have incorrect migrate type on free finally
causes pageblock migration type change from MIGRATE_CMA to MIGRATE_MOVABLE.
I'm not quite seeing this. In free_hot_cold_page(), the pageblock
type is checked so the page private should be set to MIGRATE_CMA or
MIGRATE_ISOLATE for the CMA area. It's not clear how this can change a
pageblock to MIGRATE_MOVABLE in error. If it turns out that you
absolutely have to call get_pageblock_migratetype() from
free_pcppages_bulk() and my alternative suggestion did not work out then
document all these issues in a comment when putting the call under
CONFIG_CMA so that it is not forgotten.
--
Mel Gorman
SUSE Labs
From: Michal Nazarewicz <hidden> Date: 2012-02-02 19:53:32
On Tue, Jan 31, 2012 at 05:23:59PM +0100, Marek Szyprowski wrote:
quoted
Pages, which have incorrect migrate type on free finally
causes pageblock migration type change from MIGRATE_CMA to MIGRATE_MOVABLE.
On Thu, 02 Feb 2012 13:47:29 +0100, Mel Gorman [off-list ref] wrote:
I'm not quite seeing this. In free_hot_cold_page(), the pageblock
type is checked so the page private should be set to MIGRATE_CMA or
MIGRATE_ISOLATE for the CMA area. It's not clear how this can change a
pageblock to MIGRATE_MOVABLE in error.
Here's what I think may happen:
When drain_all_pages() is called, __free_one_page() is called for each page on
pcp list with migrate type deducted from page_private() which is MIGRATE_CMA.
This result in the page being put on MIGRATE_CMA freelist even though its
pageblock's migrate type is MIGRATE_ISOLATE.
When allocation happens and pcp list is empty, rmqueue_bulk() will get executed
with migratetype argument set to MIGRATE_MOVABLE. It calls __rmqueue() to grab
some pages and because the page described above is on MIGRATE_CMA freelist it
may be returned back to rmqueue_bulk().
But, pageblock's migrate type is not MIGRATE_CMA but MIGRATE_ISOLATE, so the
following code:
#ifdef CONFIG_CMA
if (is_pageblock_cma(page))
set_page_private(page, MIGRATE_CMA);
else
#endif
set_page_private(page, migratetype);
will set it's private to MIGRATE_MOVABLE and in the end the page lands back
on MIGRATE_MOVABLE pcp list but this time with page_private == MIGRATE_MOVABLE
and not MIGRATE_CMA.
One more drain_all_pages() (which may happen since alloc_contig_range() calls
set_migratetype_isolate() for each block) and next __rmqueue_fallback() may
convert the whole pageblock to MIGRATE_MOVABLE.
I know, this sounds crazy and improbable, but I couldn't find an easier path
to destruction. As you pointed, once the page is allocated, free_hot_cold_page()
will do the right thing by reading pageblock's migrate type.
Marek is currently experimenting with various patches including the following
change:
#ifdef CONFIG_CMA
int mt = get_pageblock_migratetype(page);
if (is_migrate_cma(mt) || mt == MIGRATE_ISOLATE)
set_page_private(page, mt);
else
#endif
set_page_private(page, migratetype);
As a matter of fact, if __rmqueue() was changed to return migrate type of the
freelist it took page from, we could avoid this get_pageblock_migratetype() all
together. For now, however, I'd rather not go that way just yet -- I'll be happy
to dig into it once CMA gets merged.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Micha? ?mina86? Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-02-03 09:32:03
Hello,
On Thursday, February 02, 2012 8:53 PM Micha? Nazarewicz wrote:
quoted
On Tue, Jan 31, 2012 at 05:23:59PM +0100, Marek Szyprowski wrote:
quoted
Pages, which have incorrect migrate type on free finally
causes pageblock migration type change from MIGRATE_CMA to MIGRATE_MOVABLE.
On Thu, 02 Feb 2012 13:47:29 +0100, Mel Gorman [off-list ref] wrote:
quoted
I'm not quite seeing this. In free_hot_cold_page(), the pageblock
type is checked so the page private should be set to MIGRATE_CMA or
MIGRATE_ISOLATE for the CMA area. It's not clear how this can change a
pageblock to MIGRATE_MOVABLE in error.
Here's what I think may happen:
When drain_all_pages() is called, __free_one_page() is called for each page on
pcp list with migrate type deducted from page_private() which is MIGRATE_CMA.
This result in the page being put on MIGRATE_CMA freelist even though its
pageblock's migrate type is MIGRATE_ISOLATE.
When allocation happens and pcp list is empty, rmqueue_bulk() will get executed
with migratetype argument set to MIGRATE_MOVABLE. It calls __rmqueue() to grab
some pages and because the page described above is on MIGRATE_CMA freelist it
may be returned back to rmqueue_bulk().
But, pageblock's migrate type is not MIGRATE_CMA but MIGRATE_ISOLATE, so the
following code:
#ifdef CONFIG_CMA
if (is_pageblock_cma(page))
set_page_private(page, MIGRATE_CMA);
else
#endif
set_page_private(page, migratetype);
will set it's private to MIGRATE_MOVABLE and in the end the page lands back
on MIGRATE_MOVABLE pcp list but this time with page_private == MIGRATE_MOVABLE
and not MIGRATE_CMA.
One more drain_all_pages() (which may happen since alloc_contig_range() calls
set_migratetype_isolate() for each block) and next __rmqueue_fallback() may
convert the whole pageblock to MIGRATE_MOVABLE.
I know, this sounds crazy and improbable, but I couldn't find an easier path
to destruction. As you pointed, once the page is allocated, free_hot_cold_page()
will do the right thing by reading pageblock's migrate type.
Marek is currently experimenting with various patches including the following
change:
#ifdef CONFIG_CMA
int mt = get_pageblock_migratetype(page);
if (is_migrate_cma(mt) || mt == MIGRATE_ISOLATE)
set_page_private(page, mt);
else
#endif
set_page_private(page, migratetype);
As a matter of fact, if __rmqueue() was changed to return migrate type of the
freelist it took page from, we could avoid this get_pageblock_migratetype() all
together. For now, however, I'd rather not go that way just yet -- I'll be happy
to dig into it once CMA gets merged.
After this and some other changes I'm unable to reproduce that issue. I did a whole
night tests and it still works fine, so it looks that it has been finally solved.
I will post v20 patchset soon :)
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
On Thu, Feb 02, 2012 at 08:53:25PM +0100, Michal Nazarewicz wrote:
quoted
On Tue, Jan 31, 2012 at 05:23:59PM +0100, Marek Szyprowski wrote:
quoted
Pages, which have incorrect migrate type on free finally
causes pageblock migration type change from MIGRATE_CMA to MIGRATE_MOVABLE.
On Thu, 02 Feb 2012 13:47:29 +0100, Mel Gorman [off-list ref] wrote:
quoted
I'm not quite seeing this. In free_hot_cold_page(), the pageblock
type is checked so the page private should be set to MIGRATE_CMA or
MIGRATE_ISOLATE for the CMA area. It's not clear how this can change a
pageblock to MIGRATE_MOVABLE in error.
Here's what I think may happen:
When drain_all_pages() is called, __free_one_page() is called for each page on
pcp list with migrate type deducted from page_private() which is MIGRATE_CMA.
This result in the page being put on MIGRATE_CMA freelist even though its
pageblock's migrate type is MIGRATE_ISOLATE.
Ok, although it will only be allocated for MIGRATE_CMA-compatible
requests so it is not a disaster.
When allocation happens and pcp list is empty, rmqueue_bulk() will get executed
with migratetype argument set to MIGRATE_MOVABLE. It calls __rmqueue() to grab
some pages and because the page described above is on MIGRATE_CMA freelist it
may be returned back to rmqueue_bulk().
This will allocate the page from a pageblock we are trying to isolate
pages from, but only for a movable page that can still be migrated. It
does mean that CMA is doing more work than it should of course and
the problem also impacts memory hot-remove. It's worse for memory
hot-remove because potentially an UNMOVABLE page was allocated from
a MIGRATE_ISOLATE pageblock.
But, pageblock's migrate type is not MIGRATE_CMA but MIGRATE_ISOLATE, so the
following code:
#ifdef CONFIG_CMA
if (is_pageblock_cma(page))
set_page_private(page, MIGRATE_CMA);
else
#endif
set_page_private(page, migratetype);
will set it's private to MIGRATE_MOVABLE and in the end the page lands back
on MIGRATE_MOVABLE pcp list but this time with page_private == MIGRATE_MOVABLE
and not MIGRATE_CMA.
One more drain_all_pages() (which may happen since alloc_contig_range() calls
set_migratetype_isolate() for each block) and next __rmqueue_fallback() may
convert the whole pageblock to MIGRATE_MOVABLE.
I know, this sounds crazy and improbable, but I couldn't find an easier path
to destruction. As you pointed, once the page is allocated, free_hot_cold_page()
will do the right thing by reading pageblock's migrate type.
Ok, it's crazy but the problem is there.
Marek is currently experimenting with various patches including the following
change:
#ifdef CONFIG_CMA
int mt = get_pageblock_migratetype(page);
if (is_migrate_cma(mt) || mt == MIGRATE_ISOLATE)
set_page_private(page, mt);
else
#endif
set_page_private(page, migratetype);
As a matter of fact, if __rmqueue() was changed to return migrate type of the
freelist it took page from, we could avoid this get_pageblock_migratetype() all
together. For now, however, I'd rather not go that way just yet -- I'll be happy
to dig into it once CMA gets merged.
Ok, thanks for persisting with this.
--
Mel Gorman
SUSE Labs
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-01-26 09:10:12
From: Michal Nazarewicz <redacted>
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 <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/page-isolation.h | 7 ++
mm/page_alloc.c | 183 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 190 insertions(+), 0 deletions(-)
@@ -39,5 +39,12 @@ extern void update_pcp_isolate_block(unsigned long pfn);externintset_migratetype_isolate(structpage*page);externvoidunset_migratetype_isolate(structpage*page);+#ifdef CONFIG_CMA++/* The below functions must be run on a range from a single zone. */+externintalloc_contig_range(unsignedlongstart,unsignedlongend);+externvoidfree_contig_range(unsignedlongpfn,unsignednr_pages);++#endif#endif
@@ -5488,6 +5489,188 @@ out:spin_unlock_irqrestore(&zone->lock,flags);}+#ifdef CONFIG_CMA++staticunsignedlongpfn_align_to_maxpage_down(unsignedlongpfn)+{+returnpfn&~(MAX_ORDER_NR_PAGES-1);+}++staticunsignedlongpfn_align_to_maxpage_up(unsignedlongpfn)+{+returnALIGN(pfn,MAX_ORDER_NR_PAGES);+}++staticstructpage*+__alloc_contig_migrate_alloc(structpage*page,unsignedlongprivate,+int**resultp)+{+returnalloc_page(GFP_HIGHUSER_MOVABLE);+}++/* [start, end) must belong to a single zone. */+staticint__alloc_contig_migrate_range(unsignedlongstart,unsignedlongend)+{+/* This function is based on compact_zone() from compaction.c. */++unsignedlongpfn=start;+unsignedinttries=0;+intret=0;++structcompact_controlcc={+.nr_migratepages=0,+.order=-1,+.zone=page_zone(pfn_to_page(start)),+.sync=true,+};+INIT_LIST_HEAD(&cc.migratepages);++migrate_prep_local();++while(pfn<end||!list_empty(&cc.migratepages)){+if(fatal_signal_pending(current)){+ret=-EINTR;+break;+}++if(list_empty(&cc.migratepages)){+cc.nr_migratepages=0;+pfn=isolate_migratepages_range(cc.zone,&cc,+pfn,end);+if(!pfn){+ret=-EINTR;+break;+}+tries=0;+}elseif(++tries==5){+ret=ret<0?ret:-EBUSY;+break;+}++ret=migrate_pages(&cc.migratepages,+__alloc_contig_migrate_alloc,+0,false,true);+}++putback_lru_pages(&cc.migratepages);+returnret;+}++/**+*alloc_contig_range()--triestoallocategivenrangeofpages+*@start:startPFNtoallocate+*@end:one-past-the-lastPFNtoallocate+*+*ThePFNrangedoesnothavetobepageblockorMAX_ORDER_NR_PAGES+*aligned,howeverit'sthecaller'sresponsibilitytoguaranteethat+*wearetheonlythreadthatchangesmigratetypeofpageblocksthe+*pagesfallin.+*+*ThePFNrangemustbelongtoasinglezone.+*+*Returnszeroonsuccessornegativeerrorcode.Onsuccessall+*pageswhichPFNisin[start,end)areallocatedforthecallerand+*needtobefreedwithfree_contig_range().+*/+intalloc_contig_range(unsignedlongstart,unsignedlongend)+{+unsignedlongouter_start,outer_end;+intret=0,order;++/*+*Whatwedohereiswemarkallpageblocksinrangeas+*MIGRATE_ISOLATE.Becauseofthewaypageallocatorwork,we+*aligntherangetoMAX_ORDERpagessothatpageallocator+*won'ttrytomergebuddiesfromdifferentpageblocksand+*changeMIGRATE_ISOLATEtosomeothermigrationtype.+*+*OncethepageblocksaremarkedasMIGRATE_ISOLATE,we+*migratethepagesfromanunalignedrange(ie.pagesthat+*weareinterestedin).Thiswillputallthepagesin+*rangebacktopageallocatorasMIGRATE_ISOLATE.+*+*Whenthisisdone,wetakethepagesinrangefrompage+*allocatorremovingthemfromthebuddysystem.Thisway+*pageallocatorwillneverconsiderusingthem.+*+*Thisletsusmarkthepageblocksbackas+*MIGRATE_CMA/MIGRATE_MOVABLEsothatfreepagesinthe+*MAX_ORDERalignedrangebutnotintheunaligned,original+*rangeareputbacktopageallocatorsothatbuddycanuse+*them.+*/++ret=start_isolate_page_range(pfn_align_to_maxpage_down(start),+pfn_align_to_maxpage_up(end));+if(ret)+gotodone;++ret=__alloc_contig_migrate_range(start,end);+if(ret)+gotodone;++/*+*Pagesfrom[start,end)arewithinaMAX_ORDER_NR_PAGES+*alignedblocksthataremarkedasMIGRATE_ISOLATE.What's+*more,allpagesin[start,end)arefreeinpageallocator.+*Whatwearegoingtodoistoallocateallpagesfrom+*[start,end)(thatisremovethemfrompageallocater).+*+*Theonlyproblemisthatpagesatthebeginningand@the+*endofinterestingrangemaybenotalignedwithpagesthat+*pageallocatorholds,ie.theycanbepartofhigherorder+*pages.Becauseofthis,wereservethebiggerrangeand+*oncethisisdonefreethepageswearenotinterestedin.+*/++lru_add_drain_all();+drain_all_pages();++order=0;+outer_start=start;+while(!PageBuddy(pfn_to_page(outer_start))){+if(WARN_ON(++order>=MAX_ORDER)){+ret=-EINVAL;+gotodone;+}+outer_start&=~0UL<<order;+}++/* Make sure the range is really isolated. */+if(test_pages_isolated(outer_start,end)){+pr_warn("__alloc_contig_migrate_range: test_pages_isolated(%lx, %lx) failed\n",+outer_start,end);+ret=-EBUSY;+gotodone;+}++outer_end=isolate_freepages_range(outer_start,end);+if(!outer_end){+ret=-EBUSY;+gotodone;+}++/* Free head and tail (if any) */+if(start!=outer_start)+free_contig_range(outer_start,start-outer_start);+if(end!=outer_end)+free_contig_range(end,outer_end-end);++done:+undo_isolate_page_range(pfn_align_to_maxpage_down(start),+pfn_align_to_maxpage_up(end));+returnret;+}++voidfree_contig_range(unsignedlongpfn,unsignednr_pages)+{+for(;nr_pages--;++pfn)+__free_page(pfn_to_page(pfn));+}++#endif++#ifdef CONFIG_MEMORY_HOTREMOVE/**Allpagesintherangemustbeisolatedbeforecallingthis.
On Thu, Jan 26, 2012 at 10:00:48AM +0100, Marek Szyprowski wrote:
quoted hunk
From: Michal Nazarewicz <redacted>
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 <redacted>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/page-isolation.h | 7 ++
mm/page_alloc.c | 183 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 190 insertions(+), 0 deletions(-)
@@ -39,5 +39,12 @@ extern void update_pcp_isolate_block(unsigned long pfn);externintset_migratetype_isolate(structpage*page);externvoidunset_migratetype_isolate(structpage*page);+#ifdef CONFIG_CMA++/* The below functions must be run on a range from a single zone. */+externintalloc_contig_range(unsignedlongstart,unsignedlongend);+externvoidfree_contig_range(unsignedlongpfn,unsignednr_pages);++#endif
Did you really mean page-isolation.h? I would have thought gfp.h
would be a more suitable fit.
@@ -5488,6 +5489,188 @@ out:spin_unlock_irqrestore(&zone->lock,flags);}+#ifdef CONFIG_CMA++staticunsignedlongpfn_align_to_maxpage_down(unsignedlongpfn)+{+returnpfn&~(MAX_ORDER_NR_PAGES-1);+}++staticunsignedlongpfn_align_to_maxpage_up(unsignedlongpfn)+{+returnALIGN(pfn,MAX_ORDER_NR_PAGES);+}++staticstructpage*+__alloc_contig_migrate_alloc(structpage*page,unsignedlongprivate,+int**resultp)+{+returnalloc_page(GFP_HIGHUSER_MOVABLE);+}++/* [start, end) must belong to a single zone. */+staticint__alloc_contig_migrate_range(unsignedlongstart,unsignedlongend)+{+/* This function is based on compact_zone() from compaction.c. */++unsignedlongpfn=start;+unsignedinttries=0;+intret=0;++structcompact_controlcc={+.nr_migratepages=0,+.order=-1,+.zone=page_zone(pfn_to_page(start)),+.sync=true,+};+INIT_LIST_HEAD(&cc.migratepages);++migrate_prep_local();++while(pfn<end||!list_empty(&cc.migratepages)){+if(fatal_signal_pending(current)){+ret=-EINTR;+break;+}++if(list_empty(&cc.migratepages)){+cc.nr_migratepages=0;+pfn=isolate_migratepages_range(cc.zone,&cc,+pfn,end);+if(!pfn){+ret=-EINTR;+break;+}+tries=0;+}elseif(++tries==5){+ret=ret<0?ret:-EBUSY;+break;+}++ret=migrate_pages(&cc.migratepages,+__alloc_contig_migrate_alloc,+0,false,true);+}++putback_lru_pages(&cc.migratepages);+returnret;+}++/**+*alloc_contig_range()--triestoallocategivenrangeofpages+*@start:startPFNtoallocate+*@end:one-past-the-lastPFNtoallocate+*+*ThePFNrangedoesnothavetobepageblockorMAX_ORDER_NR_PAGES+*aligned,howeverit'sthecaller'sresponsibilitytoguaranteethat+*wearetheonlythreadthatchangesmigratetypeofpageblocksthe+*pagesfallin.+*+*ThePFNrangemustbelongtoasinglezone.+*+*Returnszeroonsuccessornegativeerrorcode.Onsuccessall+*pageswhichPFNisin[start,end)areallocatedforthecallerand+*needtobefreedwithfree_contig_range().+*/+intalloc_contig_range(unsignedlongstart,unsignedlongend)+{+unsignedlongouter_start,outer_end;+intret=0,order;++/*+*Whatwedohereiswemarkallpageblocksinrangeas+*MIGRATE_ISOLATE.Becauseofthewaypageallocatorwork,we+*aligntherangetoMAX_ORDERpagessothatpageallocator+*won'ttrytomergebuddiesfromdifferentpageblocksand+*changeMIGRATE_ISOLATEtosomeothermigrationtype.+*+*OncethepageblocksaremarkedasMIGRATE_ISOLATE,we+*migratethepagesfromanunalignedrange(ie.pagesthat+*weareinterestedin).Thiswillputallthepagesin+*rangebacktopageallocatorasMIGRATE_ISOLATE.+*+*Whenthisisdone,wetakethepagesinrangefrompage+*allocatorremovingthemfromthebuddysystem.Thisway+*pageallocatorwillneverconsiderusingthem.+*+*Thisletsusmarkthepageblocksbackas+*MIGRATE_CMA/MIGRATE_MOVABLEsothatfreepagesinthe+*MAX_ORDERalignedrangebutnotintheunaligned,original+*rangeareputbacktopageallocatorsothatbuddycanuse+*them.+*/++ret=start_isolate_page_range(pfn_align_to_maxpage_down(start),+pfn_align_to_maxpage_up(end));+if(ret)+gotodone;++ret=__alloc_contig_migrate_range(start,end);+if(ret)+gotodone;++/*+*Pagesfrom[start,end)arewithinaMAX_ORDER_NR_PAGES+*alignedblocksthataremarkedasMIGRATE_ISOLATE.What's+*more,allpagesin[start,end)arefreeinpageallocator.+*Whatwearegoingtodoistoallocateallpagesfrom+*[start,end)(thatisremovethemfrompageallocater).+*+*Theonlyproblemisthatpagesatthebeginningandatthe+*endofinterestingrangemaybenotalignedwithpagesthat+*pageallocatorholds,ie.theycanbepartofhigherorder+*pages.Becauseofthis,wereservethebiggerrangeand+*oncethisisdonefreethepageswearenotinterestedin.+*/++lru_add_drain_all();+drain_all_pages();+
You unconditionally drain all pages here. It's up to you whether to
keep that or try reduce IPIs by only sending one if a page with count
0 is found in the range. I think it is something that could be followed
up on later and is not necessary for initial merging and wider testing.
+ order = 0;
+ outer_start = start;
+ while (!PageBuddy(pfn_to_page(outer_start))) {
+ if (WARN_ON(++order >= MAX_ORDER)) {
+ ret = -EINVAL;
+ goto done;
+ }
+ outer_start &= ~0UL << order;
+ }
+
Just a small note here - you are checking PageBuddy without zone->lock .
As you have isolated the range, you have a reasonable expectation that
this is safe but if you spin another version of the patch it might
justify a small comment.
+ /* Make sure the range is really isolated. */
+ if (test_pages_isolated(outer_start, end)) {
+ pr_warn("__alloc_contig_migrate_range: test_pages_isolated(%lx, %lx) failed\n",
+ outer_start, end);
+ ret = -EBUSY;
+ goto done;
+ }
+
+ outer_end = isolate_freepages_range(outer_start, end);
+ if (!outer_end) {
+ ret = -EBUSY;
+ goto done;
+ }
+
+ /* Free head and tail (if any) */
+ if (start != outer_start)
+ free_contig_range(outer_start, start - outer_start);
+ if (end != outer_end)
+ free_contig_range(end, outer_end - end);
+
+done:
+ undo_isolate_page_range(pfn_align_to_maxpage_down(start),
+ pfn_align_to_maxpage_up(end));
+ return ret;
+}
+
+void free_contig_range(unsigned long pfn, unsigned nr_pages)
+{
+ for (; nr_pages--; ++pfn)
+ __free_page(pfn_to_page(pfn));
+}
+
+#endif
+
+
Bit of whitespace damage there.
I confess that I did not read this one quite as carefully because I
think I looked a previous version that looked ok at the time. As it
affects CMA and only CMA I also expect others will be spending a lot
of effort and testing on this. Nothing obvious or horrible jumped out
at me other than the page_isolation.h thing and that could be argued
either way so;
Acked-by: Mel Gorman <redacted>
/*
#ifdef CONFIG_MEMORY_HOTREMOVE
/*
* All pages in the range must be isolated before calling this.