Re: [PATCH 2/3] mm/compaction: add and use for_each_populated_zone_pgdat() helper

[Luis Chamberlain]

On Fri, Mar 15, 2024 at 12:09:29PM +0100, Vlastimil Babka wrote:
> On 3/15/24 06:38, Luis Chamberlain wrote:
> > On Thu, Mar 14, 2024 at 03:19:45PM +0800, Baolin Wang wrote:
> >> 
> >> 
> >> On 2024/3/14 08:54, Luis Chamberlain wrote:
> >> > We can just wrap most of the work done on fragmentation_score_node()
> >> > into a pgdat helper for populated zones. Add the helper and use it.
> >> > 
> >> > Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> >> > ---
> >> >   include/linux/mmzone.h | 8 ++++++++
> >> >   mm/compaction.c        | 9 ++-------
> >> >   2 files changed, 10 insertions(+), 7 deletions(-)
> >> > 
> >> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> >> > index a497f189d988..1fd74c7100ec 100644
> >> > --- a/include/linux/mmzone.h
> >> > +++ b/include/linux/mmzone.h
> >> > @@ -1597,6 +1597,14 @@ extern struct zone *next_zone(struct zone *zone);
> >> >   			; /* do nothing */		\
> >> >   		else
> >> > +#define for_each_populated_zone_pgdat(zone, pgdat)	\
> >> > +	for (zone = pgdat->node_zones;			\
> >> > +	     zone;					\
> >> > +	     zone = next_zone(zone))			\
> >> > +		if (!populated_zone(zone))		\
> >> > +			; /* do nothing */		\
> >> > +		else
> >> 
> >> I think this will break the original logics, since the next_zone() will
> >> iterate over all memory zones, instead of only the memory zones of the
> >> specified node.
> > 
> > Definitely, thanks, so we'd need something like this in addition:
> 
> IMHO that's unnecessarily complex, why not just do the iteration all inline
> without this next_zone_pgdat() helper?

Sure.

> Also maybe you could find more users if you created just a
> for_each_zone_pgdat() and left the populated_zone() in the user? Otherwise
> it's quite a specific helper with just one user.

Indeed, this was the only immediately clear user for_each_populated_zone_pgdat()
which stood out, but I'll look more closely. I added this helper because in an
RFC patch I had another use case:

https://lkml.kernel.org/r/20240314005710.2964798-1-mcgrof@xxxxxxxxxx

Granted this is just an RFC. So happy to drop this if we don't have other users

  Luis