Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 25 Mar 2020 13:53:31 +0800
From:   Baoquan He <bhe@redhat.com>
To:     David Rientjes <rientjes@google.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
Message-ID: <20200325055331.GB9942@MiWiFi-R3L-srv>
References: <20200324142229.12028-1-bhe@redhat.com>
 <20200324142229.12028-5-bhe@redhat.com>
 <alpine.DEB.2.21.2003241220360.34058@chino.kir.corp.google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.21.2003241220360.34058@chino.kir.corp.google.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 03/24/20 at 12:25pm, David Rientjes wrote:
> 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.

Oh, I may not describe the change and result clearly. This patch doesn't
change zone order at all.  I only move the per-node stats to the front of
each node, the zone order is completely kept the same, still DMA, DMA32,
NORMAL, MOVABLE.

Before this patch, per-node stats are printed inside the first populated
zone of each node. E.g in this node 2 which only has movable zone, the
per-node stats is embedded in the last zone. In fact, this per-node stats
are made for the whole node, but not for one zone. 

Node 2, zone      DMA
  pages free     0
        min      0
        low      0
        high     0
        spanned  0
        present  0
        managed  0
        protection: (0, 0, 0, 1024, 1024)
Node 2, zone    DMA32
  pages free     0
        min      0
        low      0
        high     0
        spanned  0
        present  0
        managed  0
        protection: (0, 0, 0, 1024, 1024)
Node 2, zone   Normal
  pages free     0
        min      0
        low      0
        high     0
        spanned  0
        present  0
        managed  0
        protection: (0, 0, 0, 8192, 8192)
Node 2, zone  Movable
  per-node stats              --------------------------->> start of per-node stats
      nr_inactive_anon 42
      nr_active_anon 11787
      nr_inactive_file 32222
      nr_active_file 6081
      nr_unevictable 0
      nr_slab_reclaimable 0
      nr_slab_unreclaimable 0
      ......                  --------- (mid items are omitted)
      nr_anon_transparent_hugepages 0
      nr_unstable  0
      nr_vmscan_write 0
      nr_vmscan_immediate_reclaim 0
      nr_dirtied   25523
      nr_written   25113
      nr_kernel_misc_reclaimable 0  ------------------------->> end of per-node stats
  pages free     211331             ------------------------->> start printing data of zone Movable
        min      3524
        low      4405
        high     5286
        spanned  262144
        present  262144
        managed  262144
        protection: (0, 0, 0, 0, 0)
      nr_free_pages 211331
      nr_zone_inactive_anon 42
      nr_zone_active_anon 11787
      nr_zone_inactive_file 32222
      nr_zone_active_file 6081
      nr_zone_unevictable 0
      nr_zone_write_pending 2

With this patch applied, only the per-node stats part is moved to the
front of each node.

Node 2, per-node stats             --------------------------->> start of per-node stats
      nr_inactive_anon 42
      nr_active_anon 12358
      nr_inactive_file 33139
      nr_active_file 10088
      nr_unevictable 0
      nr_slab_reclaimable 0
      ......                  --------- (mid items are omitted)
      nr_vmscan_write 0
      nr_vmscan_immediate_reclaim 0
      nr_dirtied   9082
      nr_written   8844
      nr_kernel_misc_reclaimable 0
      nr_foll_pin_acquired 0
      nr_foll_pin_released 0     ------------------------->> end of per-node stats
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     205601     ------------------------->> start printing data of zone Movable
        min      3525
        low      4406
        high     5287
        spanned  262144
        present  262144
        managed  262144
        protection: (0, 0, 0, 0)
      nr_free_pages 205601
      nr_zone_inactive_anon 42
      nr_zone_active_anon 12358
      ........