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

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

On Thu, Dec 20, 2018 at 05:23:02PM +0100, Michal Hocko wrote:
>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

Thanks for your clarification:-)

I would come up with a patch to remove this one.

>
>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me