From: Leo Martins <loemra.dev@gmail.com>
To: Naohiro Aota <Naohiro.Aota@wdc.com>, David Sterba <dsterba@suse.cz>
Cc: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
"kernel-team@fb.com" <kernel-team@fb.com>
Subject: Re: [PATCH] btrfs: set flag to message when ratelimited
Date: Tue, 24 Sep 2024 15:38:12 -0700 [thread overview]
Message-ID: <kcaj8.ncm2uv11etg@gmail.com> (raw)
In-Reply-To: <p6svr5p3q7yezuqsevuzq7jxqbhn2wbq2cjz43e3nuk5bxfxhl@pgkaoqo53byw>
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
prev parent reply other threads:[~2024-09-24 22:59 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-13 21:26 [PATCH] btrfs: set flag to message when ratelimited Leo Martins
2024-09-13 22:58 ` Omar Sandoval
2024-09-14 0:18 ` Qu Wenruo
2024-09-17 16:30 ` David Sterba
2024-09-18 6:06 ` Naohiro Aota
2024-09-24 22:38 ` Leo Martins [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=kcaj8.ncm2uv11etg@gmail.com \
--to=loemra.dev@gmail.com \
--cc=Naohiro.Aota@wdc.com \
--cc=dsterba@suse.cz \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).