Re: [PATCH v6 16/16] PCI/AER: Add sysfs attributes for log ratelimits

Bjorn Helgaas
2025-05-20T16:31:02Z

On Tue, May 20, 2025 at 03:02:06PM +0300, Ilpo Järvinen wrote:
> On Mon, 19 May 2025, Bjorn Helgaas wrote:
>
> > From: Jon Pan-Doh <pandoh@google.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.
> >
> > Update AER sysfs ABI filename to reflect the broader scope of AER sysfs
> > attributes (e.g. stats and ratelimits).
> >
> > Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats ->
> > sysfs-bus-pci-devices-aer
> >
> > Tested using aer-inject[1]. Configured correctable log ratelimit to 5.
> > Sent 6 AER errors. Observed 5 errors logged while AER stats
> > (cat /sys/bus/pci/devices/<dev>/aer_dev_correctable) shows 6.
> >
> > Disabled ratelimiting and sent 6 more AER errors. Observed all 6 errors
> > logged and accounted in AER stats (12 total errors).
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/gong.chen/aer-inject.git
> >
> > Signed-off-by: Karolina Stolarek <karolina.stolarek@oracle.com>
> > Signed-off-by: Jon Pan-Doh <pandoh@google.com>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > Acked-by: Paul E. McKenney <paulmck@kernel.org>
> > ---
> > ...es-aer_stats => sysfs-bus-pci-devices-aer} | 34 +++++++
> > Documentation/PCI/pcieaer-howto.rst | 5 +-
> > drivers/pci/pci-sysfs.c | 1 +
> > drivers/pci/pci.h | 1 +
> > drivers/pci/pcie/aer.c | 99 +++++++++++++++++++
> > 5 files changed, 139 insertions(+), 1 deletion(-)
> > rename Documentation/ABI/testing/{sysfs-bus-pci-devices-aer_stats => sysfs-bus-pci-devices-aer} (77%)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
> > similarity index 77%
> > rename from Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
> > rename to Documentation/ABI/testing/sysfs-bus-pci-devices-aer
> > index d1f67bb81d5d..771204197b71 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
> > +++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
> > @@ -117,3 +117,37 @@ Date: July 2018
> > KernelVersion: 4.19.0
> > Contact: linux-pci@vger.kernel.org, rajatja@google.com
> > Description: Total number of ERR_NONFATAL messages reported to rootport.
> > +
> > +PCIe AER ratelimits
> > +-------------------
> > +
> > +These attributes show up under all the devices that are AER capable.
> > +They represent configurable ratelimits of logs per error type.
> > +
> > +See Documentation/PCI/pcieaer-howto.rst for more info on ratelimits.
> > +
> > +What: /sys/bus/pci/devices/<dev>/aer/ratelimit_log_enable
> > +Date: March 2025
> > +KernelVersion: 6.15.0
>
> This ship has sailed.

Updated to May 2025 and 6.16.0 (I hope :)).

> > +Contact: linux-pci@vger.kernel.org, pandoh@google.com
> > +Description: Writing 1/0 enables/disables AER log ratelimiting. Reading
> > + gets whether or not AER is currently enabled.
>
> AER or AER ratelimiting is enabled?

I think we want "AER ratelimiting" here, thanks!

> > + * Ratelimit enable toggle
> > + * 0: disabled with ratelimit.interval = 0
> > + * 1: enabled with ratelimit.interval = nonzero
> > + */
> > +static ssize_t ratelimit_log_enable_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + bool enabled = pdev->aer_report->cor_log_ratelimit.interval != 0;
> > +
> > + return sysfs_emit(buf, "%d\n", enabled);
> > +}
> > +
> > +static ssize_t ratelimit_log_enable_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + bool enable;
> > + int interval;
> > +
> > + if (!capable(CAP_SYS_ADMIN))
> > + return -EPERM;
> > +
> > + if (kstrtobool(buf, &enable) < 0)
> > + return -EINVAL;
> > +
> > + if (enable)
> > + interval = DEFAULT_RATELIMIT_INTERVAL;
> > + else
> > + interval = 0;
> > +
> > + pdev->aer_report->cor_log_ratelimit.interval = interval;
> > + pdev->aer_report->uncor_log_ratelimit.interval = interval;
> > +
> > + return count;
> > +}
> > +static DEVICE_ATTR_RW(ratelimit_log_enable);