Re: [PATCH 4/8] PCI/AER: Introduce ratelimit for error logs

[Karolina Stolarek <karolina.stolarek@oracle.com>]

On 18/01/2025 02:59, Jon Pan-Doh wrote:
> On Thu, Jan 16, 2025 at 3:11 AM Karolina Stolarek
> <karolina.stolarek@oracle.com> wrote:
>> In some cases, it would be beneficial to keep the "X callbacks
>> suppressed" to give an idea of how prevalent the errors are. On some
>> devices, we could see just 11 Correctable Errors per 5 seconds, but on
>> others this would be >1k (seen such a case myself).
>>
>> As it's not immediately clear what the defaults are, could you add a
>> comment for this?
> 
> It seems like the convention is not to use RATELIMIT_MSG_ON_RELEASE (I
> was unfamiliar :)). I'll omit this in the next version. Let me know if
> you'd still like the comment in that case.

Interesting, I wasn't aware of that. There are still a couple of places 
where this flag is used, but maybe it's legacy code.

I'm not too passionate about the comment, it was just a suggestion (I 
tend to be verbose in my code), feel free to omit it.

> FYI the majority of existing usage seems to be split between
> __ratelimit() and !__ratelimit() (though majority doesn't always
> indicate the right thing). The only instance I see of a variable is in
> drivers/iommu/intel/dmar.c:dmar_fault where the variable is used in
> repeated conditionals.

Yeah, it's a problem that's hard to solve at this level.

I thought that introducing a variable to make it easier to read is 
acceptable, but it's hard to defend this approach when the value is 
checked only once. Let's keep things simple and leave it as it is.

All the best,
Karolina

> 
> Thanks,
> Jon