On Tue, Apr 26, 2016 at 09:40:45PM +0200, Vlastimil Babka wrote: > On 04/26/2016 02:55 AM, Joonsoo Kim wrote: > >On Mon, Apr 25, 2016 at 03:35:50PM +0200, Vlastimil Babka wrote: > >>@@ -846,9 +845,11 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, > >> spin_unlock_irqrestore(&zone->lru_lock, flags); > >> locked = false; > >> } > >>- putback_movable_pages(migratelist); > >>- nr_isolated = 0; > >>+ acct_isolated(zone, cc); > >>+ putback_movable_pages(&cc->migratepages); > >>+ cc->nr_migratepages = 0; > >> cc->last_migrated_pfn = 0; > >>+ nr_isolated = 0; > > > >Is it better to use separate list and merge it cc->migratepages when > >finishing instead of using cc->migratepages directly? If > >isolate_migratepages() try to isolate more than one page block and keep > >isolated page on previous pageblock, this putback all will invalidate > >all the previous work. It would be beyond of the scope of this > >function. Now, isolate_migratepages() try to isolate the page in one > >pageblock so this code is safe. But, I think that removing such > >dependency will be helpful in the future. I'm not strongly insisting it > >so if you think it's not useful thing, please ignore this comment. > > migratelist was merely a reference to cc->migratepages, so it > wouldn't prevent the situation you are suggesting. A truly separate > list would need to be appended to cc->migratepages when leaving > isolate_migratepages_block() and there's no need to do that right > now. What I suggest is using truly separate list by defining LIST_HEAD(xxx) on top of the function. But, I'm okay you think that there's no need to do it right now. > > BTW, can you check patch 1/3? Thanks! Done. Thanks. -- 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/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>