Hello Xishi, On Tue, Aug 21, 2012 at 08:12:05PM +0800, qiuxishi wrote: > From: Xishi Qiu <qiuxishi@xxxxxxxxxx> > > When offline a section, we move all the free pages and pcp into MIGRATE_ISOLATE list first. > start_isolate_page_range() > set_migratetype_isolate() > drain_all_pages(), > > Here is a problem, it is not sure that pcp will be moved into MIGRATE_ISOLATE list. They may > be moved into MIGRATE_MOVABLE list because page_private() maybe 2. So when finish migrating > pages, the free pages from pcp may be allocated again, and faild in check_pages_isolated(). > drain_all_pages() > drain_local_pages() > drain_pages() > free_pcppages_bulk() > __free_one_page(page, zone, 0, page_private(page)); > > If we add move_freepages_block() after drain_all_pages(), it can not sure that all the pcp > will be moved into MIGRATE_ISOLATE list when the system works on high load. The free pages > which from pcp may immediately be allocated again. > > I think the similar bug described in http://marc.info/?t=134250882300003&r=1&w=2 Yes. I reported the problem a few month ago but it's not real bug in practice but found by my eyes during looking the code so I wanted to confirm the problem. Do you find that problem in real practice? or just code review? Anyway, I don't like your approach which I already considered because it hurts hotpath while the race is really unlikely. Get_pageblock_migratetype is never trivial. We should avoid the overhead in hotpath and move into memory-hotplug itself. Do you see my patch in https://patchwork.kernel.org/patch/1225081/ ? > > > Signed-off-by: Xishi Qiu <qiuxishi@xxxxxxxxxx> > --- > mm/page_alloc.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index d0723b2..501f6de 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -673,7 +673,8 @@ static void free_pcppages_bulk(struct zone *zone, int count, > /* must delete as __free_one_page list manipulates */ > list_del(&page->lru); > /* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */ > - __free_one_page(page, zone, 0, page_private(page)); > + __free_one_page(page, zone, 0, > + get_pageblock_migratetype(page)); > trace_mm_page_pcpu_drain(page, 0, page_private(page)); > } while (--to_free && --batch_free && !list_empty(list)); > } > -- 1.7.6.1 . > > > > . > > -- > 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> -- Kind regards, Minchan Kim -- 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>