On Thu 20-12-18 15:58:03, Wei Yang wrote: > On Wed, Dec 19, 2018 at 03:39:27PM +0100, Michal Hocko wrote: > >On Wed 19-12-18 14:33:27, Wei Yang wrote: > >[...] > >> Then I am confused about the objection to this patch. Finally, we drain > >> all the pages in pcp list and the range is isolated. > > > >Please read my emails more carefully. As I've said, the only reason to > >do care about draining is to remove it from where it doesn't belong. > > I go through the thread again and classify two main opinions from you > and Oscar. > > 1) We can still allocate pages in a specific range from pcp list even we > have already isolate this range. > 2) We shouldn't rely on caller to drain pages and > set_migratetype_isolate() may handle a range cross zones. > > I understand the second one and agree it is not proper to rely on caller > and make the assumption on range for set_migratetype_isolate(). > > My confusion comes from the first one. As you and Oscar both mentioned > this and Oscar said "I had the same fear", this makes me think current > implementation is buggy. But your following reply said this is not. This > means current approach works fine. > > If the above understanding is correct, and combining with previous > discussion, the improvement we can do is to remove the drain_all_pages() > in __offline_pages()/alloc_contig_range(). By doing so, the pcp list > drain doesn't rely on caller and the isolation/drain on each pageblock > ensures pcp list will not contain any page in this range now and future. > This imply the drain_all_pages() in > __offline_pages()/alloc_contig_range() is not necessary. > > Is my understanding correct? Yes -- Michal Hocko SUSE Labs
