On 18.12.18 21:46, Wei Yang wrote: > Below is a brief call flow for __offline_pages() and > alloc_contig_range(): > > __offline_pages()/alloc_contig_range() > start_isolate_page_range() > set_migratetype_isolate() > drain_all_pages() > drain_all_pages() > > Current logic is: isolate and drain pcp list for each pageblock and > drain pcp list again. This is not necessary and we could just drain pcp > list once after isolate this whole range. > > The reason is start_isolate_page_range() will set the migrate type of > a range to MIGRATE_ISOLATE. After doing so, this range will never be > allocated from Buddy, neither to a real user nor to pcp list. > > Since drain_all_pages() is zone based, by reduce times of > drain_all_pages() also reduce some contention on this particular zone. > > Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx> Yes, as far as I can see, when a MIGRATE_ISOLATE page gets freed, it will not go onto the pcp list again. However, start_isolate_page_range() is also called via alloc_contig_range(). Are you sure we can effectively drop the drain_all_pages() for that call path? > > --- > v2: adjust changelog with MIGRATE_ISOLATE effects for the isolated range > --- > mm/page_isolation.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/mm/page_isolation.c b/mm/page_isolation.c > index 43e085608846..f44c0e333bed 100644 > --- a/mm/page_isolation.c > +++ b/mm/page_isolation.c > @@ -83,8 +83,6 @@ static int set_migratetype_isolate(struct page *page, int migratetype, > } > > spin_unlock_irqrestore(&zone->lock, flags); > - if (!ret) > - drain_all_pages(zone); > return ret; > } > > -- Thanks, David / dhildenb
