All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] xen/console: updates to rate-limiting
@ 2026-07-15 20:18 dmukhin
  2026-07-15 20:18 ` [PATCH v3 1/4] xen/console: use bool as return value from printk_ratelimit() dmukhin
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: dmukhin @ 2026-07-15 20:18 UTC (permalink / raw)
  To: xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini, dmukhin

The series introduces compile-configuration for diagnostic
messages rate-limiting.

Patch 1 is a tiny cleanup for ratelimiting APIs.
Patch 2 is a fixup for the rate-limiter to adjust to new user-defined
rate-limiting parameters.
Patch 3 introduces build-time rate-limiting controls.
Patch 4 is another tiny cleanup to reduce the amount of code.

[1] Link to v2: https://lore.kernel.org/xen-devel/20260206202424.2054758-1-dmukhin@ford.com/
[2] CI run: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/2679857573

Denis Mukhin (4):
  xen/console: use bool as return value from printk_ratelimit()
  xen/console: correct leaky-bucket rate limiter
  xen/console: add build-time rate-limiting controls
  xen/console: reduce number of printouts in __printk_ratelimit()

 xen/common/Kconfig         | 29 ++++++++++++++++++++++++
 xen/drivers/char/console.c | 46 ++++++++++++++++++++++----------------
 xen/include/xen/lib.h      |  6 ++---
 3 files changed, 59 insertions(+), 22 deletions(-)

-- 
2.54.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/4] xen/console: use bool as return value from printk_ratelimit()
  2026-07-15 20:18 [PATCH v3 0/4] xen/console: updates to rate-limiting dmukhin
@ 2026-07-15 20:18 ` dmukhin
  2026-07-16  9:23   ` Teddy Astie
  2026-07-15 20:19 ` [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter dmukhin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: dmukhin @ 2026-07-15 20:18 UTC (permalink / raw)
  To: xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini, dmukhin

From: Denis Mukhin <dmukhin@ford.com> 

{__,}printk_ratelimit() are predicates. Make that pronounced by updating the
return value to boolean.

Not a functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes since v2:
- rebased
---
 xen/drivers/char/console.c | 9 +++++----
 xen/include/xen/lib.h      | 6 +++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index dbda7f259668..5d395f882e08 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1270,7 +1270,8 @@ void console_end_sync(void)
  * This enforces a rate limit: not more than one kernel message
  * every printk_ratelimit_ms (millisecs).
  */
-int __printk_ratelimit(unsigned int ratelimit_ms, unsigned int ratelimit_burst)
+bool __printk_ratelimit(unsigned int ratelimit_ms,
+                        unsigned int ratelimit_burst)
 {
     static DEFINE_SPINLOCK(ratelimit_lock);
     static unsigned long toks = 10 * 5 * 1000;
@@ -1309,11 +1310,11 @@ int __printk_ratelimit(unsigned int ratelimit_ms, unsigned int ratelimit_burst)
             rspin_unlock(&console_lock);
         }
         local_irq_restore(flags);
-        return 1;
+        return true;
     }
     missed++;
     spin_unlock_irqrestore(&ratelimit_lock, flags);
-    return 0;
+    return false;
 }
 
 /* Minimum time in ms between messages */
@@ -1322,7 +1323,7 @@ static const unsigned int printk_ratelimit_ms = 5 * 1000;
 /* Number of messages we send before ratelimiting */
 static const unsigned int printk_ratelimit_burst = 10;
 
-int printk_ratelimit(void)
+bool printk_ratelimit(void)
 {
     return __printk_ratelimit(printk_ratelimit_ms, printk_ratelimit_burst);
 }
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index bb0fd446b484..3c545ff33c61 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -82,9 +82,9 @@ extern void guest_printk(const struct domain *d, const char *fmt, ...)
     __attribute__ ((format (printf, 2, 3)));
 extern void noreturn panic(const char *fmt, ...)
     __attribute__ ((format (printf, 1, 2)));
-extern int __printk_ratelimit(unsigned int ratelimit_ms,
-                              unsigned int ratelimit_burst);
-extern int printk_ratelimit(void);
+extern bool __printk_ratelimit(unsigned int ratelimit_ms,
+                               unsigned int ratelimit_burst);
+extern bool printk_ratelimit(void);
 
 #define gprintk(lvl, fmt, args...) \
     printk(XENLOG_GUEST lvl "%pv " fmt, current, ## args)
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter
  2026-07-15 20:18 [PATCH v3 0/4] xen/console: updates to rate-limiting dmukhin
  2026-07-15 20:18 ` [PATCH v3 1/4] xen/console: use bool as return value from printk_ratelimit() dmukhin
@ 2026-07-15 20:19 ` dmukhin
  2026-07-16  9:50   ` Teddy Astie
  2026-07-16 15:38   ` Jan Beulich
  2026-07-15 20:19 ` [PATCH v3 3/4] xen/console: add build-time rate-limiting controls dmukhin
  2026-07-15 20:19 ` [PATCH v3 4/4] xen/console: reduce number of printouts in __printk_ratelimit() dmukhin
  3 siblings, 2 replies; 13+ messages in thread
From: dmukhin @ 2026-07-15 20:19 UTC (permalink / raw)
  To: xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini, dmukhin

From: Denis Mukhin <dmukhin@ford.com> 

Use existing printk_ratelimit_ms and printk_ratelimit_burst variables in
do_printk_ratelimit() instead of hardcoded values 5000 and 10 respectively.

Ensure rate limiter is disabled if either printk_ratelimit_ms or
printk_ratelimit_burst is 0. Make sure no unnecessary initialization is done
in the corner case.

Also, simplify the limiter code by using min().

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- fixed typing and 32-bit integer overflow problem
---
 xen/drivers/char/console.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 5d395f882e08..de9f2432445d 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1274,21 +1274,26 @@ bool __printk_ratelimit(unsigned int ratelimit_ms,
                         unsigned int ratelimit_burst)
 {
     static DEFINE_SPINLOCK(ratelimit_lock);
-    static unsigned long toks = 10 * 5 * 1000;
-    static unsigned long last_msg;
+    static unsigned long long toks, last_msg;
     static unsigned int missed;
+    unsigned long long now, limit;
     unsigned long flags;
-    unsigned long long now = NOW(); /* ns */
-    unsigned long ms;
+    s_time_t ms;
 
-    do_div(now, 1000000);
-    ms = (unsigned long)now;
+    if ( !ratelimit_burst || !ratelimit_burst )
+        return true;
+
+    limit = min(ratelimit_burst * ratelimit_ms, UINT_MAX);
+    if ( !toks )
+        toks = limit;
+
+    now = NOW(); /* ns */
+    ms = do_div(now, MILLISECS(1));
 
     spin_lock_irqsave(&ratelimit_lock, flags);
     toks += ms - last_msg;
     last_msg = ms;
-    if ( toks > (ratelimit_burst * ratelimit_ms))
-        toks = ratelimit_burst * ratelimit_ms;
+    toks = min(toks, limit);
     if ( toks >= ratelimit_ms )
     {
         unsigned int lost = missed;
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/4] xen/console: add build-time rate-limiting controls
  2026-07-15 20:18 [PATCH v3 0/4] xen/console: updates to rate-limiting dmukhin
  2026-07-15 20:18 ` [PATCH v3 1/4] xen/console: use bool as return value from printk_ratelimit() dmukhin
  2026-07-15 20:19 ` [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter dmukhin
@ 2026-07-15 20:19 ` dmukhin
  2026-07-16  9:59   ` Teddy Astie
  2026-07-15 20:19 ` [PATCH v3 4/4] xen/console: reduce number of printouts in __printk_ratelimit() dmukhin
  3 siblings, 1 reply; 13+ messages in thread
From: dmukhin @ 2026-07-15 20:19 UTC (permalink / raw)
  To: xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini, dmukhin

From: Denis Mukhin <dmukhin@ford.com> 

Introduce CONFIG_PRINTK_RATELIMIT_MS and CONFIG_PRINTK_RATELIMIT_BURST
for configuring rate-limiting policy at the compile time.

Use symbols for global rate-limiting initialization in the console driver.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- moved rate-limiting controls to common/Kconfig
---
 xen/common/Kconfig         | 29 +++++++++++++++++++++++++++++
 xen/drivers/char/console.c |  6 ++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index da80fdba8469..fd22806884e0 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -672,4 +672,33 @@ config PM_STATS
 	  Enable collection of performance management statistics to aid in
 	  analyzing and tuning power/performance characteristics of the system
 
+menu "Console rate-limiting"
+
+config PRINTK_RATELIMIT_MS
+	int "printk rate-limiting time window (milliseconds)"
+	default 5000
+	help
+	  Specifies the time window, in milliseconds, for rate-limited printk
+	  messages. No more than `CONFIG_PRINTK_RATELIMIT_BURST` messages will be
+	  printed within this window.
+
+	  Setting this value to 0 disables rate-limiting entirely.
+
+	  Rate-limited messages are those controlled by the `loglvl` and
+	  `guest_loglvl` command-line parameters.
+
+config PRINTK_RATELIMIT_BURST
+	int "printk rate-limited message burst size"
+	default 10
+	help
+	  Defines the maximum number of rate-limited printk messages that may be
+	  printed within each `CONFIG_PRINTK_RATELIMIT_MS` time window.
+
+	  Setting this value to 0 disables rate-limiting entirely.
+
+	  Rate-limited messages are those controlled by the `loglvl` and
+	  `guest_loglvl` command-line parameters.
+
+endmenu
+
 endmenu
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index de9f2432445d..a9520f5521e4 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1323,10 +1323,12 @@ bool __printk_ratelimit(unsigned int ratelimit_ms,
 }
 
 /* Minimum time in ms between messages */
-static const unsigned int printk_ratelimit_ms = 5 * 1000;
+static const unsigned int printk_ratelimit_ms =
+    CONFIG_PRINTK_RATELIMIT_MS;
 
 /* Number of messages we send before ratelimiting */
-static const unsigned int printk_ratelimit_burst = 10;
+static const unsigned int printk_ratelimit_burst =
+    CONFIG_PRINTK_RATELIMIT_BURST;
 
 bool printk_ratelimit(void)
 {
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 4/4] xen/console: reduce number of printouts in __printk_ratelimit()
  2026-07-15 20:18 [PATCH v3 0/4] xen/console: updates to rate-limiting dmukhin
                   ` (2 preceding siblings ...)
  2026-07-15 20:19 ` [PATCH v3 3/4] xen/console: add build-time rate-limiting controls dmukhin
@ 2026-07-15 20:19 ` dmukhin
  2026-07-16 10:02   ` Teddy Astie
  2026-07-16 15:49   ` Jan Beulich
  3 siblings, 2 replies; 13+ messages in thread
From: dmukhin @ 2026-07-15 20:19 UTC (permalink / raw)
  To: xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini, dmukhin

From: Denis Mukhin <dmukhin@ford.com> 

Fold several __putstr() calls into one around notification of how many
messages have been rate-limited.

Not a functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- corrected the commit message
---
 xen/drivers/char/console.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index a9520f5521e4..f9a565fc59bc 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1303,15 +1303,15 @@ bool __printk_ratelimit(unsigned int ratelimit_ms,
         spin_unlock(&ratelimit_lock);
         if ( lost )
         {
-            char lost_str[10];
+            char msg[64];
+
+            snprintf(msg, sizeof(msg),
+                     "printk: %u messages suppressed\n", lost);
 
-            snprintf(lost_str, sizeof(lost_str), "%u", lost);
             /* console_lock may already be acquired by printk(). */
             rspin_lock(&console_lock);
             printk_start_of_line(CONSOLE_PREFIX);
-            __putstr("printk: ");
-            __putstr(lost_str);
-            __putstr(" messages suppressed.\n");
+            __putstr(msg);
             rspin_unlock(&console_lock);
         }
         local_irq_restore(flags);
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] xen/console: use bool as return value from printk_ratelimit()
  2026-07-15 20:18 ` [PATCH v3 1/4] xen/console: use bool as return value from printk_ratelimit() dmukhin
@ 2026-07-16  9:23   ` Teddy Astie
  0 siblings, 0 replies; 13+ messages in thread
From: Teddy Astie @ 2026-07-16  9:23 UTC (permalink / raw)
  To: dmukhin, xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini


[-- Attachment #1.1.1: Type: text/plain, Size: 391 bytes --]

Le 15/07/2026 à 22:24, dmukhin@ford.com a écrit :
> From: Denis Mukhin <dmukhin@ford.com>
> 
> {__,}printk_ratelimit() are predicates. Make that pronounced by updating the
> return value to boolean.
> 
> Not a functional change.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Teddy Astie <teddy.astie@vates.tech>

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2489 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter
  2026-07-15 20:19 ` [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter dmukhin
@ 2026-07-16  9:50   ` Teddy Astie
  2026-07-16 10:26     ` Jan Beulich
  2026-07-16 15:38   ` Jan Beulich
  1 sibling, 1 reply; 13+ messages in thread
From: Teddy Astie @ 2026-07-16  9:50 UTC (permalink / raw)
  To: dmukhin, xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini


[-- Attachment #1.1.1: Type: text/plain, Size: 2858 bytes --]

Le 15/07/2026 à 22:24, dmukhin@ford.com a écrit :
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Use existing printk_ratelimit_ms and printk_ratelimit_burst variables in
> do_printk_ratelimit() instead of hardcoded values 5000 and 10 respectively.
> 
> Ensure rate limiter is disabled if either printk_ratelimit_ms or
> printk_ratelimit_burst is 0. Make sure no unnecessary initialization is done
> in the corner case.
> 
> Also, simplify the limiter code by using min().
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v2:
> - fixed typing and 32-bit integer overflow problem
> ---
>   xen/drivers/char/console.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 5d395f882e08..de9f2432445d 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -1274,21 +1274,26 @@ bool __printk_ratelimit(unsigned int ratelimit_ms,
>                           unsigned int ratelimit_burst)
>   {
>       static DEFINE_SPINLOCK(ratelimit_lock);
> -    static unsigned long toks = 10 * 5 * 1000;
> -    static unsigned long last_msg;
> +    static unsigned long long toks, last_msg;
>       static unsigned int missed;
> +    unsigned long long now, limit;
>       unsigned long flags;
> -    unsigned long long now = NOW(); /* ns */
> -    unsigned long ms;
> +    s_time_t ms;
>   
> -    do_div(now, 1000000);
> -    ms = (unsigned long)now;
> +    if ( !ratelimit_burst || !ratelimit_burst )

Do you intend here ( !ratelimit_ms || !ratelimit_burst ) ?

> +        return true;
> +
> +    limit = min(ratelimit_burst * ratelimit_ms, UINT_MAX);

That looks no-op (at least at first stance), as UINT_MAX is the largest 
unsigned int value; hence `ratelimit_burst * ratelimit_ms` (both 
unsigned int) can't be larger than it; even if it overflows.

I think we need to cast both ratelimit_ms and ratelimit_burst to 
unsigned long long before doing the multiply, so that the multiply can't 
overflow (not sure exactly how to write it without being too verbose 
though).

https://godbolt.org/z/sxWaPM3cr

> +    if ( !toks )
> +        toks = limit;
> +
> +    now = NOW(); /* ns */
> +    ms = do_div(now, MILLISECS(1));
>   
>       spin_lock_irqsave(&ratelimit_lock, flags);
>       toks += ms - last_msg;
>       last_msg = ms;
> -    if ( toks > (ratelimit_burst * ratelimit_ms))
> -        toks = ratelimit_burst * ratelimit_ms;
> +    toks = min(toks, limit);
>       if ( toks >= ratelimit_ms )
>       {
>           unsigned int lost = missed;

With the if part fixed and the adjustments in the limit computation 
(cast to unsigned long long before the multiply) :
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>

Teddy

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2489 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/4] xen/console: add build-time rate-limiting controls
  2026-07-15 20:19 ` [PATCH v3 3/4] xen/console: add build-time rate-limiting controls dmukhin
@ 2026-07-16  9:59   ` Teddy Astie
  2026-07-16 15:44     ` Jan Beulich
  0 siblings, 1 reply; 13+ messages in thread
From: Teddy Astie @ 2026-07-16  9:59 UTC (permalink / raw)
  To: dmukhin, xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini


[-- Attachment #1.1.1: Type: text/plain, Size: 2524 bytes --]

Le 15/07/2026 à 22:24, dmukhin@ford.com a écrit :
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Introduce CONFIG_PRINTK_RATELIMIT_MS and CONFIG_PRINTK_RATELIMIT_BURST
> for configuring rate-limiting policy at the compile time.
> 
> Use symbols for global rate-limiting initialization in the console driver.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v2:
> - moved rate-limiting controls to common/Kconfig
> ---
>   xen/common/Kconfig         | 29 +++++++++++++++++++++++++++++
>   xen/drivers/char/console.c |  6 ++++--
>   2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index da80fdba8469..fd22806884e0 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -672,4 +672,33 @@ config PM_STATS
>   	  Enable collection of performance management statistics to aid in
>   	  analyzing and tuning power/performance characteristics of the system
>   
> +menu "Console rate-limiting"
> +
> +config PRINTK_RATELIMIT_MS
> +	int "printk rate-limiting time window (milliseconds)"
> +	default 5000
> +	help
> +	  Specifies the time window, in milliseconds, for rate-limited printk
> +	  messages. No more than `CONFIG_PRINTK_RATELIMIT_BURST` messages will be
> +	  printed within this window.
> +
> +	  Setting this value to 0 disables rate-limiting entirely.
> +
> +	  Rate-limited messages are those controlled by the `loglvl` and
> +	  `guest_loglvl` command-line parameters.
> +
> +config PRINTK_RATELIMIT_BURST
> +	int "printk rate-limited message burst size"
> +	default 10
> +	help
> +	  Defines the maximum number of rate-limited printk messages that may be
> +	  printed within each `CONFIG_PRINTK_RATELIMIT_MS` time window.
> +
> +	  Setting this value to 0 disables rate-limiting entirely.
> +
> +	  Rate-limited messages are those controlled by the `loglvl` and
> +	  `guest_loglvl` command-line parameters.
> +
> +endmenu
> +
>   endmenu

Given that setting 0 to either will disable rate limiting, would it be 
preferable instead to have a more general PRINTK_RATELIMIT toggle, where :

Setting it to true would enable configuration of ms and burst (rejecting 
0 if possible).
Setting it to false would set both to 0 hence disabling rate limiting by 
default.

And we do something like

static const unsigned int printk_ratelimit_ms =
   IS_ENABLED(CONFIG_PRINTK_RATELIMIT) ? CONFIG_PRINTK_RATELIMIT_MS : 0;

(similarly for burst)

?

Teddy

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2489 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 4/4] xen/console: reduce number of printouts in __printk_ratelimit()
  2026-07-15 20:19 ` [PATCH v3 4/4] xen/console: reduce number of printouts in __printk_ratelimit() dmukhin
@ 2026-07-16 10:02   ` Teddy Astie
  2026-07-16 15:49   ` Jan Beulich
  1 sibling, 0 replies; 13+ messages in thread
From: Teddy Astie @ 2026-07-16 10:02 UTC (permalink / raw)
  To: dmukhin, xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini


[-- Attachment #1.1.1: Type: text/plain, Size: 348 bytes --]

Le 15/07/2026 à 22:24, dmukhin@ford.com a écrit :
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Fold several __putstr() calls into one around notification of how many
> messages have been rate-limited.
> 
> Not a functional change.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Reviewed-by: Teddy Astie <teddy.astie@vates.tech>

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2489 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter
  2026-07-16  9:50   ` Teddy Astie
@ 2026-07-16 10:26     ` Jan Beulich
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2026-07-16 10:26 UTC (permalink / raw)
  To: Teddy Astie, dmukhin
  Cc: andrew.cooper3, anthony.perard, julien, michal.orzel, roger.pau,
	sstabellini, xen-devel

On 16.07.2026 11:50, Teddy Astie wrote:
> Le 15/07/2026 à 22:24, dmukhin@ford.com a écrit :
>> From: Denis Mukhin <dmukhin@ford.com>
>>
>> Use existing printk_ratelimit_ms and printk_ratelimit_burst variables in
>> do_printk_ratelimit() instead of hardcoded values 5000 and 10 respectively.
>>
>> Ensure rate limiter is disabled if either printk_ratelimit_ms or
>> printk_ratelimit_burst is 0. Make sure no unnecessary initialization is done
>> in the corner case.
>>
>> Also, simplify the limiter code by using min().
>>
>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>> ---
>> Changes since v2:
>> - fixed typing and 32-bit integer overflow problem
>> ---
>>   xen/drivers/char/console.c | 21 +++++++++++++--------
>>   1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
>> index 5d395f882e08..de9f2432445d 100644
>> --- a/xen/drivers/char/console.c
>> +++ b/xen/drivers/char/console.c
>> @@ -1274,21 +1274,26 @@ bool __printk_ratelimit(unsigned int ratelimit_ms,
>>                           unsigned int ratelimit_burst)
>>   {
>>       static DEFINE_SPINLOCK(ratelimit_lock);
>> -    static unsigned long toks = 10 * 5 * 1000;
>> -    static unsigned long last_msg;
>> +    static unsigned long long toks, last_msg;
>>       static unsigned int missed;
>> +    unsigned long long now, limit;
>>       unsigned long flags;
>> -    unsigned long long now = NOW(); /* ns */
>> -    unsigned long ms;
>> +    s_time_t ms;
>>   
>> -    do_div(now, 1000000);
>> -    ms = (unsigned long)now;
>> +    if ( !ratelimit_burst || !ratelimit_burst )
> 
> Do you intend here ( !ratelimit_ms || !ratelimit_burst ) ?
> 
>> +        return true;
>> +
>> +    limit = min(ratelimit_burst * ratelimit_ms, UINT_MAX);
> 
> That looks no-op (at least at first stance), as UINT_MAX is the largest 
> unsigned int value; hence `ratelimit_burst * ratelimit_ms` (both 
> unsigned int) can't be larger than it; even if it overflows.
> 
> I think we need to cast both ratelimit_ms and ratelimit_burst to 
> unsigned long long before doing the multiply, so that the multiply can't 
> overflow (not sure exactly how to write it without being too verbose 
> though).

No such casting please. __builtin_umul_overflow() is what wants using
for such purposes.

Jan


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter
  2026-07-15 20:19 ` [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter dmukhin
  2026-07-16  9:50   ` Teddy Astie
@ 2026-07-16 15:38   ` Jan Beulich
  1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2026-07-16 15:38 UTC (permalink / raw)
  To: dmukhin
  Cc: andrew.cooper3, anthony.perard, julien, michal.orzel, roger.pau,
	sstabellini, xen-devel

On 15.07.2026 22:19, dmukhin@ford.com wrote:
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -1274,21 +1274,26 @@ bool __printk_ratelimit(unsigned int ratelimit_ms,
>                          unsigned int ratelimit_burst)
>  {
>      static DEFINE_SPINLOCK(ratelimit_lock);
> -    static unsigned long toks = 10 * 5 * 1000;
> -    static unsigned long last_msg;
> +    static unsigned long long toks, last_msg;

Along the lines of what Teddy said, I question the need to widen the variables.

>      static unsigned int missed;
> +    unsigned long long now, limit;
>      unsigned long flags;
> -    unsigned long long now = NOW(); /* ns */
> -    unsigned long ms;
> +    s_time_t ms;

This is the type to use for e.g. NOW() return values; it isn't ...

> -    do_div(now, 1000000);
> -    ms = (unsigned long)now;
> +    if ( !ratelimit_burst || !ratelimit_burst )
> +        return true;
> +
> +    limit = min(ratelimit_burst * ratelimit_ms, UINT_MAX);
> +    if ( !toks )
> +        toks = limit;
> +
> +    now = NOW(); /* ns */
> +    ms = do_div(now, MILLISECS(1));

... a type to hold a millisecond granularity value.

Jan


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/4] xen/console: add build-time rate-limiting controls
  2026-07-16  9:59   ` Teddy Astie
@ 2026-07-16 15:44     ` Jan Beulich
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2026-07-16 15:44 UTC (permalink / raw)
  To: Teddy Astie, dmukhin
  Cc: andrew.cooper3, anthony.perard, julien, michal.orzel, roger.pau,
	sstabellini, xen-devel

On 16.07.2026 11:59, Teddy Astie wrote:
> Le 15/07/2026 à 22:24, dmukhin@ford.com a écrit :
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -672,4 +672,33 @@ config PM_STATS
>>   	  Enable collection of performance management statistics to aid in
>>   	  analyzing and tuning power/performance characteristics of the system
>>   
>> +menu "Console rate-limiting"
>> +
>> +config PRINTK_RATELIMIT_MS
>> +	int "printk rate-limiting time window (milliseconds)"
>> +	default 5000
>> +	help
>> +	  Specifies the time window, in milliseconds, for rate-limited printk
>> +	  messages. No more than `CONFIG_PRINTK_RATELIMIT_BURST` messages will be
>> +	  printed within this window.
>> +
>> +	  Setting this value to 0 disables rate-limiting entirely.
>> +
>> +	  Rate-limited messages are those controlled by the `loglvl` and
>> +	  `guest_loglvl` command-line parameters.
>> +
>> +config PRINTK_RATELIMIT_BURST
>> +	int "printk rate-limited message burst size"
>> +	default 10
>> +	help
>> +	  Defines the maximum number of rate-limited printk messages that may be
>> +	  printed within each `CONFIG_PRINTK_RATELIMIT_MS` time window.
>> +
>> +	  Setting this value to 0 disables rate-limiting entirely.
>> +
>> +	  Rate-limited messages are those controlled by the `loglvl` and
>> +	  `guest_loglvl` command-line parameters.
>> +
>> +endmenu
>> +
>>   endmenu
> 
> Given that setting 0 to either will disable rate limiting, would it be 
> preferable instead to have a more general PRINTK_RATELIMIT toggle, where :
> 
> Setting it to true would enable configuration of ms and burst (rejecting 
> 0 if possible).
> Setting it to false would set both to 0 hence disabling rate limiting by 
> default.
> 
> And we do something like
> 
> static const unsigned int printk_ratelimit_ms =
>    IS_ENABLED(CONFIG_PRINTK_RATELIMIT) ? CONFIG_PRINTK_RATELIMIT_MS : 0;
> 
> (similarly for burst)
> 
> ?

While I'm not quite sure we need to go this far (it's tidier overall, but
as it is right now the effect of disabling rate limiting is also clearly
stated), there's a security angle here: Disabling rate limiting altogether
must be clearly documented as not security supported. But we may need to
go beyond that, and also document as not security supported any lowering
of the default values.

Jan


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 4/4] xen/console: reduce number of printouts in __printk_ratelimit()
  2026-07-15 20:19 ` [PATCH v3 4/4] xen/console: reduce number of printouts in __printk_ratelimit() dmukhin
  2026-07-16 10:02   ` Teddy Astie
@ 2026-07-16 15:49   ` Jan Beulich
  1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2026-07-16 15:49 UTC (permalink / raw)
  To: dmukhin
  Cc: andrew.cooper3, anthony.perard, julien, michal.orzel, roger.pau,
	sstabellini, xen-devel

On 15.07.2026 22:19, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Fold several __putstr() calls into one around notification of how many
> messages have been rate-limited.
> 
> Not a functional change.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Hmm, the upsides and downsides aren't being discussed at all. You're ...

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -1303,15 +1303,15 @@ bool __printk_ratelimit(unsigned int ratelimit_ms,
>          spin_unlock(&ratelimit_lock);
>          if ( lost )
>          {
> -            char lost_str[10];
> +            char msg[64];
> +
> +            snprintf(msg, sizeof(msg),
> +                     "printk: %u messages suppressed\n", lost);
>  
> -            snprintf(lost_str, sizeof(lost_str), "%u", lost);
>              /* console_lock may already be acquired by printk(). */
>              rspin_lock(&console_lock);
>              printk_start_of_line(CONSOLE_PREFIX);
> -            __putstr("printk: ");
> -            __putstr(lost_str);
> -            __putstr(" messages suppressed.\n");
> +            __putstr(msg);
>              rspin_unlock(&console_lock);
>          }
>          local_irq_restore(flags);

... trading slightly smaller code size + slightly better performance for
slightly bigger stack utilization. It's not entirely obvious that this is
a good deal.

Jan


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-07-16 15:49 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 20:18 [PATCH v3 0/4] xen/console: updates to rate-limiting dmukhin
2026-07-15 20:18 ` [PATCH v3 1/4] xen/console: use bool as return value from printk_ratelimit() dmukhin
2026-07-16  9:23   ` Teddy Astie
2026-07-15 20:19 ` [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter dmukhin
2026-07-16  9:50   ` Teddy Astie
2026-07-16 10:26     ` Jan Beulich
2026-07-16 15:38   ` Jan Beulich
2026-07-15 20:19 ` [PATCH v3 3/4] xen/console: add build-time rate-limiting controls dmukhin
2026-07-16  9:59   ` Teddy Astie
2026-07-16 15:44     ` Jan Beulich
2026-07-15 20:19 ` [PATCH v3 4/4] xen/console: reduce number of printouts in __printk_ratelimit() dmukhin
2026-07-16 10:02   ` Teddy Astie
2026-07-16 15:49   ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.