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? > >-- >Michal Hocko >SUSE Labs -- Wei Yang Help you, Help me
