| Message ID | 0628fc55984c3507c5365d4e1d5ed96d28693939.1726261774.git.loemra.dev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: set flag to message when ratelimited | expand |
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>
在 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, ...)
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.
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?
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
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, ...)
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(-)