Re: [PATCH v2 5/7] xen/console: correct leaky-bucket rate limiter

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.02.2026 21:24, dmukhin@xxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx> 
> 
> 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@xxxxxxxx>
> ---
> Changes since v1:
> - new patch
> ---
>  xen/drivers/char/console.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index f607e8f84d7a..e3962512d282 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -1291,21 +1291,27 @@ static bool do_printk_ratelimit(unsigned int 
> ratelimit_ms,
>                                  unsigned int ratelimit_burst)
>  {
>      static DEFINE_SPINLOCK(ratelimit_lock);
> -    static unsigned long toks = 10 * 5 * 1000;

Shouldn't this have been adjusted already in patches 3 and 4?

> +    static unsigned long toks;
>      static unsigned long last_msg;
>      static unsigned int missed;
>      unsigned long flags;
> -    unsigned long long now = NOW(); /* ns */
> +    unsigned long long now;
>      unsigned long ms;
>  
> +    if ( !printk_ratelimit_burst || !printk_ratelimit_burst )
> +        return true;
> +
> +    if ( !toks )
> +        toks = printk_ratelimit_burst * printk_ratelimit_ms;

This multiplication can overflow (which is avoidable on 64-bit arch-es, but
needs checking for on 32-bit ones in any event).

> +    now = NOW(); /* ns */
>      do_div(now, 1000000);
>      ms = (unsigned long)now;
>  
>      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, (unsigned long)(ratelimit_burst * ratelimit_ms));

If you need the product anyway, calculate it once (correctly) and then you
won't need a cast here either.

Jan