| Message ID | 20250325073803.852594-1-ye.liu@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/show_mem: Optimize si_meminfo_node by reducing redundant code | expand |
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 > >
在 2025/3/27 03:29, Mike Rapoport 写道: > 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 Agree. > >> if (is_highmem(zone)) { >> managed_highpages += zone_managed_pages(zone); > Looks like highmem pages get counted twice, no? twice? The original logic has not been changed. managed_highpages is just high memory, managed_pages is all. I don't understand what you mean. Thanks, Ye Liu > >> 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 >> >>
Please don't send html emails to the Linux kernel mailing lists. On Thu, Mar 27, 2025 at 05:32:14PM +0800, Ye Liu wrote: > > 在 2025/3/27 03:29, Mike Rapoport 写道: > > 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 > > Agree. > > if (is_highmem(zone)) { > managed_highpages += zone_managed_pages(zone); > > Looks like highmem pages get counted twice, no? > > twice? The original logic has not been changed. > > managed_highpages is just high memory, managed_pages is all. You are right, I misread the patch. Feel free to add Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org> > Thanks, > > Ye Liu > > > > 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 > > >
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