RE: [PATCH v2 10/19] xen/sysctl: introduce CONFIG_PM_STATS

["Penny, Zheng" <penny.zheng@xxxxxxx>]

[Public]

Hi,

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Wednesday, April 16, 2025 2:37 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Stabellini, Stefano <stefano.stabellini@xxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 10/19] xen/sysctl: introduce CONFIG_PM_STATS
>
> On 16.04.2025 05:54, Penny, Zheng wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: Tuesday, April 1, 2025 9:10 PM
> >>
> >> On 26.03.2025 06:50, Penny Zheng wrote:
> >>> --- a/xen/common/Kconfig
> >>> +++ b/xen/common/Kconfig
> >>> @@ -557,4 +557,9 @@ config SYSCTL
> >>>       to reduce Xen footprint.
> >>>  endmenu
> >>>
> >>> +config PM_STATS
> >>> +   bool "Enable Performance Management Statistics"
> >>> +   depends on ACPI && HAS_CPUFREQ && SYSCTL
> >>> +   default y
> >>
> >> As per above - either name, prompt and the description that Stefano
> >> suggested are wrong, or it is too much that is being covered by this new
> control.
> >>
> >
> > We have two sysctl-op on performance, do_get_pm_info() and do_pm_op().
> > I think do_get_pm_info() is to collect PM statistic info, which could
> > be wrapped with CONFIG_PM_STATS, while maybe do_pm_op() is more
> focusing on performance tuning.
> > How about we introduce another Kconfig CONFIG_PM_TUNE to wrap
> > do_pm_op() and related helpers? I suggest to introduce a new file pmtune.c 
> > to
> contain.
> > Or any better suggestion?
>
> "tune" is too narrow imo. "ctrl" may be an option, but how about simply 
> pm-op.c,
> fitting do_pm_op() pretty nicely? Question is what else is going to end up in 
> that
> file.
>

The following functions will be included in pm_op.c and wrapped with PM_CTRL or 
PM_OP
    - get_cpufreq_para()
    - set_cpufreq_para()
    - set_cpufreq_gov()
    - set_cpufreq_cppc()
    - cpufreq_driver_getavg()
    - cpufreq_update_turbo()
    - cpufreq_get_turbo_status()
And description for PM_STATS, I'll take stefano's suggestion and for 
PM_CTRL/PM_OP, maybe it will be
```
Enable userspace performance management control to do power/performance 
analyzing and tuning
```

> Jan