On Mon, Feb 17, 2025 at 5:31 AM Karolina Stolarek <karolina.stolarek@xxxxxxxxxx> wrote: > I don't think it's neccessary to keep ratelimits in a separate directory > when we decided to we keep the rest of AER attributes at the dev level. My motivation for this is that there may be more AER sysfs attributes we want to expose. An example being OCP Fault Management roadmap which aims to have userspace manage/enforce AER settings (set/get PCIE AER regs) for datacenter repairability. Given the permanence of sysfs entries, I think that it is reasonable to create a new directory to make AER sysfs attributes more extensible. > > +These attributes show up under all the devices that are AER capable. > > +Provides configurable ratelimits of logs/irq per error type. Writing a > > This sentence seems to refer to _one_ attribute and mentions IRQ > ratelimiting, which is not a part of the series. > > How about rephrasing this section to say that the attributes allow > configuration of the rate at which AER errors are reported, with each of > them dedicated to one error type (correctable or uncorrectable)? > Something along these lines. Good catch. IRQ verbiage slipped in from v1. How's this: These attributes show up under all the devices that are AER capable. They allow configuration of the rate at which AER errors are reported, with each of them dedicated to one error type (correctable or uncorrectable). I kept the first sentence as that is common under the other AER sysfs attributes. > The convention is that sysfs files should provide one value per file. It > won't be just people interacting with this interface, but scripts as > well. Parsing such a string is a hassle. As we can only change the burst > of the ratelimit, I'd simply emit pdev->aer_report->ratelimit.burst. Ack. Will update in v3 and add explicit mention of ratelimit interval in sysfs-bus-pci-devices-aer_stats so context is still there. Thanks, Jon