Re: [PATCH v2] mm, page_isolation: remove drain_all_pages() in set_migratetype_isolate()

[Michal Hocko <mhocko@suse.com>]

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