On Monday 30 April 2012 11:02:39 Mel Gorman wrote:
> On Fri, Apr 27, 2012 at 12:57:11PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > From: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx>
> > Subject: [PATCH v4] mm: compaction: handle incorrect Unmovable type pageblocks
> >
> > When Unmovable pages are freed from Unmovable type pageblock
> > (and some Movable type pages are left in it) waiting until
> > an allocation takes ownership of the block may take too long.
> > The type of the pageblock remains unchanged so the pageblock
> > cannot be used as a migration target during compaction.
> >
> > Fix it by:
> >
> > * Adding enum compact_mode (COMPACT_ASYNC_MOVABLE,
> > COMPACT_ASYNC_UNMOVABLE and COMPACT_SYNC) and then converting
> > sync field in struct compact_control to use it.
> >
> > * Scanning the Unmovable pageblocks (during COMPACT_ASYNC_UNMOVABLE
> > and COMPACT_SYNC compactions) and building a count based on
> > finding PageBuddy pages, page_count(page) == 0 or PageLRU pages.
> > If all pages within the Unmovable pageblock are in one of those
> > three sets change the whole pageblock type to Movable.
> >
> > My particular test case (on a ARM EXYNOS4 device with 512 MiB,
> > which means 131072 standard 4KiB pages in 'Normal' zone) is to:
> > - allocate 120000 pages for kernel's usage
> > - free every second page (60000 pages) of memory just allocated
> > - allocate and use 60000 pages from user space
> > - free remaining 60000 pages of kernel memory
> > (now we have fragmented memory occupied mostly by user space pages)
> > - try to allocate 100 order-9 (2048 KiB) pages for kernel's usage
> >
> > The results:
> > - with compaction disabled I get 11 successful allocations
> > - with compaction enabled - 14 successful allocations
> > - with this patch I'm able to get all 100 successful allocations
> >
>
> This is looking much better to me. However, I would really like to see
> COMPACT_ASYNC_UNMOVABLE being used by the page allocator instead of depending
> on kswapd to do the work. Right now as it uses COMPACT_ASYNC_MOVABLE only,
> I think it uses COMPACT_SYNC too easily (making latency worse).
Is the following v4 code in __alloc_pages_direct_compact() not enough?
@@ -2122,7 +2122,7 @@
__alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
struct zonelist *zonelist, enum zone_type high_zoneidx,
nodemask_t *nodemask, int alloc_flags, struct zone *preferred_zone,
- int migratetype, bool sync_migration,
+ int migratetype, enum compact_mode migration_mode,
bool *deferred_compaction,
unsigned long *did_some_progress)
{
@@ -2285,7 +2285,7 @@
int alloc_flags;
unsigned long pages_reclaimed = 0;
unsigned long did_some_progress;
- bool sync_migration = false;
+ enum compact_mode migration_mode = COMPACT_ASYNC_MOVABLE;
bool deferred_compaction = false;
/*
@@ -2360,19 +2360,31 @@
goto nopage;
/*
- * Try direct compaction. The first pass is asynchronous. Subsequent
- * attempts after direct reclaim are synchronous
+ * Try direct compaction. The first and second pass are asynchronous.
+ * Subsequent attempts after direct reclaim are synchronous.
*/
page = __alloc_pages_direct_compact(gfp_mask, order,
zonelist, high_zoneidx,
nodemask,
alloc_flags, preferred_zone,
- migratetype, sync_migration,
+ migratetype, migration_mode,
&deferred_compaction,
&did_some_progress);
if (page)
goto got_pg;
- sync_migration = true;
+
+ migration_mode = COMPACT_ASYNC_UNMOVABLE;
+ page = __alloc_pages_direct_compact(gfp_mask, order,
+ zonelist, high_zoneidx,
+ nodemask,
+ alloc_flags, preferred_zone,
+ migratetype, migration_mode,
+ &deferred_compaction,
+ &did_some_progress);
+ if (page)
+ goto got_pg;
+
+ migration_mode = COMPACT_SYNC;
/*
* If compaction is deferred for high-order allocations, it is because
> Specifically
>
> 1. Leave try_to_compact_pages() taking a sync parameter. It is up to
> compaction how to treat sync==false
> 2. When sync==false, start with ASYNC_MOVABLE. Track how many pageblocks
> were scanned during compaction and how many of them were
> MIGRATE_UNMOVABLE. If compaction ran fully (COMPACT_COMPLETE) it implies
> that there is not a suitable page for allocation. In this case then
> check how if there were enough MIGRATE_UNMOVABLE pageblocks to try a
> second pass in ASYNC_FULL. By keeping all the logic in compaction.c
> it prevents too much knowledge of compaction sneaking into
> page_alloc.c
Do you mean that try_to_compact_pages() should handle COMPACT_ASYNC_MOVABLE
and COMPACT_ASYNC_UNMOVABLE internally while __alloc_pages_direct_compact()
(and its users) should only pass bool sync to it?
> 3. When scanning ASYNC_FULL, *only* scan the MIGRATE_UNMOVABLE blocks as
> migration targets because the first pass would have scanned within
> MIGRATE_MOVABLE. This will reduce the cost of the second pass.
That is what the current v4 code should already be doing with:
[...]
/* Returns true if the page is within a block suitable for migration to */
-static bool suitable_migration_target(struct page *page)
+static bool suitable_migration_target(struct page *page,
+ enum compact_mode mode)
{
int migratetype = get_pageblock_migratetype(page);
@@ -373,7 +413,13 @@
return true;
/* If the block is MIGRATE_MOVABLE or MIGRATE_CMA, allow migration */
- if (migrate_async_suitable(migratetype))
+ if (mode != COMPACT_ASYNC_UNMOVABLE &&
+ migrate_async_suitable(migratetype))
+ return true;
+
+ if (mode != COMPACT_ASYNC_MOVABLE &&
+ migratetype == MIGRATE_UNMOVABLE &&
+ rescue_unmovable_pageblock(page))
return true;
/* Otherwise skip the block */
?
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung Poland R&D Center
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>