From 3957c7768e5ea02fd3345176ddd340f820e5d285 Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Wed, 15 Jun 2011 15:08:25 -0700 Subject: mm: compaction: fix special case -1 order checks Commit 56de7263fcf3 ("mm: compaction: direct compact when a high-order allocation fails") introduced a check for cc->order == -1 in compact_finished. We should continue compacting in that case because the request came from userspace and there is no particular order to compact for. Similar check has been added by 82478fb7 (mm: compaction: prevent division-by-zero during user-requested compaction) for compaction_suitable. The check is, however, done after zone_watermark_ok which uses order as a right hand argument for shifts. Not only watermark check is pointless if we can break out without it but it also uses 1 << -1 which is not well defined (at least from C standard). Let's move the -1 check above zone_watermark_ok. [minchan.kim@gmail.com> - caught compaction_suitable] Signed-off-by: Michal Hocko Cc: Mel Gorman Reviewed-by: Minchan Kim Reviewed-by: KAMEZAWA Hiroyuki Acked-by: Mel Gorman Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/compaction.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'mm') diff --git a/mm/compaction.c b/mm/compaction.c index 021a296..94bdbe1 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -420,13 +420,6 @@ static int compact_finished(struct zone *zone, if (cc->free_pfn <= cc->migrate_pfn) return COMPACT_COMPLETE; - /* Compaction run is not finished if the watermark is not met */ - watermark = low_wmark_pages(zone); - watermark += (1 << cc->order); - - if (!zone_watermark_ok(zone, cc->order, watermark, 0, 0)) - return COMPACT_CONTINUE; - /* * order == -1 is expected when compacting via * /proc/sys/vm/compact_memory @@ -434,6 +427,13 @@ static int compact_finished(struct zone *zone, if (cc->order == -1) return COMPACT_CONTINUE; + /* Compaction run is not finished if the watermark is not met */ + watermark = low_wmark_pages(zone); + watermark += (1 << cc->order); + + if (!zone_watermark_ok(zone, cc->order, watermark, 0, 0)) + return COMPACT_CONTINUE; + /* Direct compactor: Is a suitable page free? */ for (order = cc->order; order < MAX_ORDER; order++) { /* Job done if page is free of the right migratetype */ @@ -461,6 +461,13 @@ unsigned long compaction_suitable(struct zone *zone, int order) unsigned long watermark; /* + * order == -1 is expected when compacting via + * /proc/sys/vm/compact_memory + */ + if (order == -1) + return COMPACT_CONTINUE; + + /* * Watermarks for order-0 must be met for compaction. Note the 2UL. * This is because during migration, copies of pages need to be * allocated and for a short time, the footprint is higher @@ -470,13 +477,6 @@ unsigned long compaction_suitable(struct zone *zone, int order) return COMPACT_SKIPPED; /* - * order == -1 is expected when compacting via - * /proc/sys/vm/compact_memory - */ - if (order == -1) - return COMPACT_CONTINUE; - - /* * fragmentation index determines if allocation failures are due to * low memory or external fragmentation * -- cgit v1.1