diff mbox series

Revert "mm/vmstat.c: do not show lowmem reserve protection information of empty zone"

Message ID 20200811075412.12872-1-bhe@redhat.com (mailing list archive)
State New, archived
Headers show
Series Revert "mm/vmstat.c: do not show lowmem reserve protection information of empty zone" | expand

Commit Message

Baoquan He Aug. 11, 2020, 7:54 a.m. UTC
This reverts commit 26e7deadaae1755faf1f6d1a68988c4b8348df59.

Sonny reported that one of their tests started failing on the latest
kernel on their Chrome OS platform. The root cause is that the above
commit removed the protection line of empty zone, while the parser used
in the test relies on the protection line to mark the end of each zone.

Let's revert it to avoid breaking userspace testing or applications.

Cc: <stable@vger.kernel.org> # 5.8.x
Reported-by: Sonny Rao <sonnyrao@chromium.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
 mm/vmstat.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

David Hildenbrand Aug. 11, 2020, 8:34 a.m. UTC | #1
On 11.08.20 09:54, Baoquan He wrote:
> This reverts commit 26e7deadaae1755faf1f6d1a68988c4b8348df59.
> 
> Sonny reported that one of their tests started failing on the latest
> kernel on their Chrome OS platform. The root cause is that the above
> commit removed the protection line of empty zone, while the parser used
> in the test relies on the protection line to mark the end of each zone.
> 
> Let's revert it to avoid breaking userspace testing or applications.
> 
> Cc: <stable@vger.kernel.org> # 5.8.x
> Reported-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  mm/vmstat.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 5b35c0eb8c57..62376dc66ec0 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1644,12 +1644,6 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
>  		   zone->present_pages,
>  		   zone_managed_pages(zone));
>  
> -	/* If unpopulated, no other information is useful */
> -	if (!populated_zone(zone)) {
> -		seq_putc(m, '\n');
> -		return;
> -	}
> -
>  	seq_printf(m,
>  		   "\n        protection: (%ld",
>  		   zone->lowmem_reserve[0]);
> @@ -1657,6 +1651,12 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
>  		seq_printf(m, ", %ld", zone->lowmem_reserve[i]);
>  	seq_putc(m, ')');
>  
> +	/* If unpopulated, no other information is useful */
> +	if (!populated_zone(zone)) {
> +		seq_putc(m, '\n');
> +		return;
> +	}
> +
>  	for (i = 0; i < NR_VM_ZONE_STAT_ITEMS; i++)
>  		seq_printf(m, "\n      %-12s %lu", zone_stat_name(i),
>  			   zone_page_state(zone, i));
> 

Reviewed-by: David Hildenbrand <david@redhat.com>
David Rientjes Aug. 11, 2020, 9:46 p.m. UTC | #2
On Tue, 11 Aug 2020, Baoquan He wrote:

> This reverts commit 26e7deadaae1755faf1f6d1a68988c4b8348df59.
> 
> Sonny reported that one of their tests started failing on the latest
> kernel on their Chrome OS platform. The root cause is that the above
> commit removed the protection line of empty zone, while the parser used
> in the test relies on the protection line to mark the end of each zone.
> 
> Let's revert it to avoid breaking userspace testing or applications.
> 

Acked-by: David Rientjes <rientjes@google.com>

No objection since I noted userspace parsing as a potential risk in 
https://marc.info/?l=linux-mm&m=158507790824995 but I didn't catch this 
earlier patch which could have led to the same thing.
diff mbox series

Patch

diff --git a/mm/vmstat.c b/mm/vmstat.c
index 5b35c0eb8c57..62376dc66ec0 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1644,12 +1644,6 @@  static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
 		   zone->present_pages,
 		   zone_managed_pages(zone));
 
-	/* If unpopulated, no other information is useful */
-	if (!populated_zone(zone)) {
-		seq_putc(m, '\n');
-		return;
-	}
-
 	seq_printf(m,
 		   "\n        protection: (%ld",
 		   zone->lowmem_reserve[0]);
@@ -1657,6 +1651,12 @@  static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
 		seq_printf(m, ", %ld", zone->lowmem_reserve[i]);
 	seq_putc(m, ')');
 
+	/* If unpopulated, no other information is useful */
+	if (!populated_zone(zone)) {
+		seq_putc(m, '\n');
+		return;
+	}
+
 	for (i = 0; i < NR_VM_ZONE_STAT_ITEMS; i++)
 		seq_printf(m, "\n      %-12s %lu", zone_stat_name(i),
 			   zone_page_state(zone, i));