linux-kernel - Re: Sysfs one-value-per-file (was 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]

Date: Thu, 28 Dec 2023 10:17:37 +0000
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: David Rientjes <rientjes@...gle.com>
Cc: Pasha Tatashin <pasha.tatashin@...een.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>, rafael@...nel.org,
	Andrew Morton <akpm@...ux-foundation.org>, surenb@...gle.com,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	souravpanda@...gle.com
Subject: Re: Sysfs one-value-per-file (was Re: [PATCH] vmstat: don't auto
 expand the sysfs files)

On Tue, Dec 26, 2023 at 04:53:31PM -0800, David Rientjes wrote:
> I'd argue that the ship on the "sysfs one-value-per-file rule" has sailed 
> for long-standing use cases where either (1) switching is just not 
> possible or (2) switching would be an undue burden to the user.
> 
> An example of (1) would be THP enablement and defrag options:
> 
> $ grep . /sys/kernel/mm/transparent_hugepage/{defrag,enabled,shmem_enabled}
> /sys/kernel/mm/transparent_hugepage/defrag:always defer defer+madvise [madvise] never
> /sys/kernel/mm/transparent_hugepage/enabled:[always] madvise never
> /sys/kernel/mm/transparent_hugepage/shmem_enabled:always within_size advise [never] deny force
> 
> This convention isn't going to change.  We're not going to suddenly add a 
> new enablement or defrag option that can only be set in a newly added 
> file that is one-value-per-file.
> 
> THP was obviously introduced before any sysfs "one-value-per-file rule" 

No, the rule has been there since "day one" for sysfs, this file snuck
in much later with no one noticing it against the "rules" and I've been
complaining about it every time someone tries to add a new field to it
that I notice.

> and, in retrospect, I think most people would agree that these files would 
> be much better off implemented returning a single word.  But, choices 
> where made in the "before times", and it was implemented in a way that 
> shows all the possible choices and which one is in effect.  Owell.  We 
> deal with it, and we move on.
> 
> Imagine if I add a new choice of "lightweight" to the "defrag" file.  The 
> only rational way to do that would be to extend the current interface, not 
> create a new /sys/kernel/mm/transparent_hugepage/defrag/lightweight file 
> that is one-value-per-file that unsets all the other options in "defrag."
> Please.

Please remember that the sysfs rule is there for a good reason, it makes
it very hard to break existing userspace tools if you stick with it.  If
you decide to ignore that rule, then you are on your own and better make
sure that nothing breaks.

Again, please learn from our previous mistakes with proc files, that is
why the rule is there.

If you wish to ignore the rule, you all really are on your own, good
luck!

greg k-h