Move some code around now that common xen_sysctl_pm_op get_para fields
are together. In particular, the scaling governor information like
scaling_available_governors is inside the union, so it is not always
available.
With that, gov_num may be 0, so bounce buffer handling needs
to be modified.
scaling_governor won't be filled for hwp, so this will simplify the
change when it is introduced.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
tools/libs/ctrl/xc_pm.c | 12 ++++++++----
tools/misc/xenpm.c | 3 ++-
xen/drivers/acpi/pmstat.c | 32 +++++++++++++++++---------------
3 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/tools/libs/ctrl/xc_pm.c b/tools/libs/ctrl/xc_pm.c
index f92542eaf7..19fe1a79dd 100644
--- a/tools/libs/ctrl/xc_pm.c
+++ b/tools/libs/ctrl/xc_pm.c
@@ -221,7 +221,7 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
{
if ( (!user_para->affected_cpus) ||
(!user_para->scaling_available_frequencies) ||
- (!user_para->scaling_available_governors) )
+ (user_para->gov_num && !user_para->scaling_available_governors) )
{
errno = EINVAL;
return -1;
@@ -230,12 +230,15 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
goto unlock_1;
if ( xc_hypercall_bounce_pre(xch, scaling_available_frequencies) )
goto unlock_2;
- if ( xc_hypercall_bounce_pre(xch, scaling_available_governors) )
+ if ( user_para->gov_num &&
+ xc_hypercall_bounce_pre(xch, scaling_available_governors) )
goto unlock_3;
set_xen_guest_handle(sys_para->affected_cpus, affected_cpus);
set_xen_guest_handle(sys_para->scaling_available_frequencies, scaling_available_frequencies);
- set_xen_guest_handle(sys_para->scaling_available_governors, scaling_available_governors);
+ if ( user_para->gov_num )
+ set_xen_guest_handle(sys_para->scaling_available_governors,
+ scaling_available_governors);
}
sysctl.cmd = XEN_SYSCTL_pm_op;
@@ -278,7 +281,8 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
}
unlock_4:
- xc_hypercall_bounce_post(xch, scaling_available_governors);
+ if ( user_para->gov_num )
+ xc_hypercall_bounce_post(xch, scaling_available_governors);
unlock_3:
xc_hypercall_bounce_post(xch, scaling_available_frequencies);
unlock_2:
diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index ee8ce5d5f2..1c474c3b59 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -811,7 +811,8 @@ static int show_cpufreq_para_by_cpuid(xc_interface *xc_handle, int cpuid)
ret = -ENOMEM;
goto out;
}
- if (!(p_cpufreq->scaling_available_governors =
+ if (p_cpufreq->gov_num &&
+ !(p_cpufreq->scaling_available_governors =
malloc(p_cpufreq->gov_num * CPUFREQ_NAME_LEN * sizeof(char))))
{
fprintf(stderr,
diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index f5a9ac3f1a..57359c21d8 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -239,11 +239,24 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
if ( ret )
return ret;
+ op->u.get_para.cpuinfo_cur_freq =
+ cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
+ op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
+ op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
+ op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
+
+ if ( cpufreq_driver.name[0] )
+ strlcpy(op->u.get_para.scaling_driver,
+ cpufreq_driver.name, CPUFREQ_NAME_LEN);
+ else
+ strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
+
if ( !(scaling_available_governors =
xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
return -ENOMEM;
- if ( (ret = read_scaling_available_governors(scaling_available_governors,
- gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
+ if ( (ret = read_scaling_available_governors(
+ scaling_available_governors,
+ gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
{
xfree(scaling_available_governors);
return ret;
@@ -254,26 +267,16 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
if ( ret )
return ret;
- op->u.get_para.cpuinfo_cur_freq =
- cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
- op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
- op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
-
op->u.get_para.u.s.scaling_cur_freq = policy->cur;
op->u.get_para.u.s.scaling_max_freq = policy->max;
op->u.get_para.u.s.scaling_min_freq = policy->min;
- if ( cpufreq_driver.name[0] )
- strlcpy(op->u.get_para.scaling_driver,
- cpufreq_driver.name, CPUFREQ_NAME_LEN);
- else
- strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
-
if ( policy->governor->name[0] )
strlcpy(op->u.get_para.u.s.scaling_governor,
policy->governor->name, CPUFREQ_NAME_LEN);
else
- strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown", CPUFREQ_NAME_LEN);
+ strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
+ CPUFREQ_NAME_LEN);
/* governor specific para */
if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
@@ -291,7 +294,6 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
&op->u.get_para.u.s.u.ondemand.sampling_rate,
&op->u.get_para.u.s.u.ondemand.up_threshold);
}
- op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
return ret;
}
--
2.40.1On 14.06.2023 20:02, Jason Andryuk wrote:
> Move some code around now that common xen_sysctl_pm_op get_para fields
> are together. In particular, the scaling governor information like
> scaling_available_governors is inside the union, so it is not always
> available.
>
> With that, gov_num may be 0, so bounce buffer handling needs
> to be modified.
>
> scaling_governor won't be filled for hwp, so this will simplify the
> change when it is introduced.
While I think this suitably describes the tool stack side changes, ...
> --- a/xen/drivers/acpi/pmstat.c
> +++ b/xen/drivers/acpi/pmstat.c
> @@ -239,11 +239,24 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> if ( ret )
> return ret;
>
> + op->u.get_para.cpuinfo_cur_freq =
> + cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
> + op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
> + op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
> + op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
> +
> + if ( cpufreq_driver.name[0] )
> + strlcpy(op->u.get_para.scaling_driver,
> + cpufreq_driver.name, CPUFREQ_NAME_LEN);
> + else
> + strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
> +
> if ( !(scaling_available_governors =
> xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
> return -ENOMEM;
> - if ( (ret = read_scaling_available_governors(scaling_available_governors,
> - gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
> + if ( (ret = read_scaling_available_governors(
> + scaling_available_governors,
> + gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
> {
> xfree(scaling_available_governors);
> return ret;
> @@ -254,26 +267,16 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> if ( ret )
> return ret;
>
> - op->u.get_para.cpuinfo_cur_freq =
> - cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
> - op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
> - op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
> -
> op->u.get_para.u.s.scaling_cur_freq = policy->cur;
> op->u.get_para.u.s.scaling_max_freq = policy->max;
> op->u.get_para.u.s.scaling_min_freq = policy->min;
>
> - if ( cpufreq_driver.name[0] )
> - strlcpy(op->u.get_para.scaling_driver,
> - cpufreq_driver.name, CPUFREQ_NAME_LEN);
> - else
> - strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
> -
> if ( policy->governor->name[0] )
> strlcpy(op->u.get_para.u.s.scaling_governor,
> policy->governor->name, CPUFREQ_NAME_LEN);
> else
> - strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown", CPUFREQ_NAME_LEN);
> + strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
> + CPUFREQ_NAME_LEN);
>
> /* governor specific para */
> if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
> @@ -291,7 +294,6 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> &op->u.get_para.u.s.u.ondemand.sampling_rate,
> &op->u.get_para.u.s.u.ondemand.up_threshold);
> }
> - op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
>
> return ret;
> }
... all I see on the hypervisor side is re-ordering of steps and re-formatting
of over-long lines. It's not clear to me why what you do is necessary for your
purpose.
Jan
On Thu, Jun 15, 2023 at 10:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.06.2023 20:02, Jason Andryuk wrote:
> > Move some code around now that common xen_sysctl_pm_op get_para fields
> > are together. In particular, the scaling governor information like
> > scaling_available_governors is inside the union, so it is not always
> > available.
> >
> > With that, gov_num may be 0, so bounce buffer handling needs
> > to be modified.
> >
> > scaling_governor won't be filled for hwp, so this will simplify the
> > change when it is introduced.
>
> While I think this suitably describes the tool stack side changes, ...
>
> > --- a/xen/drivers/acpi/pmstat.c
> > +++ b/xen/drivers/acpi/pmstat.c
> > @@ -239,11 +239,24 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> > if ( ret )
> > return ret;
> >
> > + op->u.get_para.cpuinfo_cur_freq =
> > + cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
> > + op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
> > + op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
> > + op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
> > +
> > + if ( cpufreq_driver.name[0] )
> > + strlcpy(op->u.get_para.scaling_driver,
> > + cpufreq_driver.name, CPUFREQ_NAME_LEN);
> > + else
> > + strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
> > +
> > if ( !(scaling_available_governors =
> > xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
> > return -ENOMEM;
> > - if ( (ret = read_scaling_available_governors(scaling_available_governors,
> > - gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
> > + if ( (ret = read_scaling_available_governors(
> > + scaling_available_governors,
> > + gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
> > {
> > xfree(scaling_available_governors);
> > return ret;
> > @@ -254,26 +267,16 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> > if ( ret )
> > return ret;
> >
> > - op->u.get_para.cpuinfo_cur_freq =
> > - cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
> > - op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
> > - op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
> > -
> > op->u.get_para.u.s.scaling_cur_freq = policy->cur;
> > op->u.get_para.u.s.scaling_max_freq = policy->max;
> > op->u.get_para.u.s.scaling_min_freq = policy->min;
> >
> > - if ( cpufreq_driver.name[0] )
> > - strlcpy(op->u.get_para.scaling_driver,
> > - cpufreq_driver.name, CPUFREQ_NAME_LEN);
> > - else
> > - strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
> > -
> > if ( policy->governor->name[0] )
> > strlcpy(op->u.get_para.u.s.scaling_governor,
> > policy->governor->name, CPUFREQ_NAME_LEN);
> > else
> > - strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown", CPUFREQ_NAME_LEN);
> > + strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
> > + CPUFREQ_NAME_LEN);
> >
> > /* governor specific para */
> > if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
> > @@ -291,7 +294,6 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> > &op->u.get_para.u.s.u.ondemand.sampling_rate,
> > &op->u.get_para.u.s.u.ondemand.up_threshold);
> > }
> > - op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
> >
> > return ret;
> > }
>
> ... all I see on the hypervisor side is re-ordering of steps and re-formatting
> of over-long lines. It's not clear to me why what you do is necessary for your
> purpose.
The purpose was to move accesses to the nested struct and union
"op->u.get_para.u.s.u" to the end of the function, and the accesses to
common fields (e.g. op->u.get_para.turbo_enabled) earlier. This
simplifies the changes in "cpufreq: Export HWP parameters to userspace
as CPPC". These governor fields get indented, and that needed some
re-formatting. Some re-formatting slipped in while I rebased changes
- sorry about that.
Regards,
Jason
On 15.06.2023 17:05, Jason Andryuk wrote:
> On Thu, Jun 15, 2023 at 10:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 14.06.2023 20:02, Jason Andryuk wrote:
>>> Move some code around now that common xen_sysctl_pm_op get_para fields
>>> are together. In particular, the scaling governor information like
>>> scaling_available_governors is inside the union, so it is not always
>>> available.
>>>
>>> With that, gov_num may be 0, so bounce buffer handling needs
>>> to be modified.
>>>
>>> scaling_governor won't be filled for hwp, so this will simplify the
>>> change when it is introduced.
>>
>> While I think this suitably describes the tool stack side changes, ...
>>
>>> --- a/xen/drivers/acpi/pmstat.c
>>> +++ b/xen/drivers/acpi/pmstat.c
>>> @@ -239,11 +239,24 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
>>> if ( ret )
>>> return ret;
>>>
>>> + op->u.get_para.cpuinfo_cur_freq =
>>> + cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
>>> + op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
>>> + op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
>>> + op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
>>> +
>>> + if ( cpufreq_driver.name[0] )
>>> + strlcpy(op->u.get_para.scaling_driver,
>>> + cpufreq_driver.name, CPUFREQ_NAME_LEN);
>>> + else
>>> + strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
>>> +
>>> if ( !(scaling_available_governors =
>>> xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
>>> return -ENOMEM;
>>> - if ( (ret = read_scaling_available_governors(scaling_available_governors,
>>> - gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
>>> + if ( (ret = read_scaling_available_governors(
>>> + scaling_available_governors,
>>> + gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
>>> {
>>> xfree(scaling_available_governors);
>>> return ret;
>>> @@ -254,26 +267,16 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
>>> if ( ret )
>>> return ret;
>>>
>>> - op->u.get_para.cpuinfo_cur_freq =
>>> - cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
>>> - op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
>>> - op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
>>> -
>>> op->u.get_para.u.s.scaling_cur_freq = policy->cur;
>>> op->u.get_para.u.s.scaling_max_freq = policy->max;
>>> op->u.get_para.u.s.scaling_min_freq = policy->min;
>>>
>>> - if ( cpufreq_driver.name[0] )
>>> - strlcpy(op->u.get_para.scaling_driver,
>>> - cpufreq_driver.name, CPUFREQ_NAME_LEN);
>>> - else
>>> - strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
>>> -
>>> if ( policy->governor->name[0] )
>>> strlcpy(op->u.get_para.u.s.scaling_governor,
>>> policy->governor->name, CPUFREQ_NAME_LEN);
>>> else
>>> - strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown", CPUFREQ_NAME_LEN);
>>> + strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
>>> + CPUFREQ_NAME_LEN);
>>>
>>> /* governor specific para */
>>> if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
>>> @@ -291,7 +294,6 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
>>> &op->u.get_para.u.s.u.ondemand.sampling_rate,
>>> &op->u.get_para.u.s.u.ondemand.up_threshold);
>>> }
>>> - op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
>>>
>>> return ret;
>>> }
>>
>> ... all I see on the hypervisor side is re-ordering of steps and re-formatting
>> of over-long lines. It's not clear to me why what you do is necessary for your
>> purpose.
>
> The purpose was to move accesses to the nested struct and union
> "op->u.get_para.u.s.u" to the end of the function, and the accesses to
> common fields (e.g. op->u.get_para.turbo_enabled) earlier. This
> simplifies the changes in "cpufreq: Export HWP parameters to userspace
> as CPPC".
Could you mention this as (part of) the purpose in the description?
> These governor fields get indented, and that needed some re-formatting.
Would it maybe make sense to split the function?
Jan
On Thu, Jun 15, 2023 at 11:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 15.06.2023 17:05, Jason Andryuk wrote:
> > On Thu, Jun 15, 2023 at 10:38 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 14.06.2023 20:02, Jason Andryuk wrote:
> >>> Move some code around now that common xen_sysctl_pm_op get_para fields
> >>> are together. In particular, the scaling governor information like
> >>> scaling_available_governors is inside the union, so it is not always
> >>> available.
> >>>
> >>> With that, gov_num may be 0, so bounce buffer handling needs
> >>> to be modified.
> >>>
> >>> scaling_governor won't be filled for hwp, so this will simplify the
> >>> change when it is introduced.
> >>
> >> While I think this suitably describes the tool stack side changes, ...
> >>
> >>> --- a/xen/drivers/acpi/pmstat.c
> >>> +++ b/xen/drivers/acpi/pmstat.c
> >>> @@ -239,11 +239,24 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> >>> if ( ret )
> >>> return ret;
> >>>
> >>> + op->u.get_para.cpuinfo_cur_freq =
> >>> + cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
> >>> + op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
> >>> + op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
> >>> + op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
> >>> +
> >>> + if ( cpufreq_driver.name[0] )
> >>> + strlcpy(op->u.get_para.scaling_driver,
> >>> + cpufreq_driver.name, CPUFREQ_NAME_LEN);
> >>> + else
> >>> + strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
> >>> +
> >>> if ( !(scaling_available_governors =
> >>> xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
> >>> return -ENOMEM;
> >>> - if ( (ret = read_scaling_available_governors(scaling_available_governors,
> >>> - gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
> >>> + if ( (ret = read_scaling_available_governors(
> >>> + scaling_available_governors,
> >>> + gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
> >>> {
> >>> xfree(scaling_available_governors);
> >>> return ret;
> >>> @@ -254,26 +267,16 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> >>> if ( ret )
> >>> return ret;
> >>>
> >>> - op->u.get_para.cpuinfo_cur_freq =
> >>> - cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
> >>> - op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
> >>> - op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
> >>> -
> >>> op->u.get_para.u.s.scaling_cur_freq = policy->cur;
> >>> op->u.get_para.u.s.scaling_max_freq = policy->max;
> >>> op->u.get_para.u.s.scaling_min_freq = policy->min;
> >>>
> >>> - if ( cpufreq_driver.name[0] )
> >>> - strlcpy(op->u.get_para.scaling_driver,
> >>> - cpufreq_driver.name, CPUFREQ_NAME_LEN);
> >>> - else
> >>> - strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
> >>> -
> >>> if ( policy->governor->name[0] )
> >>> strlcpy(op->u.get_para.u.s.scaling_governor,
> >>> policy->governor->name, CPUFREQ_NAME_LEN);
> >>> else
> >>> - strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown", CPUFREQ_NAME_LEN);
> >>> + strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
> >>> + CPUFREQ_NAME_LEN);
> >>>
> >>> /* governor specific para */
> >>> if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
> >>> @@ -291,7 +294,6 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> >>> &op->u.get_para.u.s.u.ondemand.sampling_rate,
> >>> &op->u.get_para.u.s.u.ondemand.up_threshold);
> >>> }
> >>> - op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
> >>>
> >>> return ret;
> >>> }
> >>
> >> ... all I see on the hypervisor side is re-ordering of steps and re-formatting
> >> of over-long lines. It's not clear to me why what you do is necessary for your
> >> purpose.
> >
> > The purpose was to move accesses to the nested struct and union
> > "op->u.get_para.u.s.u" to the end of the function, and the accesses to
> > common fields (e.g. op->u.get_para.turbo_enabled) earlier. This
> > simplifies the changes in "cpufreq: Export HWP parameters to userspace
> > as CPPC".
>
> Could you mention this as (part of) the purpose in the description?
Certainly.
> > These governor fields get indented, and that needed some re-formatting.
>
> Would it maybe make sense to split the function?
While that could be done, I don't think it's needed. Maybe you'll
feel otherwise after you look at "cpufreq: Export HWP parameters to
userspace as CPPC".
Regards,
Jason
© 2016 - 2026 Red Hat, Inc.