On Wed, 25 Mar 2020, Baoquan He wrote:
> > Even this can break existing parsers. Fixing that up is likely not hard
> > and existing parsers would be mostly debugging hacks here and there but
> > I do miss any actual justification except for you considering it more
> > sensible. I do not remember this would be a common pain point for people
> > parsing this file. If anything the overal structure of the file makes it
> > hard to parse and your patches do not really address that. We are likely
> > too late to make the output much more sensible TBH.
> >
> > That being said, I haven't looked more closely on your patches because I
> > do not have spare cycles for that. Your justification for touching the
> > code which seems to be working relatively well is quite weak IMHO, yet
> > it adds a non zero risk for breaking existing parsers.
>
> I would take the saying of non zero risk for breaking existing parsers.
> When considering this change, I thought about the possible risk. However,
> found out the per-node stats was added in 2016 which is not so late, and
> assume nobody will rely on the order of per-node stats embeded into a
> zone. But I have to admit any concern or worry of risk is worth being
> considerred carefully since /proc/zoneinfo is a classic interface.
>
For context, we started parsing /proc/zoneinfo in initscripts to be able
to determine the order in which vm.lowmem_reserve_ratio needs to be set
and this required my kernel change from 2017:
commit b2bd8598195f1b2a72130592125ac6b4218988a2
Author: David Rientjes <rientjes@xxxxxxxxxx>
Date: Wed May 3 14:52:59 2017 -0700
mm, vmstat: print non-populated zones in zoneinfo
Otherwise, we found, it's much more difficult to determine how this array
should be structured. So at least we parse this file very carefully, I'm
not sure how much others do, but it seems like an unnecessary risk for
little reward. I'm happy to see it has been decided to drop this patch
and patch 5.
> So, in view of objections from you and David, I would like to drop this
> patch and patch 5. It's a small improvement, not worth taking any risk.
> But if it goes back to this time of 2017, I would like to spend some
> time to defend it :-)
>
> commit e2ecc8a79ed49f7838b4fdf352c4c48cec9424ac
> Author: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
> Date: Thu Jul 28 15:47:02 2016 -0700
>
> mm, vmstat: print node-based stats in zoneinfo file
>
>
>
