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

[Wei Yang <richard.weiyang@gmail.com>]

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