On 08/10/20 at 09:01pm, Andrew Morton wrote: > On Mon, 10 Aug 2020 20:18:11 -0700 Sonny Rao <sonnyrao@xxxxxxxxxxxx> wrote: > > > On Mon, Aug 10, 2020 at 7:37 PM Baoquan He <bhe@xxxxxxxxxx> wrote: > > > > > > On 08/10/20 at 03:46pm, Yu Zhao wrote: > > > > On Mon, Aug 10, 2020 at 02:24:03PM -0700, Sonny Rao wrote: > > > > > We (Chrome OS) noticed recently one of our tests started failing on > > > > > upstream kernels while parsing /proc/zoneinfo > > > > > I think this patch is the cause: > > > > > > > > > > 26e7deadaae17 mm/vmstat.c: do not show lowmem reserve protection > > > > > information of empty zone > > > > > > > > > > Maybe our parser was being overly strict by looking for the protection > > > > > line, and it's not hard to fix but raised the question of whether there's > > > > > any ABI compatibility guarantees about these files? > > > > > > > > According to Documentation/admin-guide/sysctl/vm.rst, "Each zone has > > > > an array of protection pages". I'm not sure if this is the guarantee, > > > > but the doc should reflect the actual format. > > > > > > The current code will list all zones in one memory node, even though > > > that node only has one existing zone. E.g in below node 1, > > > it only has NORMAL zone, but we will list zone DMA, DMA32, MOVABLE, > > > DEVICE which are all empty zone, namely doesn't exist. So, each zone > > > has an array of protection pages, it should not include the nonexistent > > > zone. I thought nobody would check the protection line of an empty zone, > > > seems I was wrong. > > > > > > This particular parser was written as a state machine and that line > > was a convenient thing to look for to mark the end of each zone. > > AFAICT, there's no explicit documentation on the layout of that file > > though. > > This only affects 5.8 and later, yes? > > 26e7deadaae17 didn't gain us much at all and we shouldn't break > userspace unless it's super important. So I think it's best to revert, > and to backport the revert to 5.8.x. > > Could someone (Baoquan He?) please send a patch with a suitable > changelog, cc:stable, reported-by:, etc? OK, I will post a patch to revert this.
