mm/show_mem: Optimize si_meminfo_node by reducing redundant code

[PATCH] mm/show_mem: Optimize si_meminfo_node by reducing redundant code

Posted by Ye Liu 8 months, 3 weeks ago

From: Ye Liu <liuye@kylinos.cn>

Refactors the si_meminfo_node() function by reducing redundant code and
improving readability.

Moved the calculation of managed_pages inside the existing loop that
processes pgdat->node_zones, eliminating the need for a separate loop.

Simplified the logic by removing unnecessary preprocessor conditionals.

Ensured that both totalram, totalhigh, and other memory statistics are
consistently set without duplication.

This change results in cleaner and more efficient code without altering
functionality.

Signed-off-by: Ye Liu <liuye@kylinos.cn>
---
 mm/show_mem.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/mm/show_mem.c b/mm/show_mem.c
index 6af13bcd2ab3..ad373b4b6e39 100644
--- a/mm/show_mem.c
+++ b/mm/show_mem.c
@@ -94,26 +94,20 @@ 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;
 }
 #endif
-- 
2.25.1

Re: [PATCH] mm/show_mem: Optimize si_meminfo_node by reducing redundant code

Posted by Harry Yoo 8 months, 3 weeks ago

On Tue, Mar 25, 2025 at 03:38:03PM +0800, Ye Liu wrote:
> From: Ye Liu <liuye@kylinos.cn>
> 
> Refactors the si_meminfo_node() function by reducing redundant code and
> improving readability.
> 
> Moved the calculation of managed_pages inside the existing loop that
> processes pgdat->node_zones, eliminating the need for a separate loop.
> 
> Simplified the logic by removing unnecessary preprocessor conditionals.
> 
> Ensured that both totalram, totalhigh, and other memory statistics are
> consistently set without duplication.
> 
> This change results in cleaner and more efficient code without altering
> functionality.
> 
> Signed-off-by: Ye Liu <liuye@kylinos.cn>
> ---

Looks good to me,
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

>  mm/show_mem.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/show_mem.c b/mm/show_mem.c
> index 6af13bcd2ab3..ad373b4b6e39 100644
> --- a/mm/show_mem.c
> +++ b/mm/show_mem.c
> @@ -94,26 +94,20 @@ 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;
>  }
>  #endif
> -- 
> 2.25.1
> 
> 

-- 
Cheers,
Harry (formerly known as Hyeonggon)

Re: [PATCH] mm/show_mem: Optimize si_meminfo_node by reducing redundant code

Posted by Mike Rapoport 8 months, 3 weeks ago

On Tue, Mar 25, 2025 at 03:38:03PM +0800, Ye Liu wrote:
> From: Ye Liu <liuye@kylinos.cn>
> 
> Refactors the si_meminfo_node() function by reducing redundant code and
> improving readability.
> 
> Moved the calculation of managed_pages inside the existing loop that
> processes pgdat->node_zones, eliminating the need for a separate loop.
> 
> Simplified the logic by removing unnecessary preprocessor conditionals.
> 
> Ensured that both totalram, totalhigh, and other memory statistics are
> consistently set without duplication.
> 
> This change results in cleaner and more efficient code without altering
> functionality.
> 
> Signed-off-by: Ye Liu <liuye@kylinos.cn>
> ---
>  mm/show_mem.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/show_mem.c b/mm/show_mem.c
> index 6af13bcd2ab3..ad373b4b6e39 100644
> --- a/mm/show_mem.c
> +++ b/mm/show_mem.c
> @@ -94,26 +94,20 @@ 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);

nit: don't remove the empty line after the declaration

>  		if (is_highmem(zone)) {
>  			managed_highpages += zone_managed_pages(zone);

Looks like highmem pages get counted twice, no?

>  			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;
>  }
>  #endif
> -- 
> 2.25.1
> 
> 

-- 
Sincerely yours,
Mike.