linux-kernel - Re: [PATCH v7 17/17] PCI/AER: Add sysfs attributes for log ratelimits

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: <20250522122113.000030c0@huawei.com>
Date: Thu, 22 May 2025 12:21:13 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: <linux-pci@...r.kernel.org>, Jon Pan-Doh <pandoh@...gle.com>, "Karolina
 Stolarek" <karolina.stolarek@...cle.com>, Weinan Liu <wnliu@...gle.com>,
	Martin Petersen <martin.petersen@...cle.com>, Ben Fuller
	<ben.fuller@...cle.com>, Drew Walton <drewwalton@...rosoft.com>, "Anil
 Agrawal" <anilagrawal@...a.com>, Tony Luck <tony.luck@...el.com>, Ilpo
 Järvinen <ilpo.jarvinen@...ux.intel.com>, "Sathyanarayanan
 Kuppuswamy" <sathyanarayanan.kuppuswamy@...ux.intel.com>, Lukas Wunner
	<lukas@...ner.de>, Sargun Dhillon <sargun@...a.com>, "Paul E . McKenney"
	<paulmck@...nel.org>, Mahesh J Salgaonkar <mahesh@...ux.ibm.com>, "Oliver
 O'Halloran" <oohall@...il.com>, Kai-Heng Feng <kaihengf@...dia.com>, "Keith
 Busch" <kbusch@...nel.org>, Robert Richter <rrichter@....com>, Terry Bowman
	<terry.bowman@....com>, Shiju Jose <shiju.jose@...wei.com>, Dave Jiang
	<dave.jiang@...el.com>, <linux-kernel@...r.kernel.org>,
	<linuxppc-dev@...ts.ozlabs.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>
Subject: Re: [PATCH v7 17/17] PCI/AER: Add sysfs attributes for log
 ratelimits

On Wed, 21 May 2025 17:59:42 -0500
Bjorn Helgaas <helgaas@...nel.org> wrote:

> On Wed, May 21, 2025 at 11:46:00AM +0100, Jonathan Cameron wrote:
> > On Tue, 20 May 2025 16:50:34 -0500
> > Bjorn Helgaas <helgaas@...nel.org> wrote:
> >   
> > > From: Jon Pan-Doh <pandoh@...gle.com>
> > > 
> > > Allow userspace to read/write log ratelimits per device (including
> > > enable/disable). Create aer/ sysfs directory to store them and any
> > > future aer configs.  
> > ...  
> 
> > There is some relatively new SYSFS infra that I think will help
> > make this slightly nicer by getting rid of the extra directory when
> > there is nothing to be done with it.  
> 
> > > +#define aer_ratelimit_burst_attr(name, ratelimit)			\
> > > +	static ssize_t							\
> > > +	name##_show(struct device *dev, struct device_attribute *attr,	\
> > > +		    char *buf)						\
> > > +{									\  
> > 
> > A little odd looking to indent this less than the line above.  
> 
> Yep, fixed.
> 
> > > +const struct attribute_group aer_attr_group = {
> > > +	.name = "aer",
> > > +	.attrs = aer_attrs,
> > > +	.is_visible = aer_attrs_are_visible,
> > > +};  
> > 
> > There are a bunch of macros to simplify cases where
> > a whole group is either enabled or not and make the group
> > itself go away if there is nothing to be shown.
> > 
> > DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() combined with
> > SYSFS_GROUP_VISIBLE() around the assignment does what we
> > want here I think.
> > 
> > Whilst we can't retrofit that stuff onto existing ABI
> > as someone may be assuming directory presence, we can
> > make sysfs less cluttered for new stuff.
> > 
> > Maybe I'm missing why that doesn't work here though!  
> 
> Is this something we can fix later, or are we locking ourselves into
> user-visible ABI that's hard to change?  I'm kind of against the wall
> relative to the v6.16 merge window and haven't had time to dig into
> this part.

That comes down to Ilpo's question of whether empty directories
are ABI (specifically if anyone notices us removing them).
It seems unlikely anyone will code against requirement for an empty
dir, but you never know.

Given we probably have a bunch of these in PCI anyway that predate
that magic, one more isn't a problem even if we decide we can't
tidy it up later.

So I'm fine with not bothering to hide the dir for now (and maybe for
ever).

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>


Jonathan

>