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:12:35PM +0100, Michal Hocko wrote:
>On Wed 19-12-18 13:56:35, Wei Yang wrote:
>> On Wed, Dec 19, 2018 at 02:40:56PM +0100, Michal Hocko wrote:
>> >On Wed 19-12-18 13:29:34, Wei Yang wrote:
>[...]
>> >> As the comment mentioned, in current implementation the range must be in
>> >> one zone.
>> >
>> >I do not see anything like that documented for set_migratetype_isolate.
>> 
>> The comment is not on set_migratetype_isolate, but for its two
>> (grandparent) callers:
>> 
>>    __offline_pages
>>    alloc_contig_range
>
>But those are consumers while the main api here is
>start_isolate_page_range. What happens if we grow a new user?
>Go over the same problems? See the difference?

I didn't intend to fight for my patch, just want to clarify current
implementation :-)

>
>Please try to look at these things from a higher level. We really do not
>want micro optimise on behalf of a sane API. Unless there is a very good
>reason to do that - e.g. when the performance difference is really huge.

Well, actually I get your idea and agree with you not rely on the caller
to drain the page is the proper way to handle this.

Again, I just want to clarify current situation and try to find a proper
way to make it better. Maybe I lost some point, while I am willing get
feedback and suggestions from all of you.

>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me