[PATCH] btrfs: set flag to message when ratelimited

[PATCH] btrfs: set flag to message when ratelimited

[Leo Martins]

Set RATELIMIT_MSG_ON_RELEASE flag to send a message when being
ratelimited. During some recent debugging not realizing that
logs were being ratelimited caused some confusion so making
sure there is a warning seems prudent.

Signed-off-by: Leo Martins <loemra.dev@gmail.com>
---
 fs/btrfs/messages.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index 77752eec125d9..22316067bde75 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -199,14 +199,22 @@ static const char * const logtypes[] = {
  * messages doesn't cause more important ones to be dropped.
  */
 static struct ratelimit_state printk_limits[] = {
-	RATELIMIT_STATE_INIT(printk_limits[0], DEFAULT_RATELIMIT_INTERVAL, 100),
-	RATELIMIT_STATE_INIT(printk_limits[1], DEFAULT_RATELIMIT_INTERVAL, 100),
-	RATELIMIT_STATE_INIT(printk_limits[2], DEFAULT_RATELIMIT_INTERVAL, 100),
-	RATELIMIT_STATE_INIT(printk_limits[3], DEFAULT_RATELIMIT_INTERVAL, 100),
-	RATELIMIT_STATE_INIT(printk_limits[4], DEFAULT_RATELIMIT_INTERVAL, 100),
-	RATELIMIT_STATE_INIT(printk_limits[5], DEFAULT_RATELIMIT_INTERVAL, 100),
-	RATELIMIT_STATE_INIT(printk_limits[6], DEFAULT_RATELIMIT_INTERVAL, 100),
-	RATELIMIT_STATE_INIT(printk_limits[7], DEFAULT_RATELIMIT_INTERVAL, 100),
+	RATELIMIT_STATE_INIT_FLAGS(printk_limits[0], DEFAULT_RATELIMIT_INTERVAL,
+				   100, RATELIMIT_MSG_ON_RELEASE),
+	RATELIMIT_STATE_INIT_FLAGS(printk_limits[1], DEFAULT_RATELIMIT_INTERVAL,
+				   100, RATELIMIT_MSG_ON_RELEASE),
+	RATELIMIT_STATE_INIT_FLAGS(printk_limits[2], DEFAULT_RATELIMIT_INTERVAL,
+				   100, RATELIMIT_MSG_ON_RELEASE),
+	RATELIMIT_STATE_INIT_FLAGS(printk_limits[3], DEFAULT_RATELIMIT_INTERVAL,
+				   100, RATELIMIT_MSG_ON_RELEASE),
+	RATELIMIT_STATE_INIT_FLAGS(printk_limits[4], DEFAULT_RATELIMIT_INTERVAL,
+				   100, RATELIMIT_MSG_ON_RELEASE),
+	RATELIMIT_STATE_INIT_FLAGS(printk_limits[5], DEFAULT_RATELIMIT_INTERVAL,
+				   100, RATELIMIT_MSG_ON_RELEASE),
+	RATELIMIT_STATE_INIT_FLAGS(printk_limits[6], DEFAULT_RATELIMIT_INTERVAL,
+				   100, RATELIMIT_MSG_ON_RELEASE),
+	RATELIMIT_STATE_INIT_FLAGS(printk_limits[7], DEFAULT_RATELIMIT_INTERVAL,
+				   100, RATELIMIT_MSG_ON_RELEASE),
 };
 
 void __cold _btrfs_printk(const struct btrfs_fs_info *fs_info, const char *fmt, ...)
-- 
2.43.5

Re: [PATCH] btrfs: set flag to message when ratelimited

[Omar Sandoval]

On Fri, Sep 13, 2024 at 02:26:06PM -0700, Leo Martins wrote:
> Set RATELIMIT_MSG_ON_RELEASE flag to send a message when being
> ratelimited. During some recent debugging not realizing that
> logs were being ratelimited caused some confusion so making
> sure there is a warning seems prudent.
> 
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>

Reviewed-by: Omar Sandoval <osandov@fb.com>

Re: [PATCH] btrfs: set flag to message when ratelimited

[Qu Wenruo]

在 2024/9/14 06:56, Leo Martins 写道:
> Set RATELIMIT_MSG_ON_RELEASE flag to send a message when being
> ratelimited. During some recent debugging not realizing that
> logs were being ratelimited caused some confusion so making
> sure there is a warning seems prudent.
>
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/messages.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
> index 77752eec125d9..22316067bde75 100644
> --- a/fs/btrfs/messages.c
> +++ b/fs/btrfs/messages.c
> @@ -199,14 +199,22 @@ static const char * const logtypes[] = {
>    * messages doesn't cause more important ones to be dropped.
>    */
>   static struct ratelimit_state printk_limits[] = {
> -	RATELIMIT_STATE_INIT(printk_limits[0], DEFAULT_RATELIMIT_INTERVAL, 100),
> -	RATELIMIT_STATE_INIT(printk_limits[1], DEFAULT_RATELIMIT_INTERVAL, 100),
> -	RATELIMIT_STATE_INIT(printk_limits[2], DEFAULT_RATELIMIT_INTERVAL, 100),
> -	RATELIMIT_STATE_INIT(printk_limits[3], DEFAULT_RATELIMIT_INTERVAL, 100),
> -	RATELIMIT_STATE_INIT(printk_limits[4], DEFAULT_RATELIMIT_INTERVAL, 100),
> -	RATELIMIT_STATE_INIT(printk_limits[5], DEFAULT_RATELIMIT_INTERVAL, 100),
> -	RATELIMIT_STATE_INIT(printk_limits[6], DEFAULT_RATELIMIT_INTERVAL, 100),
> -	RATELIMIT_STATE_INIT(printk_limits[7], DEFAULT_RATELIMIT_INTERVAL, 100),
> +	RATELIMIT_STATE_INIT_FLAGS(printk_limits[0], DEFAULT_RATELIMIT_INTERVAL,
> +				   100, RATELIMIT_MSG_ON_RELEASE),
> +	RATELIMIT_STATE_INIT_FLAGS(printk_limits[1], DEFAULT_RATELIMIT_INTERVAL,
> +				   100, RATELIMIT_MSG_ON_RELEASE),
> +	RATELIMIT_STATE_INIT_FLAGS(printk_limits[2], DEFAULT_RATELIMIT_INTERVAL,
> +				   100, RATELIMIT_MSG_ON_RELEASE),
> +	RATELIMIT_STATE_INIT_FLAGS(printk_limits[3], DEFAULT_RATELIMIT_INTERVAL,
> +				   100, RATELIMIT_MSG_ON_RELEASE),
> +	RATELIMIT_STATE_INIT_FLAGS(printk_limits[4], DEFAULT_RATELIMIT_INTERVAL,
> +				   100, RATELIMIT_MSG_ON_RELEASE),
> +	RATELIMIT_STATE_INIT_FLAGS(printk_limits[5], DEFAULT_RATELIMIT_INTERVAL,
> +				   100, RATELIMIT_MSG_ON_RELEASE),
> +	RATELIMIT_STATE_INIT_FLAGS(printk_limits[6], DEFAULT_RATELIMIT_INTERVAL,
> +				   100, RATELIMIT_MSG_ON_RELEASE),
> +	RATELIMIT_STATE_INIT_FLAGS(printk_limits[7], DEFAULT_RATELIMIT_INTERVAL,
> +				   100, RATELIMIT_MSG_ON_RELEASE),
>   };
>
>   void __cold _btrfs_printk(const struct btrfs_fs_info *fs_info, const char *fmt, ...)

Re: [PATCH] btrfs: set flag to message when ratelimited

[David Sterba]

On Fri, Sep 13, 2024 at 02:26:06PM -0700, Leo Martins wrote:
> Set RATELIMIT_MSG_ON_RELEASE flag to send a message when being
> ratelimited.

What does this really do? The documentation does not help either:

/* issue num suppressed message on exit */

> During some recent debugging not realizing that
> logs were being ratelimited caused some confusion so making
> sure there is a warning seems prudent.

So you can enable it only for debugging level.

Re: [PATCH] btrfs: set flag to message when ratelimited

[Naohiro Aota]

On Tue, Sep 17, 2024 at 06:30:45PM GMT, David Sterba wrote:
> On Fri, Sep 13, 2024 at 02:26:06PM -0700, Leo Martins wrote:
> > Set RATELIMIT_MSG_ON_RELEASE flag to send a message when being
> > ratelimited.
> 
> What does this really do? The documentation does not help either:
> 
> /* issue num suppressed message on exit */
> 
> > During some recent debugging not realizing that
> > logs were being ratelimited caused some confusion so making
> > sure there is a warning seems prudent.
> 
> So you can enable it only for debugging level.

Adding my point, I often see btrfs_info() in btrfs_dump_space_info() is
ratelimited and the info is incomplete, because it tries to print usage info
about all the block groups. For that purpose, I don't think printing the
number of suppressed lines helps.

Instead, I'd prefer disabling rate limit for them, given that the messages
are behind some debug flag (e.g, CONFIG_BTRFS_DEBUG and/or mount -o
enospc_debug). So, how about adding btrfs_debug_info() which prints the
message in INFO level without rate limitting?

Re: [PATCH] btrfs: set flag to message when ratelimited

[Leo Martins]

On Tue, 17 Sep 2024 23:06, Naohiro Aota <Naohiro.Aota@wdc.com> wrote:
>On Tue, Sep 17, 2024 at 06:30:45PM GMT, David Sterba wrote:
>> On Fri, Sep 13, 2024 at 02:26:06PM -0700, Leo Martins wrote:
>> > Set RATELIMIT_MSG_ON_RELEASE flag to send a message when being
>> > ratelimited.
>> 
>> What does this really do? The documentation does not help either:
>> 
>> /* issue num suppressed message on exit */
>> 
>> > During some recent debugging not realizing that
>> > logs were being ratelimited caused some confusion so making
>> > sure there is a warning seems prudent.
>> 
>> So you can enable it only for debugging level.
>
>Adding my point, I often see btrfs_info() in btrfs_dump_space_info() is
>ratelimited and the info is incomplete, because it tries to print usage info
>about all the block groups. For that purpose, I don't think printing the
>number of suppressed lines helps.
>
>Instead, I'd prefer disabling rate limit for them, given that the messages
>are behind some debug flag (e.g, CONFIG_BTRFS_DEBUG and/or mount -o
>enospc_debug). So, how about adding btrfs_debug_info() which prints the
>message in INFO level without rate limitting?

When I previously sent this patch I thought that
RATELIMIT_MSG_ON_RELEASE flag caused a warning message to be printed
when ratelimited. This is true, however, a warning message is printed
regardless of whether this flag is enabled. When the flag is disabled
there is a printk_deferred [1] that will warn the user of the number
of callbacks that were suppressed. When the flag is enabled the warning
is instead printed when ratelimit_state_exit [2] is called which happens
on closing of /dev/kmsg. I now think that setting the flag isn't
necessary and instead I'll send out a different patch that implements
what Naohiro suggested.

[1] https://github.com/btrfs/linux/blob/21b136cc63d2a9ddd60d4699552b69c214b32964/lib/ratelimit.c#L56
[2] https://github.com/btrfs/linux/blob/21b136cc63d2a9ddd60d4699552b69c214b32964/include/linux/ratelimit.h#L31