Re: [PATCH v5 05/18] xen/cpufreq: refactor cmdline "cpufreq=xxx"

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2025 10:48, Penny Zheng wrote:
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -69,8 +69,55 @@ enum cpufreq_xen_opt __initdata cpufreq_xen_opts[2] = { 
> CPUFREQ_xen,
>                                                          CPUFREQ_none };
>  unsigned int __initdata cpufreq_xen_cnt = 1;
>  
> +static const char __initconst cpufreq_opts_str[][5] = {
> +    [CPUFREQ_none] = "none",
> +    [CPUFREQ_xen] = "xen",
> +    [CPUFREQ_hwp] = "hwp",
> +};
> +
>  static int __init cpufreq_cmdline_parse(const char *s, const char *e);
>  
> +static bool __init cpufreq_opts_contain(enum cpufreq_xen_opt option)
> +{
> +    unsigned int count = cpufreq_xen_cnt;
> +
> +    while ( count )
> +    {
> +        if ( cpufreq_xen_opts[--count] == option )

Instead of this, "while ( count-- )" would likely be slightly better;
less risk of an edit to the loop body (however unlikely that may seem
right now) then bypassing the decrement.

> +            return true;
> +    }
> +
> +    return false;
> +}
> +
> +static int __init handle_cpufreq_cmdline(enum cpufreq_xen_opt option)
> +{
> +    int ret = 0;
> +
> +    if ( cpufreq_opts_contain(option) )
> +    {
> +        printk(XENLOG_WARNING "Duplicate cpufreq driver option: %s\n",
> +               cpufreq_opts_str[option]);

Do we really need such a warning?

> +        return 0;
> +    }
> +
> +    cpufreq_controller = FREQCTL_xen;
> +    cpufreq_xen_opts[cpufreq_xen_cnt++] = option;

This could do with (at least) an assertion that we indeed don't overrun the
array.

Jan