On 16.04.2025 10:38, Penny, Zheng wrote: > [Public] > > Hi, > >> -----Original Message----- >> From: Jan Beulich <jbeul...@suse.com> >> Sent: Wednesday, April 16, 2025 2:37 PM >> To: Penny, Zheng <penny.zh...@amd.com> >> Cc: Stabellini, Stefano <stefano.stabell...@amd.com>; xen- >> de...@lists.xenproject.org >> 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 <jbeul...@suse.com> >>>> 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
To avoid another round trip just for this - I wrote pm-op.c for a reason. > 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 > ``` Reads okay at the first glance. Jan
