* printk_ratelimited() && proc/sys/kernel/printk_ratelimit*
@ 2019-03-20 17:03 Oleg Nesterov
2019-03-20 17:30 ` Linus Torvalds
0 siblings, 1 reply; 3+ messages in thread
From: Oleg Nesterov @ 2019-03-20 17:03 UTC (permalink / raw)
To: Andrew Morton, Ingo Molnar, Linus Torvalds; +Cc: Steffen Froemer, linux-kernel
Sorry for spam, but I simply do not know whom to ask ;)
/proc/sys/kernel/printk_ratelimit and /proc/sys/kernel/printk_ratelimit_burst
affect printk_ratelimit_state which have a single user, printk_ratelimit() which
is "don't use" according to its comment.
At the same time, printk_ratelimited() uses a static ratelimit_state, so user-
space can't override the default DEFAULT_RATELIMIT_.* numbers.
Isn't it strange? Shouldn't printk_ratelimited() use printk_ratelimit_state ?
--- x/include/linux/printk.h
+++ x/include/linux/printk.h
@@ -415,11 +415,7 @@ extern int kptr_restrict;
#ifdef CONFIG_PRINTK
#define printk_ratelimited(fmt, ...) \
({ \
- static DEFINE_RATELIMIT_STATE(_rs, \
- DEFAULT_RATELIMIT_INTERVAL, \
- DEFAULT_RATELIMIT_BURST); \
- \
- if (__ratelimit(&_rs)) \
+ if (__ratelimit(&printk_ratelimit_state)) \
printk(fmt, ##__VA_ARGS__); \
})
#else
--- x/kernel/printk/printk.c
+++ x/kernel/printk/printk.c
@@ -2979,7 +2979,8 @@ int printk_deferred(const char *fmt, ...
* This enforces a rate limit: not more than 10 kernel messages
* every 5s to make a denial-of-service attack impossible.
*/
-DEFINE_RATELIMIT_STATE(printk_ratelimit_state, 5 * HZ, 10);
+DEFINE_RATELIMIT_STATE(printk_ratelimit_state,
+ DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
int __printk_ratelimit(const char *func)
{
------------------------------------------------------------------------------
Another question is spin_trylock() in ___ratelimit(). This means that a message
can be silently dropped "for no reason", without rs->missed++. Doesn't look good.
The changelog for commit edaac8e3167 "ratelimit: Fix/allow use in atomic contexts"
says:
I'd like to use printk_ratelimit() in NMI context, but it's not
robust right now due to spinlock usage in lib/ratelimit.c. If an
NMI is unlucky enough to hit just that spot we might lock up trying
to take the spinlock again.
if NMI is the the only problem, why can't we do
if (in_nmi()) {
if (!raw_spin_trylock_irqsave(&rs->lock, flags))
return 0;
} else {
raw_spin_lock_irqsave(&rs->lock, flags);
}
?
Oleg.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: printk_ratelimited() && proc/sys/kernel/printk_ratelimit*
2019-03-20 17:03 printk_ratelimited() && proc/sys/kernel/printk_ratelimit* Oleg Nesterov
@ 2019-03-20 17:30 ` Linus Torvalds
2019-03-20 17:48 ` Oleg Nesterov
0 siblings, 1 reply; 3+ messages in thread
From: Linus Torvalds @ 2019-03-20 17:30 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Ingo Molnar, Steffen Froemer,
Linux List Kernel Mailing
On Wed, Mar 20, 2019 at 10:03 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> At the same time, printk_ratelimited() uses a static ratelimit_state, so user-
> space can't override the default DEFAULT_RATELIMIT_.* numbers.
>
> Isn't it strange? Shouldn't printk_ratelimited() use printk_ratelimit_state ?
No it shouldn't.
Each printk_ratelimited() is independent, and that's very much on purpose.
One screaming printk shouldn't mean that every *other* printk would be shut up.
That said, it's likely true that nobody should really use
printk_ratelimited() anyway. But if two different users do, they
definitely shouldn't then affect each other.
Linus
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: printk_ratelimited() && proc/sys/kernel/printk_ratelimit*
2019-03-20 17:30 ` Linus Torvalds
@ 2019-03-20 17:48 ` Oleg Nesterov
0 siblings, 0 replies; 3+ messages in thread
From: Oleg Nesterov @ 2019-03-20 17:48 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Ingo Molnar, Steffen Froemer,
Linux List Kernel Mailing
On 03/20, Linus Torvalds wrote:
>
> On Wed, Mar 20, 2019 at 10:03 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > At the same time, printk_ratelimited() uses a static ratelimit_state, so user-
> > space can't override the default DEFAULT_RATELIMIT_.* numbers.
> >
> > Isn't it strange? Shouldn't printk_ratelimited() use printk_ratelimit_state ?
>
> No it shouldn't.
>
> Each printk_ratelimited() is independent, and that's very much on purpose.
Damn, I am really stupid.
Oleg.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-20 17:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-20 17:03 printk_ratelimited() && proc/sys/kernel/printk_ratelimit* Oleg Nesterov
2019-03-20 17:30 ` Linus Torvalds
2019-03-20 17:48 ` Oleg Nesterov
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).