On Wed 19-12-18 13:53:07, Wei Yang wrote: > On Wed, Dec 19, 2018 at 10:57:19AM +0100, Oscar Salvador wrote: > >On Wed, Dec 19, 2018 at 10:51:10AM +0100, Michal Hocko wrote: > >> On Wed 19-12-18 04:46:56, 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. > >> > >> But it is important to note that those pages still can be allocated from > >> the pcp lists until we do drain_all_pages(). > > > >I had the same fear, but then I saw that move_freepages_block()->move_freepages() moves > >the pages to a new list: > > > ><-- > >list_move(&page->lru, > > &zone->free_area[order].free_list[migratetype]); > >--> > > > > > >But looking at it again, I see that this is only for BuddyPages, so I guess > >that pcp-pages do not really get unlinked, so we could still allocate them. > > Well, I think you are right. But with this in mind, current code looks > buggy. > > Between has_unmovable_pages() and drain_all_pages(), others still could > allocate pages on pcp list, right? This means we thought we have > isolated the range, but not. THere is no guarantee in that regards and I believe there is also no demand for such a strong semantic. Or I do not see it at least. -- Michal Hocko SUSE Labs
