From: David Rientjes <rientjes@google.com>
To: Baoquan He <bhe@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
akpm@linux-foundation.org, iamjoonsoo.kim@lge.com,
hannes@cmpxchg.org, mhocko@kernel.org, vbabka@suse.cz
Subject: Re: [PATCH 4/5] mm/vmstat.c: move the per-node stats to the front of /proc/zoneinfo
Date: Tue, 24 Mar 2020 12:25:03 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.2.21.2003241220360.34058@chino.kir.corp.google.com> (raw)
In-Reply-To: <20200324142229.12028-5-bhe@redhat.com>
On Tue, 24 Mar 2020, Baoquan He wrote:
> This moving makes the layout of /proc/zoneinfo more sensible. And there
> are 4 zones at most currently, it doesn't need to scroll down much to get
> to the 1st populated zone, even though the 1st populated zone is MOVABLE
> zone.
>
Doesn't this introduce risk that it will break existing parsers of
/proc/zoneinfo in subtle ways?
In some cases /proc/zoneinfo is a tricky file to correctly parse because
you have to rely on the existing order in which it is printed to determine
which zone is being described. We need to print zones even with unmanaged
pages, for instance, otherwise userspace may be unaware of which zones are
supported and what order they are in. That's important to be able to
construct the proper string to use when writing vm.lowmem_reserve_ratio.
I'd prefer not changing the order of /proc/zoneinfo if it can be avoided
just because the risk outweighs the reward that we may break some
initscript parsers.
> Node 2, per-node stats
> nr_inactive_anon 48
> nr_active_anon 15454
> ...
> nr_foll_pin_acquired 0
> nr_foll_pin_released 0
> Node 2, zone DMA
> pages free 0
> min 0
> low 0
> high 0
> spanned 0
> present 0
> managed 0
> Node 2, zone DMA32
> pages free 0
> min 0
> low 0
> high 0
> spanned 0
> present 0
> managed 0
> Node 2, zone Normal
> pages free 0
> min 0
> low 0
> high 0
> spanned 0
> present 0
> managed 0
> Node 2, zone Movable
> pages free 196346
> min 3540
> ...
> managed 262144
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
> mm/vmstat.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 6fd1407f4632..4bbf9be786da 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1567,13 +1567,6 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
> {
> int i;
> seq_printf(m, "Node %d, zone %8s", pgdat->node_id, zone->name);
> - if (is_zone_first_populated(pgdat, zone)) {
> - seq_printf(m, "\n per-node stats");
> - for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++) {
> - seq_printf(m, "\n %-12s %lu", node_stat_name(i),
> - node_page_state(pgdat, i));
> - }
> - }
> seq_printf(m,
> "\n pages free %lu"
> "\n min %lu"
> @@ -1648,7 +1641,18 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
> */
> static int zoneinfo_show(struct seq_file *m, void *arg)
> {
> + int i;
> pg_data_t *pgdat = (pg_data_t *)arg;
> +
> + if (node_state(pgdat->node_id, N_MEMORY)) {
> + seq_printf(m, "Node %d, per-node stats", pgdat->node_id);
> + for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++) {
> + seq_printf(m, "\n %-12s %lu", node_stat_name(i),
> + node_page_state(pgdat, i));
> + }
> + seq_putc(m, '\n');
> + }
> +
> walk_zones_in_node(m, pgdat, false, false, zoneinfo_show_print);
> return 0;
> }
> --
> 2.17.2
>
>
>
next prev parent reply other threads:[~2020-03-24 19:25 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-24 14:22 [PATCH 0/5] improvements about lowmem_reserve and /proc/zoneinfo Baoquan He
2020-03-24 14:22 ` [PATCH 1/5] mm/page_alloc.c: only tune sysctl_lowmem_reserve_ratio value once when changing it Baoquan He
2020-03-24 14:22 ` [PATCH 2/5] mm/page_alloc.c: clear out zone->lowmem_reserve[] if the zone is empty Baoquan He
2020-03-24 14:22 ` [PATCH 3/5] mm/vmstat.c: do not show lowmem reserve protection information of empty zone Baoquan He
2020-03-24 14:22 ` [PATCH 4/5] mm/vmstat.c: move the per-node stats to the front of /proc/zoneinfo Baoquan He
2020-03-24 19:25 ` David Rientjes [this message]
2020-03-25 5:53 ` Baoquan He
2020-03-25 8:55 ` Michal Hocko
2020-03-25 14:23 ` Baoquan He
2020-03-25 19:45 ` David Rientjes
2020-03-26 4:24 ` Baoquan He
2020-03-26 6:43 ` Michal Hocko
2020-03-26 11:22 ` Baoquan He
2020-03-24 14:22 ` [PATCH 5/5] mm/vmstat.c: remove the useless code Baoquan He
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.2003241220360.34058@chino.kir.corp.google.com \
--to=rientjes@google.com \
--cc=akpm@linux-foundation.org \
--cc=bhe@redhat.com \
--cc=hannes@cmpxchg.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=vbabka@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).