[Public] Hi,
> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: Wednesday, April 30, 2025 10:42 PM > To: Penny, Zheng <penny.zh...@amd.com> > Cc: Huang, Ray <ray.hu...@amd.com>; Anthony PERARD > <anthony.per...@vates.tech>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH v4 14/15] tools/xenpm: remove px_cap dependency check > for average frequency > > On 14.04.2025 09:40, Penny Zheng wrote: > > In `xenpm start` command, the monitor of average frequency shall not > > depend on the existence of legacy P-states, so removing "px_cap" > > dependancy check. > > Well, yes, I agree there. But can you explain to me what the file scope > "avgfreq" is > good for? Shouldn't we go farther and tidy things more thoroughly? Much like > show_cpufreq_by_cpuid(), we could call > get_avgfreq_by_cpuid() right before printing. (It escapes me altogether why > start_gather_func() would pre-fill the array. Unlike cxstat_start[] and > pxstat_start[] > that's not incrementally or differentially used data.) > Right, I think avgfreq[] shall be just like cxstat_start[] and pxstat_start[], getting pre-filled in start_gather_func() too, maybe we don't need a avgfreq_start[] to actually record the numbers. What we achieve there is that in get_messured_perf(), we could let per_cpu(usr_perf_pair, cpu) save APERF/MPERF value at start_gather_func() point. Then, later, in signal_int_handler(), running get_avgfreq_by_cpuid() again could actually let get_messured_perf() calculate APERF/MPERF incrementation happened between `xenpm start` duration. Right now, we are calculating incrementation between boot time, or last `xenpm start` command till now > Doing that would make yet more obvious that the px_cap part of the condition > was > bogus presumably from the very start. (I'm further inclined to say that this > change > should also have a Fixes: tag.) > > Jan
