Re: [PATCH v1] xen/console: remove __printk_ratelimit()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.07.2025 23:28, dmkhn@xxxxxxxxx wrote:
> On Thu, Jul 31, 2025 at 08:23:16AM +0200, Jan Beulich wrote:
>> On 30.07.2025 20:06, dmkhn@xxxxxxxxx wrote:
>>> On Wed, Jul 30, 2025 at 07:35:04AM +0200, Jan Beulich wrote:
>>>> On 30.07.2025 00:18, dmkhn@xxxxxxxxx wrote:
>>>>> On Mon, Jul 28, 2025 at 11:32:43AM +0200, Jan Beulich wrote:
>>>>>> On 26.07.2025 11:20, Julien Grall wrote:
>>>>>>> On 25/07/2025 22:24, dmkhn@xxxxxxxxx wrote:
>>>>>>>> From: Denis Mukhin <dmukhin@xxxxxxxx>
>>>>>>>>
>>>>>>>> __printk_ratelimit() is never used outside of the console driver.
>>>>>>>> Remove it from the lib.h and merge with the public printk_ratelimit().
>>>>>>>
>>>>>>> Is this solving any sort of violation? Asking because even if the
>>>>>>> function is only used by one caller, I could see a benefit to be able to
>>>>>>> use different value for the ratelimit. So I leaning towards keep the
>>>>>>> code as-is.
>>>>>>
>>>>>> +1
>>>>>>
>>>>>> In fact I'm surprised (or maybe not) that we still don't make better use
>>>>>> the rate limiting functionality.
>>>>>
>>>>> Out of curiosity, do you have any ideas re: make better use of the rate
>>>>> limiting functionality?
>>>>
>>>> No concrete ones; thinking about this has been way too long ago.
>>>>
>>>>> Build-time parameterization?
>>>>
>>>> That and/or command line controls.
>>>
>>> Got it.
>>>
>>> Can you please explain why exporting __printk_ratelimit() is still required
>>> for implementation of build/command line settings in console.c?
>>
>> I didn't say console.c, did I? Whatever subsystem wanted to do proper rate
> 
> But you also did not say anything about idea of having per-subsystem rate
> limiting.
> 
>> limiting would need to gain some way of controlling this (as said, build
>> time or cmdline driven), and it'll then need that function: How would it
>> effect the behavior with custom ms and/or burst values, without having
>> that function to call? (It is another thing that the function, using static
>> variables rather than per-caller state, may not be quite ready for that
>> kind of use. Also the arbitrary and hard-coded 10 * 5 * 1000 there would
>> probably also want to be customizable.)
>>
>> What you may want to do for Misra's sake is make the function static for
>> the time being. The compiler will then fold it into its sole caller,
>> until some new user appears. (At that occasion dropping one of the
>> underscores may also be reasonable.)
> 
> Do I understand it correctly that you will accept the following submission:
>  1) make __printk_ratelimit() static
>  2) drop one underscore from the name

Yes, if you really think that's worth it.

Jan

>  3) keep the only call __printk_ratelimit() in a hope of there will be
>     per-subsystem rate limiting in the future?
> 
> --
> Denis
> 
>>
>> Jan
>>
>