Re: [PATCH v4 04/15] xen/sysctl: Nest cpufreq scaling options

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.06.2023 20:02, Jason Andryuk wrote:
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -1909,16 +1909,20 @@ struct xc_get_cpufreq_para {
>      uint32_t cpuinfo_cur_freq;
>      uint32_t cpuinfo_max_freq;
>      uint32_t cpuinfo_min_freq;
> -    uint32_t scaling_cur_freq;
> -
> -    char scaling_governor[CPUFREQ_NAME_LEN];
> -    uint32_t scaling_max_freq;
> -    uint32_t scaling_min_freq;
> -
> -    /* for specific governor */
>      union {
> -        xc_userspace_t userspace;
> -        xc_ondemand_t ondemand;
> +        struct {
> +            uint32_t scaling_cur_freq;
> +
> +            char scaling_governor[CPUFREQ_NAME_LEN];
> +            uint32_t scaling_max_freq;
> +            uint32_t scaling_min_freq;
> +
> +            /* for specific governor */
> +            union {
> +                xc_userspace_t userspace;
> +                xc_ondemand_t ondemand;
> +            } u;
> +        } s;
>      } u;

There's no comment in the header that this needs to mirror the sysctl
struct. Does it really need changing?

> --- a/tools/libs/ctrl/xc_pm.c
> +++ b/tools/libs/ctrl/xc_pm.c
> @@ -265,15 +265,10 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
>          user_para->cpuinfo_cur_freq = sys_para->cpuinfo_cur_freq;
>          user_para->cpuinfo_max_freq = sys_para->cpuinfo_max_freq;
>          user_para->cpuinfo_min_freq = sys_para->cpuinfo_min_freq;
> -        user_para->scaling_cur_freq = sys_para->scaling_cur_freq;
> -        user_para->scaling_max_freq = sys_para->scaling_max_freq;
> -        user_para->scaling_min_freq = sys_para->scaling_min_freq;
>          user_para->turbo_enabled    = sys_para->turbo_enabled;
>  
>          memcpy(user_para->scaling_driver,
>                  sys_para->scaling_driver, CPUFREQ_NAME_LEN);
> -        memcpy(user_para->scaling_governor,
> -                sys_para->scaling_governor, CPUFREQ_NAME_LEN);

Did you really mean to remove the copying of these 4 entities, rather
than simply change the way the fields are accessed?

Jan