Re: [PATCH] mm/page_alloc: return 0 in case this node has no page within the zone

[Wei Yang]

On Wed, Feb 22, 2017 at 09:49:47AM +0100, Michal Hocko wrote:
>On Thu 09-02-17 21:59:29, Wei Yang wrote:
>> On Tue, Feb 07, 2017 at 04:41:21PM +0100, Michal Hocko wrote:
>> >On Tue 07-02-17 23:32:47, Wei Yang wrote:
>> >> On Tue, Feb 07, 2017 at 10:45:57AM +0100, Michal Hocko wrote:
>> >[...]
>> >> >Is there any reason why for_each_mem_pfn_range cannot be changed to
>> >> >honor the given start/end pfns instead? I can imagine that a small zone
>> >> >would see a similar pointless iterations...
>> >> >
>> >> 
>> >> Hmm... No special reason, just not thought about this implementation. And
>> >> actually I just do the similar thing as in zone_spanned_pages_in_node(), in
>> >> which also return 0 when there is no overlap.
>> >> 
>> >> BTW, I don't get your point. You wish to put the check in
>> >> for_each_mem_pfn_range() definition?
>> >
>> >My point was that you are handling one special case (an empty zone) but
>> >the underlying problem is that __absent_pages_in_range might be wasting
>> >cycles iterating over memblocks that are way outside of the given pfn
>> >range. At least this is my understanding. If you fix that you do not
>> >need the special case, right?
>> >-- 
>> >Michal Hocko
>> >SUSE Labs
>> 
>> > Not really, sorry, this area is full of awkward and subtle code when new
>> > changes build on top of previous awkwardness/surprises. Any cleanup
>> > would be really appreciated. That is the reason I didn't like the
>> > initial check all that much.
>> 
>> Looks my fetchmail failed to get your last reply. So I copied it here.
>> 
>> Yes, the change here looks not that nice, while currently this is what I can't
>> come up with.
>
>THen I will suggest dropping this patch from the mmotm tree because it
>doesn't sound like a big improvement and I would encourage you or
>anybody else to take a deeper look and unclutter this area to be more
>readable and better maintainable.

Hi, Michal

I don't get your point, which part of the code makes you feel uncomfortable?

The behavior here is similar in zone_spanned_pages_in_node(), in which it also
checks whether this node has pages within the zone's required range. The
improvement may not that much, while the logic here is clear. If the node has
no page in the zone's required range, it is not necessary to continue the
calculation.

>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

Attachment: signature.asc
Description: PGP signature