Re: [PATCH v3 2/4] xen/console: correct leaky-bucket rate limiter

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.07.2026 11:50, Teddy Astie wrote:
> Le 15/07/2026 à 22:24, dmukhin@xxxxxxxx a écrit :
>> 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 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