On 06/09/2014 05:26 PM, Vlastimil Babka wrote: > isolate_freepages_block() rechecks if the pageblock is suitable to be a target > for migration after it has taken the zone->lock. However, the check has been > optimized to occur only once per pageblock, and compact_checklock_irqsave() > might be dropping and reacquiring lock, which means somebody else might have > changed the pageblock's migratetype meanwhile. > > Furthermore, nothing prevents the migratetype to change right after > isolate_freepages_block() has finished isolating. Given how imperfect this is, > it's simpler to just rely on the check done in isolate_freepages() without > lock, and not pretend that the recheck under lock guarantees anything. It is > just a heuristic after all. > > Signed-off-by: Vlastimil Babka <[email protected]>
Reviewed-by: Zhang Yanfei <[email protected]> > Cc: Minchan Kim <[email protected]> > Cc: Mel Gorman <[email protected]> > Cc: Joonsoo Kim <[email protected]> > Cc: Michal Nazarewicz <[email protected]> > Cc: Naoya Horiguchi <[email protected]> > Cc: Christoph Lameter <[email protected]> > Cc: Rik van Riel <[email protected]> > Cc: David Rientjes <[email protected]> > --- > I suggest folding mm-compactionc-isolate_freepages_block-small-tuneup.patch > into this > > mm/compaction.c | 13 ------------- > 1 file changed, 13 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 5175019..b73b182 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -276,7 +276,6 @@ static unsigned long isolate_freepages_block(struct > compact_control *cc, > struct page *cursor, *valid_page = NULL; > unsigned long flags; > bool locked = false; > - bool checked_pageblock = false; > > cursor = pfn_to_page(blockpfn); > > @@ -307,18 +306,6 @@ static unsigned long isolate_freepages_block(struct > compact_control *cc, > if (!locked) > break; > > - /* Recheck this is a suitable migration target under lock */ > - if (!strict && !checked_pageblock) { > - /* > - * We need to check suitability of pageblock only once > - * and this isolate_freepages_block() is called with > - * pageblock range, so just check once is sufficient. > - */ > - checked_pageblock = true; > - if (!suitable_migration_target(page)) > - break; > - } > - > /* Recheck this is a buddy page under lock */ > if (!PageBuddy(page)) > goto isolate_fail; > -- Thanks. Zhang Yanfei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
