linux-kernel - Re: [PATCH] vmstat: don't auto expand the sysfs files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b1049bfa-68c4-e237-30a9-1514a378c7f1@google.com>
Date: Sun, 24 Dec 2023 13:26:15 -0800 (PST)
From: David Rientjes <rientjes@...gle.com>
To: Pasha Tatashin <pasha.tatashin@...een.com>
cc: gregkh@...uxfoundation.org, rafael@...nel.org, akpm@...ux-foundation.org, 
    surenb@...gle.com, linux-kernel@...r.kernel.org, linux-mm@...ck.org, 
    souravpanda@...gle.com
Subject: Re: [PATCH] vmstat: don't auto expand the sysfs files

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.