Received-SPF: pass (google.com: domain of linux-kernel+bounces-10826-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Sun, 24 Dec 2023 13:26:15 -0800 (PST)
From: David Rientjes <rientjes@google.com>
To: Pasha Tatashin <pasha.tatashin@soleen.com>
cc: gregkh@linuxfoundation.org, rafael@kernel.org, akpm@linux-foundation.org, 
    surenb@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, 
    souravpanda@google.com
Subject: Re: [PATCH] vmstat: don't auto expand the sysfs files
In-Reply-To: <CA+CK2bA2vZp3e+HHfB-sdLsPUYghMxvKcWURktDtNjwPL79Csw@mail.gmail.com>
Message-ID: <b1049bfa-68c4-e237-30a9-1514a378c7f1@google.com>
References: <20231211154644.4103495-1-pasha.tatashin@soleen.com> <3d415ab4-e8c7-7e72-0379-952370612bdd@google.com> <CA+CK2bA2vZp3e+HHfB-sdLsPUYghMxvKcWURktDtNjwPL79Csw@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

On Thu, 14 Dec 2023, Pasha Tatashin wrote:

> > > Whenever a new fields are added one of the following: node_stat_item
> > > numa_stat_item zone_stat_item, the /sys/devices/system/node/nodeX/vmstat
> > > files are auto expanded.
> > >
> > > This is a problem, as sysfs files should be only one value per file.
> >
> > Does this patch address the one-value-per-file issue?  (I think that ship
> > has sailed for vmstat.)
> 
> That ship has sailed for vmstat, this patch addresses what was asked
> by GregKH: not to add new values to vmstat, as not to make the
> existing problem even worse. The sysfs file system has a one page
> limit per file. The developers will decide how to export the new items
> added to node_stat, numa_stat, zone_stat individually. Each new item
> can be exported in its own files, and must have its own documentation
> about interface stability, value meaning, and expectations when the
> stat file is absent.
> 

As of at least 6.5, /proc/vmstat is a strict superset of the per-node 
vmstat.  Why is that a problem?

There's great benefit to being able to use the sample implementations to 
parse either /proc/vmstat *or* the per-node vmstat and without needing to 
read the per-node vmstat plus some new set of sysfs files that are 
one-value-per-file.  The per-node vmstat will always be multiple values, 
in fact it's a key value pair.

I have to think that doing anything else for vmstat is just adding 
complexity (like this patch) and actually making it *harder* on userspace 
to read the data it needs.

Yes, the per-node vmstat likely shouldn't be in sysfs at all but it 
appears to have been added there 13+ years ago because it was a convenient 
place to add a per-node variant.  That's not ideal, but owell.