[PATCH 0/3] mm: random cleanups

[PATCH 0/3] mm: random cleanups

[Luis Chamberlain]

Here's a few random minor cleanups I spotted while reviewing a few mm/*.c files.
No functional changes.

Luis Chamberlain (3):
  mm/show_mem: simplify ifdef on si_meminfo_node()
  mm/compaction: add and use for_each_populated_zone_pgdat() helper
  mm/vmstat: simplfy extfrag_show_print with fragmentation_index()

 include/linux/mmzone.h |  8 ++++++++
 mm/compaction.c        |  9 ++-------
 mm/show_mem.c          |  5 +----
 mm/vmstat.c            | 11 +++++------
 4 files changed, 16 insertions(+), 17 deletions(-)

-- 
2.43.0

[PATCH 1/3] mm/show_mem: simplify ifdef on si_meminfo_node()

[Luis Chamberlain]

There is an ifdef where the same exact values are used at
the end, so remove the else and keep the same values for both
conditions.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 mm/show_mem.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/mm/show_mem.c b/mm/show_mem.c
index 8dcfafbd283c..529d2f9b6410 100644
--- a/mm/show_mem.c
+++ b/mm/show_mem.c
@@ -108,12 +108,9 @@ void si_meminfo_node(struct sysinfo *val, int nid)
 			free_highpages += zone_page_state(zone, NR_FREE_PAGES);
 		}
 	}
+#endif
 	val->totalhigh = managed_highpages;
 	val->freehigh = free_highpages;
-#else
-	val->totalhigh = managed_highpages;
-	val->freehigh = free_highpages;
-#endif
 	val->mem_unit = PAGE_SIZE;
 }
 #endif
-- 
2.43.0

Re: [PATCH 1/3] mm/show_mem: simplify ifdef on si_meminfo_node()

[Kefeng Wang]

On 2024/3/14 8:54, Luis Chamberlain wrote:
> There is an ifdef where the same exact values are used at
> the end, so remove the else and keep the same values for both
> conditions.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   mm/show_mem.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/mm/show_mem.c b/mm/show_mem.c
> index 8dcfafbd283c..529d2f9b6410 100644
> --- a/mm/show_mem.c
> +++ b/mm/show_mem.c
> @@ -108,12 +108,9 @@ void si_meminfo_node(struct sysinfo *val, int nid)
>   			free_highpages += zone_page_state(zone, NR_FREE_PAGES);
>   		}
>   	}
> +#endif
>   	val->totalhigh = managed_highpages;
>   	val->freehigh = free_highpages;
> -#else
> -	val->totalhigh = managed_highpages;
> -	val->freehigh = free_highpages;
> -#endif
>   	val->mem_unit = PAGE_SIZE;
>   }
>   #endif

More cleanup?
diff --git a/mm/show_mem.c b/mm/show_mem.c
index 8dcfafbd283c..44dd6d6e9488 100644
--- a/mm/show_mem.c
+++ b/mm/show_mem.c
@@ -94,26 +94,21 @@ void si_meminfo_node(struct sysinfo *val, int nid)
         unsigned long free_highpages = 0;
         pg_data_t *pgdat = NODE_DATA(nid);

-       for (zone_type = 0; zone_type < MAX_NR_ZONES; zone_type++)
-               managed_pages += 
zone_managed_pages(&pgdat->node_zones[zone_type]);
-       val->totalram = managed_pages;
-       val->sharedram = node_page_state(pgdat, NR_SHMEM);
-       val->freeram = sum_zone_node_page_state(nid, NR_FREE_PAGES);
-#ifdef CONFIG_HIGHMEM
         for (zone_type = 0; zone_type < MAX_NR_ZONES; zone_type++) {
                 struct zone *zone = &pgdat->node_zones[zone_type];

+               managed_pages += zone_managed_pages(zone);
                 if (is_highmem(zone)) {
                         managed_highpages += zone_managed_pages(zone);
                         free_highpages += zone_page_state(zone, 
NR_FREE_PAGES);
                 }
         }
+
+       val->totalram = managed_pages;
+       val->sharedram = node_page_state(pgdat, NR_SHMEM);
+       val->freeram = sum_zone_node_page_state(nid, NR_FREE_PAGES);
         val->totalhigh = managed_highpages;
         val->freehigh = free_highpages;
-#else
-       val->totalhigh = managed_highpages;
-       val->freehigh = free_highpages;
-#endif
         val->mem_unit = PAGE_SIZE;
  }

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

[Luis Chamberlain]

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@kernel.org>
---
 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
+
 static inline struct zone *zonelist_zone(struct zoneref *zoneref)
 {
 	return zoneref->zone;
diff --git a/mm/compaction.c b/mm/compaction.c
index b961db601df4..023a09d0786d 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2151,16 +2151,11 @@ static unsigned int fragmentation_score_zone_weighted(struct zone *zone)
 static unsigned int fragmentation_score_node(pg_data_t *pgdat)
 {
 	unsigned int score = 0;
+	struct zone *zone;
 	int zoneid;
 
-	for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) {
-		struct zone *zone;
-
-		zone = &pgdat->node_zones[zoneid];
-		if (!populated_zone(zone))
-			continue;
+	for_each_populated_zone_pgdat(zone, pgdat)
 		score += fragmentation_score_zone_weighted(zone);
-	}
 
 	return score;
 }
-- 
2.43.0

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

[Baolin Wang]

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@kernel.org>
> ---
>   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.

>   static inline struct zone *zonelist_zone(struct zoneref *zoneref)
>   {
>   	return zoneref->zone;
> diff --git a/mm/compaction.c b/mm/compaction.c
> index b961db601df4..023a09d0786d 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2151,16 +2151,11 @@ static unsigned int fragmentation_score_zone_weighted(struct zone *zone)
>   static unsigned int fragmentation_score_node(pg_data_t *pgdat)
>   {
>   	unsigned int score = 0;
> +	struct zone *zone;
>   	int zoneid;
>   
> -	for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) {
> -		struct zone *zone;
> -
> -		zone = &pgdat->node_zones[zoneid];
> -		if (!populated_zone(zone))
> -			continue;
> +	for_each_populated_zone_pgdat(zone, pgdat)
>   		score += fragmentation_score_zone_weighted(zone);
> -	}
>   
>   	return score;
>   }

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

[Luis Chamberlain]

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@kernel.org>
> > ---
> >   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:

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 34b729fc751b..bd11d33ea14d 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1568,6 +1568,7 @@ static inline struct pglist_data *NODE_DATA(int nid)
 extern struct pglist_data *first_online_pgdat(void);
 extern struct pglist_data *next_online_pgdat(struct pglist_data *pgdat);
 extern struct zone *next_zone(struct zone *zone);
+extern struct zone *next_zone_pgdat(struct zone *zone, struct pglist_data *pgdat);
 
 /**
  * for_each_online_pgdat - helper macro to iterate over all online nodes
@@ -1600,7 +1601,7 @@ extern struct zone *next_zone(struct zone *zone);
 #define for_each_populated_zone_pgdat(zone, pgdat)	\
 	for (zone = pgdat->node_zones;			\
 	     zone;					\
-	     zone = next_zone(zone))			\
+	     zone = next_zone_pgdat(zone, pgdat))	\
 		if (!populated_zone(zone))		\
 			; /* do nothing */		\
 		else
diff --git a/mm/compaction.c b/mm/compaction.c
index 015126803017..96434f6fc1ad 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2152,7 +2152,6 @@ static unsigned int fragmentation_score_node(pg_data_t *pgdat)
 {
 	unsigned int score = 0;
 	struct zone *zone;
-	int zoneid;
 
 	for_each_populated_zone_pgdat(zone, pgdat)
 		score += fragmentation_score_zone_weighted(zone);
diff --git a/mm/mmzone.c b/mm/mmzone.c
index c01896eca736..043a6dc16c05 100644
--- a/mm/mmzone.c
+++ b/mm/mmzone.c
@@ -43,6 +43,18 @@ struct zone *next_zone(struct zone *zone)
 	return zone;
 }
 
+/*
+ * next_zone_pgdat - helper magic for for_each_zone() per node
+ */
+struct zone *next_zone_pgdat(struct zone *zone, struct pglist_data *pgdat)
+{
+	if (!zone || !pgdat)
+		return NULL;
+	if (zone < pgdat->node_zones + MAX_NR_ZONES - 1)
+		return ++zone;
+	return NULL;
+}
+
 static inline int zref_in_nodemask(struct zoneref *zref, nodemask_t *nodes)
 {
 #ifdef CONFIG_NUMA

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

[Baolin Wang]

On 2024/3/15 13: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@kernel.org>
>>> ---
>>>    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:
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 34b729fc751b..bd11d33ea14d 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1568,6 +1568,7 @@ static inline struct pglist_data *NODE_DATA(int nid)
>   extern struct pglist_data *first_online_pgdat(void);
>   extern struct pglist_data *next_online_pgdat(struct pglist_data *pgdat);
>   extern struct zone *next_zone(struct zone *zone);
> +extern struct zone *next_zone_pgdat(struct zone *zone, struct pglist_data *pgdat);
>   
>   /**
>    * for_each_online_pgdat - helper macro to iterate over all online nodes
> @@ -1600,7 +1601,7 @@ extern struct zone *next_zone(struct zone *zone);
>   #define for_each_populated_zone_pgdat(zone, pgdat)	\
>   	for (zone = pgdat->node_zones;			\
>   	     zone;					\
> -	     zone = next_zone(zone))			\
> +	     zone = next_zone_pgdat(zone, pgdat))	\
>   		if (!populated_zone(zone))		\
>   			; /* do nothing */		\
>   		else
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 015126803017..96434f6fc1ad 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2152,7 +2152,6 @@ static unsigned int fragmentation_score_node(pg_data_t *pgdat)
>   {
>   	unsigned int score = 0;
>   	struct zone *zone;
> -	int zoneid;
>   
>   	for_each_populated_zone_pgdat(zone, pgdat)
>   		score += fragmentation_score_zone_weighted(zone);
> diff --git a/mm/mmzone.c b/mm/mmzone.c
> index c01896eca736..043a6dc16c05 100644
> --- a/mm/mmzone.c
> +++ b/mm/mmzone.c
> @@ -43,6 +43,18 @@ struct zone *next_zone(struct zone *zone)
>   	return zone;
>   }
>   
> +/*
> + * next_zone_pgdat - helper magic for for_each_zone() per node
> + */
> +struct zone *next_zone_pgdat(struct zone *zone, struct pglist_data *pgdat)
> +{
> +	if (!zone || !pgdat)
> +		return NULL;

Seems unnecessary, you already accessed them before calling 
next_zone_pgdat(). Otherwise, looks good to me.

> +	if (zone < pgdat->node_zones + MAX_NR_ZONES - 1)
> +		return ++zone;
> +	return NULL;
> +}
> +
>   static inline int zref_in_nodemask(struct zoneref *zref, nodemask_t *nodes)
>   {
>   #ifdef CONFIG_NUMA

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

[Vlastimil Babka]

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@kernel.org>
>> > ---
>> >   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?

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.

> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 34b729fc751b..bd11d33ea14d 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1568,6 +1568,7 @@ static inline struct pglist_data *NODE_DATA(int nid)
>  extern struct pglist_data *first_online_pgdat(void);
>  extern struct pglist_data *next_online_pgdat(struct pglist_data *pgdat);
>  extern struct zone *next_zone(struct zone *zone);
> +extern struct zone *next_zone_pgdat(struct zone *zone, struct pglist_data *pgdat);
>  
>  /**
>   * for_each_online_pgdat - helper macro to iterate over all online nodes
> @@ -1600,7 +1601,7 @@ extern struct zone *next_zone(struct zone *zone);
>  #define for_each_populated_zone_pgdat(zone, pgdat)	\
>  	for (zone = pgdat->node_zones;			\
>  	     zone;					\
> -	     zone = next_zone(zone))			\
> +	     zone = next_zone_pgdat(zone, pgdat))	\
>  		if (!populated_zone(zone))		\
>  			; /* do nothing */		\
>  		else
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 015126803017..96434f6fc1ad 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2152,7 +2152,6 @@ static unsigned int fragmentation_score_node(pg_data_t *pgdat)
>  {
>  	unsigned int score = 0;
>  	struct zone *zone;
> -	int zoneid;
>  
>  	for_each_populated_zone_pgdat(zone, pgdat)
>  		score += fragmentation_score_zone_weighted(zone);
> diff --git a/mm/mmzone.c b/mm/mmzone.c
> index c01896eca736..043a6dc16c05 100644
> --- a/mm/mmzone.c
> +++ b/mm/mmzone.c
> @@ -43,6 +43,18 @@ struct zone *next_zone(struct zone *zone)
>  	return zone;
>  }
>  
> +/*
> + * next_zone_pgdat - helper magic for for_each_zone() per node
> + */
> +struct zone *next_zone_pgdat(struct zone *zone, struct pglist_data *pgdat)
> +{
> +	if (!zone || !pgdat)
> +		return NULL;
> +	if (zone < pgdat->node_zones + MAX_NR_ZONES - 1)
> +		return ++zone;
> +	return NULL;
> +}
> +
>  static inline int zref_in_nodemask(struct zoneref *zref, nodemask_t *nodes)
>  {
>  #ifdef CONFIG_NUMA

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@kernel.org>
> >> > ---
> >> >   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@kernel.org

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

  Luis

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

[kernel test robot]

Hi Luis,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Luis-Chamberlain/mm-show_mem-simplify-ifdef-on-si_meminfo_node/20240314-085917
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240314005436.2962962-3-mcgrof%40kernel.org
patch subject: [PATCH 2/3] mm/compaction: add and use for_each_populated_zone_pgdat() helper
config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240314/202403141931.2XJWI4fa-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240314/202403141931.2XJWI4fa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403141931.2XJWI4fa-lkp@intel.com/

All warnings (new ones prefixed by >>):

   mm/compaction.c: In function 'fragmentation_score_node':
>> mm/compaction.c:2252:13: warning: unused variable 'zoneid' [-Wunused-variable]
    2252 |         int zoneid;
         |             ^~~~~~


vim +/zoneid +2252 mm/compaction.c

  2240	
  2241	/*
  2242	 * The per-node proactive (background) compaction process is started by its
  2243	 * corresponding kcompactd thread when the node's fragmentation score
  2244	 * exceeds the high threshold. The compaction process remains active till
  2245	 * the node's score falls below the low threshold, or one of the back-off
  2246	 * conditions is met.
  2247	 */
  2248	static unsigned int fragmentation_score_node(pg_data_t *pgdat)
  2249	{
  2250		unsigned int score = 0;
  2251		struct zone *zone;
> 2252		int zoneid;
  2253	
  2254		for_each_populated_zone_pgdat(zone, pgdat)
  2255			score += fragmentation_score_zone_weighted(zone);
  2256	
  2257		return score;
  2258	}
  2259	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 3/3] mm/vmstat: simplfy extfrag_show_print with fragmentation_index()

[Luis Chamberlain]

fragmentation_index() already uses the stack for the struct contig_page_info,
so just use that and enhance the documentation for fragmentation_index().

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 mm/vmstat.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/mm/vmstat.c b/mm/vmstat.c
index db79935e4a54..582f89b37ccf 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1129,7 +1129,10 @@ unsigned int extfrag_for_order(struct zone *zone, unsigned int order)
 			info.free_pages);
 }
 
-/* Same as __fragmentation index but allocs contig_page_info on stack */
+/*
+ * Same as __fragmentation index but allocs contig_page_info on stack,
+ * useful when walking a zone as interrupts are disabled.
+ */
 int fragmentation_index(struct zone *zone, unsigned int order)
 {
 	struct contig_page_info info;
@@ -2227,15 +2230,11 @@ static void extfrag_show_print(struct seq_file *m,
 	unsigned int order;
 	int index;
 
-	/* Alloc on stack as interrupts are disabled for zone walk */
-	struct contig_page_info info;
-
 	seq_printf(m, "Node %d, zone %8s ",
 				pgdat->node_id,
 				zone->name);
 	for (order = 0; order < NR_PAGE_ORDERS; ++order) {
-		fill_contig_page_info(zone, order, &info);
-		index = __fragmentation_index(order, &info);
+		index = fragmentation_index(zone, order);
 		seq_printf(m, "%2d.%03d ", index / 1000, index % 1000);
 	}
 
-- 
2.43.0

Re: [PATCH 3/3] mm/vmstat: simplfy extfrag_show_print with fragmentation_index()

[Vlastimil Babka]

On 3/14/24 01:54, Luis Chamberlain wrote:
> fragmentation_index() already uses the stack for the struct contig_page_info,
> so just use that and enhance the documentation for fragmentation_index().
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/vmstat.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index db79935e4a54..582f89b37ccf 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1129,7 +1129,10 @@ unsigned int extfrag_for_order(struct zone *zone, unsigned int order)
>  			info.free_pages);
>  }
>  
> -/* Same as __fragmentation index but allocs contig_page_info on stack */
> +/*
> + * Same as __fragmentation index but allocs contig_page_info on stack,
> + * useful when walking a zone as interrupts are disabled.
> + */
>  int fragmentation_index(struct zone *zone, unsigned int order)
>  {
>  	struct contig_page_info info;
> @@ -2227,15 +2230,11 @@ static void extfrag_show_print(struct seq_file *m,
>  	unsigned int order;
>  	int index;
>  
> -	/* Alloc on stack as interrupts are disabled for zone walk */
> -	struct contig_page_info info;
> -
>  	seq_printf(m, "Node %d, zone %8s ",
>  				pgdat->node_id,
>  				zone->name);
>  	for (order = 0; order < NR_PAGE_ORDERS; ++order) {
> -		fill_contig_page_info(zone, order, &info);
> -		index = __fragmentation_index(order, &info);
> +		index = fragmentation_index(zone, order);
>  		seq_printf(m, "%2d.%03d ", index / 1000, index % 1000);
>  	}
>  

Re: [PATCH 3/3] mm/vmstat: simplfy extfrag_show_print with fragmentation_index()

[Davidlohr Bueso]

On Wed, 13 Mar 2024, Luis Chamberlain wrote:

>fragmentation_index() already uses the stack for the struct contig_page_info,
>so just use that and enhance the documentation for fragmentation_index().
>

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

>Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>