On Wed, Mar 19, 2025 at 11:47 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > - I think the ratelimit lookup and __ratelimit() call should be > together since there's no need for trace_aer_event() to be in the > middle. > > - The lookup and __ratelimit() calls are repeated and are probably > worth factoring out into something like this: > > static int aer_ratelimit(struct pci_dev *dev, unsigned int severity) Changed in v4. > - Previously we *always* called trace_aer_event(), but now we don't > in the !info->status case. Maybe an unintentional change? I > think we should call trace_aer_event() always, or change that in a > separate patch if we need to. This would always have been simpler > if trace_aer_event() had been the very first thing in the > function. Good catch. That is an unintentional bug. trace_aer_event() should always be called. Moved it to the first thing in aer_print_error() in v4 (same patch as I wasn't sure what justification to put for a separate commit message other than precursor for ratelimit). > > - The !info->status case message is not rate-limited. Seems like > maybe it should be? Changed in v4. Thanks, Jon
