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

[dmkhn@proton.me]

Hi Jan,

Thanks for the feedback.

On Thu, Jul 31, 2025 at 08:23:16AM +0200, Jan Beulich wrote:
> On 30.07.2025 20:06, dmkhn@proton.me wrote:
> > On Wed, Jul 30, 2025 at 07:35:04AM +0200, Jan Beulich wrote:
> >> On 30.07.2025 00:18, dmkhn@proton.me 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@proton.me wrote:
> >>>>>> From: Denis Mukhin <dmukhin@ford.com>
> >>>>>>
> >>>>>> __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
 3) keep the only call __printk_ratelimit() in a hope of there will be
    per-subsystem rate limiting in the future?

--
Denis

> 
> Jan
>