Re: [PATCH] btrfs: Add ratelimiting to printing facility

[Nikolay Borisov <kernel@kyup.com>]

On 07/12/2016 04:20 PM, David Sterba wrote:
> On Tue, Jul 12, 2016 at 03:20:12PM +0300, Nikolay Borisov wrote:
>> Currently the btrfs printk infrastructure doesn't implement any
>> kind of ratelimiting.
> 
> If you count the whole infrastructure, it does. See ctree.h and macros
> ending with _rl (btrfs_err_rl), and should be used where the messages
> are likely to flood. Otherwise I think "more is better" regarding
> messages as this is helpful when debugging issues.

So I definitely didn't look at those. But now I have and it seems they
implement more or less the same thing. Also, if I'm reading the code
correctly, as it stands now using the _rl versions seem to be more
flexible as the limits is going to be per-message rather than
per-message class as it is in my proposal. So I'd rather move that
particular csum related message to the _rl infrastructure.

> 
>> Recently I came accross a case where due to
>> FS corruption an excessive amount of printk caused the softlockup
>> detector to trigger and reset the server. This patch aims to avoid
>> two types of issue: 
>>  * I want to avoid different levels of messages interefere with the
>>  ratelimiting of oneanother so as to avoid a situation where a 
>>  flood of INFO messages causes the ratelimit to trigger, 
>>  potentially leading to supression of more important messages. 
> 
> Yeah, that's my concern as well. What if there's a burst of several
> error messages that do not fit to the limit and some of them get
> dropped.
> 
>>  * Avoid a flood of any type of messages rendering the machine 
>>  unusable
> 
> While I'd rather set a per-message ratelimiting, it's possible that an
> unexpected error will start flooding. So some sort of per-level limiting
> could be implemented, as you propose, but I'd suggest to set the numbers
> higher. That way it would still flood up to certain level but should
> avoid the lockups.

Sure, I'm happy to set the limits higher and have it act as a safety
net. But then again, what would make a sensible limit - 100 messages in
5 seconds seems reasonable but this is completely arbitrary. Any thoughts?