[PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Wei Yang]

Zone with no managed_pages doesn't contribute totalreserv_pages. And the
more nodes we have, the more empty zones there are.

This patch skip the zones to save some cycles.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a919ba5cb3c8..567de15e1106 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7246,6 +7246,9 @@ static void calculate_totalreserve_pages(void)
 			struct zone *zone = pgdat->node_zones + i;
 			long max = 0;
 
+			if (!managed_zone(zone))
+				continue;
+
 			/* Find valid and maximum lowmem_reserve in the zone */
 			for (j = i; j < MAX_NR_ZONES; j++) {
 				if (zone->lowmem_reserve[j] > max)
-- 
2.15.1

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Michal Hocko]

On Mon 12-11-18 15:14:04, Wei Yang wrote:
> Zone with no managed_pages doesn't contribute totalreserv_pages. And the
> more nodes we have, the more empty zones there are.
> 
> This patch skip the zones to save some cycles.

What is the motivation for the patch? Does it really cause any
measurable difference in performance?

> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>  mm/page_alloc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a919ba5cb3c8..567de15e1106 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7246,6 +7246,9 @@ static void calculate_totalreserve_pages(void)
>  			struct zone *zone = pgdat->node_zones + i;
>  			long max = 0;
>  
> +			if (!managed_zone(zone))
> +				continue;
> +
>  			/* Find valid and maximum lowmem_reserve in the zone */
>  			for (j = i; j < MAX_NR_ZONES; j++) {
>  				if (zone->lowmem_reserve[j] > max)
> -- 
> 2.15.1
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Wei Yang]

On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote:
>On Mon 12-11-18 15:14:04, Wei Yang wrote:
>> Zone with no managed_pages doesn't contribute totalreserv_pages. And the
>> more nodes we have, the more empty zones there are.
>> 
>> This patch skip the zones to save some cycles.
>
>What is the motivation for the patch? Does it really cause any
>measurable difference in performance?
>

The motivation here is to reduce some unnecessary work.

Based on my understanding, almost every node has empty zones, since
zones within a node are ordered in monotonic increasing memory address.

The worst case is all zones has managed_pages. For example, there is
only one node, or configured to have only ZONE_NORMAL and
ZONE_MOVABLE. Otherwise, the more node/zone we have, the more empty
zones there are.

I didn't have detail tests on this patch, since I don't have machine
with large numa nodes. While compared with the following ten lines of
code, this check to skip them is worthwhile to me.


>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>>  mm/page_alloc.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index a919ba5cb3c8..567de15e1106 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -7246,6 +7246,9 @@ static void calculate_totalreserve_pages(void)
>>  			struct zone *zone = pgdat->node_zones + i;
>>  			long max = 0;
>>  
>> +			if (!managed_zone(zone))
>> +				continue;
>> +
>>  			/* Find valid and maximum lowmem_reserve in the zone */
>>  			for (j = i; j < MAX_NR_ZONES; j++) {
>>  				if (zone->lowmem_reserve[j] > max)
>> -- 
>> 2.15.1
>> 
>
>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Michal Hocko]

On Mon 12-11-18 14:26:41, Wei Yang wrote:
> On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote:
> >On Mon 12-11-18 15:14:04, Wei Yang wrote:
> >> Zone with no managed_pages doesn't contribute totalreserv_pages. And the
> >> more nodes we have, the more empty zones there are.
> >> 
> >> This patch skip the zones to save some cycles.
> >
> >What is the motivation for the patch? Does it really cause any
> >measurable difference in performance?
> >
> 
> The motivation here is to reduce some unnecessary work.

I have guessed so even though the changelog was quite modest on the
motivation.

> Based on my understanding, almost every node has empty zones, since
> zones within a node are ordered in monotonic increasing memory address.

Yes, this is likely the case. Btw. a check for populated_zone or
for_each_populated_zone would suite much better.

> The worst case is all zones has managed_pages. For example, there is
> only one node, or configured to have only ZONE_NORMAL and
> ZONE_MOVABLE. Otherwise, the more node/zone we have, the more empty
> zones there are.
> 
> I didn't have detail tests on this patch, since I don't have machine
> with large numa nodes. While compared with the following ten lines of
> code, this check to skip them is worthwhile to me.

Well, the main question is whether the optimization is really worth it.
There is not much work done for each zone.

I haven't looked closer whether the patch is actually correct, it seems
to be though, but optimizations without measurable effect tend to be not
that attractive.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Wei Yang]

On Mon, Nov 12, 2018 at 03:40:20PM +0100, Michal Hocko wrote:
>On Mon 12-11-18 14:26:41, Wei Yang wrote:
>> On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote:
>> >On Mon 12-11-18 15:14:04, Wei Yang wrote:
>> >> Zone with no managed_pages doesn't contribute totalreserv_pages. And the
>> >> more nodes we have, the more empty zones there are.
>> >> 
>> >> This patch skip the zones to save some cycles.
>> >
>> >What is the motivation for the patch? Does it really cause any
>> >measurable difference in performance?
>> >
>> 
>> The motivation here is to reduce some unnecessary work.
>
>I have guessed so even though the changelog was quite modest on the
>motivation.
>
>> Based on my understanding, almost every node has empty zones, since
>> zones within a node are ordered in monotonic increasing memory address.
>
>Yes, this is likely the case. Btw. a check for populated_zone or
>for_each_populated_zone would suite much better.
>

Hmm... maybe not exact.

    populated_zone checks zone->present_pages
    managed_zone checks zone->managed_pages

As the comment of managed_zone says, this one records the pages managed
by buddy system. And when we look at the usage of totalreserve_pages, it
is only used in page allocation. And finally, *max* is checked with
managed_pages instead of present_pages.

Because of this, managed_zone is more accurate at this place. Is my
understanding correct?

>> The worst case is all zones has managed_pages. For example, there is
>> only one node, or configured to have only ZONE_NORMAL and
>> ZONE_MOVABLE. Otherwise, the more node/zone we have, the more empty
>> zones there are.
>> 
>> I didn't have detail tests on this patch, since I don't have machine
>> with large numa nodes. While compared with the following ten lines of
>> code, this check to skip them is worthwhile to me.
>
>Well, the main question is whether the optimization is really worth it.
>There is not much work done for each zone.
>
>I haven't looked closer whether the patch is actually correct, it seems
>to be though, but optimizations without measurable effect tend to be not
>that attractive.
>

I believe you are right to some extend, this tiny invisible change is
far away from attractive. While I have another opinion about
optimization.

That would be great to have a strong optimizatioin which improve the
system more than 10%. And there are another kind of optimization that
improves the system a little. We may call it polish.

One polish may not obvious, while cumulative polish make a system
outstanding.

Why German products are famous all around the world? Why people is
willing to pay much more to get a ZWILLING knife than others? Because we
trust German manufactures will polish their product day after day, year
after year with any efforts they can.

So as I am to linux kernel.

BTW, I am also thinking about to reduce some unnecessary work of
lowmem_reserve[] calculation. Because those empty zone's lowmem_reserve
is never used. Even cumulative effect of these two optimization is
trivial, I still think it is worth.

>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Michal Hocko]

On Tue 13-11-18 01:39:42, Wei Yang wrote:
> On Mon, Nov 12, 2018 at 03:40:20PM +0100, Michal Hocko wrote:
> >On Mon 12-11-18 14:26:41, Wei Yang wrote:
> >> On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote:
> >> >On Mon 12-11-18 15:14:04, Wei Yang wrote:
> >> >> Zone with no managed_pages doesn't contribute totalreserv_pages. And the
> >> >> more nodes we have, the more empty zones there are.
> >> >> 
> >> >> This patch skip the zones to save some cycles.
> >> >
> >> >What is the motivation for the patch? Does it really cause any
> >> >measurable difference in performance?
> >> >
> >> 
> >> The motivation here is to reduce some unnecessary work.
> >
> >I have guessed so even though the changelog was quite modest on the
> >motivation.
> >
> >> Based on my understanding, almost every node has empty zones, since
> >> zones within a node are ordered in monotonic increasing memory address.
> >
> >Yes, this is likely the case. Btw. a check for populated_zone or
> >for_each_populated_zone would suite much better.
> >
> 
> Hmm... maybe not exact.
> 
>     populated_zone checks zone->present_pages
>     managed_zone checks zone->managed_pages
> 
> As the comment of managed_zone says, this one records the pages managed
> by buddy system. And when we look at the usage of totalreserve_pages, it
> is only used in page allocation. And finally, *max* is checked with
> managed_pages instead of present_pages.
> 
> Because of this, managed_zone is more accurate at this place. Is my
> understanding correct?

OK, fair enough. There is a certain discrepancy here. You are right that
we do not care about pages out of the page allocator scope (e.g. early
bootmem allocations, struct pages) but this is likely what other callers
of populated_zone are looking for as well. It seems that managed pages
counter which only came in later was not considered in other places.

That being said this asks for a cleanup of some sort. And I think such a
cleanup wold be appreciated much more than an optimization of an unknown
effect and wonder why this check is used here and not at other places.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Wei Yang]

On Tue, Nov 13, 2018 at 09:08:34AM +0100, Michal Hocko wrote:
>On Tue 13-11-18 01:39:42, Wei Yang wrote:
>> On Mon, Nov 12, 2018 at 03:40:20PM +0100, Michal Hocko wrote:
>> >On Mon 12-11-18 14:26:41, Wei Yang wrote:
>> >> On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote:
>> >> >On Mon 12-11-18 15:14:04, Wei Yang wrote:
>> >> >> Zone with no managed_pages doesn't contribute totalreserv_pages. And the
>> >> >> more nodes we have, the more empty zones there are.
>> >> >> 
>> >> >> This patch skip the zones to save some cycles.
>> >> >
>> >> >What is the motivation for the patch? Does it really cause any
>> >> >measurable difference in performance?
>> >> >
>> >> 
>> >> The motivation here is to reduce some unnecessary work.
>> >
>> >I have guessed so even though the changelog was quite modest on the
>> >motivation.
>> >
>> >> Based on my understanding, almost every node has empty zones, since
>> >> zones within a node are ordered in monotonic increasing memory address.
>> >
>> >Yes, this is likely the case. Btw. a check for populated_zone or
>> >for_each_populated_zone would suite much better.
>> >
>> 
>> Hmm... maybe not exact.
>> 
>>     populated_zone checks zone->present_pages
>>     managed_zone checks zone->managed_pages
>> 
>> As the comment of managed_zone says, this one records the pages managed
>> by buddy system. And when we look at the usage of totalreserve_pages, it
>> is only used in page allocation. And finally, *max* is checked with
>> managed_pages instead of present_pages.
>> 
>> Because of this, managed_zone is more accurate at this place. Is my
>> understanding correct?
>
>OK, fair enough. There is a certain discrepancy here. You are right that
>we do not care about pages out of the page allocator scope (e.g. early
>bootmem allocations, struct pages) but this is likely what other callers
>of populated_zone are looking for as well. It seems that managed pages
>counter which only came in later was not considered in other places.
>
>That being said this asks for a cleanup of some sort. And I think such a
>cleanup wold be appreciated much more than an optimization of an unknown
>effect and wonder why this check is used here and not at other places.

You are right. There are three pages(spanned, managed, present) in a
zone, which is a little confusing.

So you are willing to get rid of present_pages, if I am right?

>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Michal Hocko]

On Tue 13-11-18 08:16:44, Wei Yang wrote:
> On Tue, Nov 13, 2018 at 09:08:34AM +0100, Michal Hocko wrote:
> >On Tue 13-11-18 01:39:42, Wei Yang wrote:
> >> On Mon, Nov 12, 2018 at 03:40:20PM +0100, Michal Hocko wrote:
> >> >On Mon 12-11-18 14:26:41, Wei Yang wrote:
> >> >> On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote:
> >> >> >On Mon 12-11-18 15:14:04, Wei Yang wrote:
> >> >> >> Zone with no managed_pages doesn't contribute totalreserv_pages. And the
> >> >> >> more nodes we have, the more empty zones there are.
> >> >> >> 
> >> >> >> This patch skip the zones to save some cycles.
> >> >> >
> >> >> >What is the motivation for the patch? Does it really cause any
> >> >> >measurable difference in performance?
> >> >> >
> >> >> 
> >> >> The motivation here is to reduce some unnecessary work.
> >> >
> >> >I have guessed so even though the changelog was quite modest on the
> >> >motivation.
> >> >
> >> >> Based on my understanding, almost every node has empty zones, since
> >> >> zones within a node are ordered in monotonic increasing memory address.
> >> >
> >> >Yes, this is likely the case. Btw. a check for populated_zone or
> >> >for_each_populated_zone would suite much better.
> >> >
> >> 
> >> Hmm... maybe not exact.
> >> 
> >>     populated_zone checks zone->present_pages
> >>     managed_zone checks zone->managed_pages
> >> 
> >> As the comment of managed_zone says, this one records the pages managed
> >> by buddy system. And when we look at the usage of totalreserve_pages, it
> >> is only used in page allocation. And finally, *max* is checked with
> >> managed_pages instead of present_pages.
> >> 
> >> Because of this, managed_zone is more accurate at this place. Is my
> >> understanding correct?
> >
> >OK, fair enough. There is a certain discrepancy here. You are right that
> >we do not care about pages out of the page allocator scope (e.g. early
> >bootmem allocations, struct pages) but this is likely what other callers
> >of populated_zone are looking for as well. It seems that managed pages
> >counter which only came in later was not considered in other places.
> >
> >That being said this asks for a cleanup of some sort. And I think such a
> >cleanup wold be appreciated much more than an optimization of an unknown
> >effect and wonder why this check is used here and not at other places.
> 
> You are right. There are three pages(spanned, managed, present) in a
> zone, which is a little confusing.
> 
> So you are willing to get rid of present_pages, if I am right?

No, I believe we want all three of them. But reviewing
for_each_populated_zone users and explicit checks for present/managed
pages and unify them would be a step forward both a more optimal code
and more maintainable code. I haven't checked but
for_each_populated_zone would seem like a proper user for managed page
counter. But that really requires to review all current users.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Wei Yang]

On Tue, Nov 13, 2018 at 10:07:58AM +0100, Michal Hocko wrote:
>On Tue 13-11-18 08:16:44, Wei Yang wrote:
>> On Tue, Nov 13, 2018 at 09:08:34AM +0100, Michal Hocko wrote:
>> >On Tue 13-11-18 01:39:42, Wei Yang wrote:
>> >> On Mon, Nov 12, 2018 at 03:40:20PM +0100, Michal Hocko wrote:
>> >> >On Mon 12-11-18 14:26:41, Wei Yang wrote:
>> >> >> On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote:
>> >> >> >On Mon 12-11-18 15:14:04, Wei Yang wrote:
>> >> >> >> Zone with no managed_pages doesn't contribute totalreserv_pages. And the
>> >> >> >> more nodes we have, the more empty zones there are.
>> >> >> >> 
>> >> >> >> This patch skip the zones to save some cycles.
>> >> >> >
>> >> >> >What is the motivation for the patch? Does it really cause any
>> >> >> >measurable difference in performance?
>> >> >> >
>> >> >> 
>> >> >> The motivation here is to reduce some unnecessary work.
>> >> >
>> >> >I have guessed so even though the changelog was quite modest on the
>> >> >motivation.
>> >> >
>> >> >> Based on my understanding, almost every node has empty zones, since
>> >> >> zones within a node are ordered in monotonic increasing memory address.
>> >> >
>> >> >Yes, this is likely the case. Btw. a check for populated_zone or
>> >> >for_each_populated_zone would suite much better.
>> >> >
>> >> 
>> >> Hmm... maybe not exact.
>> >> 
>> >>     populated_zone checks zone->present_pages
>> >>     managed_zone checks zone->managed_pages
>> >> 
>> >> As the comment of managed_zone says, this one records the pages managed
>> >> by buddy system. And when we look at the usage of totalreserve_pages, it
>> >> is only used in page allocation. And finally, *max* is checked with
>> >> managed_pages instead of present_pages.
>> >> 
>> >> Because of this, managed_zone is more accurate at this place. Is my
>> >> understanding correct?
>> >
>> >OK, fair enough. There is a certain discrepancy here. You are right that
>> >we do not care about pages out of the page allocator scope (e.g. early
>> >bootmem allocations, struct pages) but this is likely what other callers
>> >of populated_zone are looking for as well. It seems that managed pages
>> >counter which only came in later was not considered in other places.
>> >
>> >That being said this asks for a cleanup of some sort. And I think such a
>> >cleanup wold be appreciated much more than an optimization of an unknown
>> >effect and wonder why this check is used here and not at other places.
>> 
>> You are right. There are three pages(spanned, managed, present) in a
>> zone, which is a little confusing.
>> 
>> So you are willing to get rid of present_pages, if I am right?
>
>No, I believe we want all three of them. But reviewing
>for_each_populated_zone users and explicit checks for present/managed
>pages and unify them would be a step forward both a more optimal code
>and more maintainable code. I haven't checked but
>for_each_populated_zone would seem like a proper user for managed page
>counter. But that really requires to review all current users.

Got your point. Let me take a look to see if I could make a cleanup.

>
>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Wei Yang]

On Tue, Nov 13, 2018 at 10:07:58AM +0100, Michal Hocko wrote:
>On Tue 13-11-18 08:16:44, Wei Yang wrote:
>
>No, I believe we want all three of them. But reviewing
>for_each_populated_zone users and explicit checks for present/managed
>pages and unify them would be a step forward both a more optimal code
>and more maintainable code. I haven't checked but
>for_each_populated_zone would seem like a proper user for managed page
>counter. But that really requires to review all current users.
>

To sync with your purpose, I searched the user of
for_each_populated_zone() and replace it with a new loop
for_each_managed_zone().

Here is a summary of what I have done.

file                          used     changed
----------------------------------------------
arch/s390/mm/page-states.c    1        1
kernel/power/snapshot.c       7        3
mm/highmem.c                  1        1
mm/huge_memory.c              1        0
mm/khugepaged.c               1        1
mm/madvise.c                  1        1
mm/page_alloc.c               8        8
mm/vmstat.c                   5        5

The general idea to replace for_each_populated_zone() with
for_each_populated_zone() is:

   * access zone->freelist
   * access zone pcp
   * access zone_page_state

Is my understanding comply with what you want? 

>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Michal Hocko]

On Wed 14-11-18 07:43:41, Wei Yang wrote:
> On Tue, Nov 13, 2018 at 10:07:58AM +0100, Michal Hocko wrote:
> >On Tue 13-11-18 08:16:44, Wei Yang wrote:
> >
> >No, I believe we want all three of them. But reviewing
> >for_each_populated_zone users and explicit checks for present/managed
> >pages and unify them would be a step forward both a more optimal code
> >and more maintainable code. I haven't checked but
> >for_each_populated_zone would seem like a proper user for managed page
> >counter. But that really requires to review all current users.
> >
> 
> To sync with your purpose, I searched the user of
> for_each_populated_zone() and replace it with a new loop
> for_each_managed_zone().

I do not think we really want a new iterator. Is there any users of
for_each_populated_zone which would be interested in something else than
managed pages?
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Wei Yang]

On Wed, Nov 14, 2018 at 08:48:21AM +0100, Michal Hocko wrote:
>On Wed 14-11-18 07:43:41, Wei Yang wrote:
>> On Tue, Nov 13, 2018 at 10:07:58AM +0100, Michal Hocko wrote:
>> >On Tue 13-11-18 08:16:44, Wei Yang wrote:
>> >
>> >No, I believe we want all three of them. But reviewing
>> >for_each_populated_zone users and explicit checks for present/managed
>> >pages and unify them would be a step forward both a more optimal code
>> >and more maintainable code. I haven't checked but
>> >for_each_populated_zone would seem like a proper user for managed page
>> >counter. But that really requires to review all current users.
>> >
>> 
>> To sync with your purpose, I searched the user of
>> for_each_populated_zone() and replace it with a new loop
>> for_each_managed_zone().
>
>I do not think we really want a new iterator. Is there any users of
>for_each_populated_zone which would be interested in something else than
>managed pages?

Your purpose is replace the populated_zone() in
for_each_populated_zone() with managed_zone()?

If this is the case, most of them is possible. Some places I am not sure
is:

    kernel/power/snapshot.c
    mm/huge_memory.c
    mm/khugepaged.c

For other places, I thinks it is ok to replace it.

>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages()

[Michal Hocko]

On Wed 14-11-18 08:20:47, Wei Yang wrote:
> On Wed, Nov 14, 2018 at 08:48:21AM +0100, Michal Hocko wrote:
> >On Wed 14-11-18 07:43:41, Wei Yang wrote:
> >> On Tue, Nov 13, 2018 at 10:07:58AM +0100, Michal Hocko wrote:
> >> >On Tue 13-11-18 08:16:44, Wei Yang wrote:
> >> >
> >> >No, I believe we want all three of them. But reviewing
> >> >for_each_populated_zone users and explicit checks for present/managed
> >> >pages and unify them would be a step forward both a more optimal code
> >> >and more maintainable code. I haven't checked but
> >> >for_each_populated_zone would seem like a proper user for managed page
> >> >counter. But that really requires to review all current users.
> >> >
> >> 
> >> To sync with your purpose, I searched the user of
> >> for_each_populated_zone() and replace it with a new loop
> >> for_each_managed_zone().
> >
> >I do not think we really want a new iterator. Is there any users of
> >for_each_populated_zone which would be interested in something else than
> >managed pages?
> 
> Your purpose is replace the populated_zone() in
> for_each_populated_zone() with managed_zone()?

Well, we might rename as well but I if we have only one or two users
then an opencoded variant with populated_zone() check sounds better than
a new iterator.

> If this is the case, most of them is possible. Some places I am not sure
> is:
> 
>     kernel/power/snapshot.c

This one really looks like it wants the full pfn range whether it is
managed or not. So changing this to opencoded for_each_zone + populated_zone
check should be OK.

>     mm/huge_memory.c
>     mm/khugepaged.c

These two are definitely page allocator related so they do care about
managed.

-- 
Michal Hocko
SUSE Labs

[PATCH] mm, page_alloc: skip to set lowmem_reserve[] for empty zones

[Wei Yang]

lowmem_reserve[] is used to make sure to keep some memory when
allocating memory for a higher zone. In case one zone is empty, no
managed_pages, this zone will never picked up by page allocator. Which
means its lowmem_reserve[] is never used.

Also, since its managed_pages is 0, it will not contribute to lower
zone's lowmem_reserve[] in case there is non empty lower zone.

This patch skip the zones to save some cycles.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a919ba5cb3c8..495feff1e5e4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7291,6 +7291,9 @@ static void setup_per_zone_lowmem_reserve(void)
 				idx--;
 				lower_zone = pgdat->node_zones + idx;
 
+				if (!lower_zone->managed_pages)
+					continue;
+
 				if (sysctl_lowmem_reserve_ratio[idx] < 1) {
 					sysctl_lowmem_reserve_ratio[idx] = 0;
 					lower_zone->lowmem_reserve[j] = 0;
-- 
2.15.1