Re: [dpdk-stable] [dpdk-dev] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq

[Liang Ma <liangma@liangbit.com>]

On Thu, Apr 08, 2021 at 05:16:34AM +0000, Richael Zhuang wrote:
> Hi Liang,
> Sorry that last email contains "confidential notice", so I resend it.
> > -----Original Message-----
> > From: Liang Ma <liangma@liangbit.com>
> > Sent: Wednesday, April 7, 2021 6:15 PM
> > To: Richael Zhuang <Richael.Zhuang@arm.com>
> > Cc: dev@dpdk.org; alan.carew@intel.com; stable@dpdk.org; David Hunt
> > <david.hunt@intel.com>; Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v3 3/3] test/power: add delay before
> > checking cpuinfo cur freq
> > On Wed, Apr 07, 2021 at 03:46:36PM +0800, Richael Zhuang wrote:
> > > Sleep for 1s before checking the newly updated value from
> > > "/sys/devices/system/cpu/cpu%u/cpufreq/cpuinfo_cur_freq", because for
> > > some systems it may not be effective immediately.
> > >
> > > Fixes: ed7c51a6a680 ("app/test: vm power management")
> > > Cc: alan.carew@intel.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> > > ---
> > >  app/test/test_power_cpufreq.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/app/test/test_power_cpufreq.c
> > > b/app/test/test_power_cpufreq.c index cda74bd8a..7a93bc90a 100644
> > > --- a/app/test/test_power_cpufreq.c
> > > +++ b/app/test/test_power_cpufreq.c
> > > @@ -47,6 +47,9 @@ static uint32_t
> > freqs[TEST_POWER_FREQS_NUM_MAX];
> > > static int  check_cur_freq(unsigned lcore_id, uint32_t idx)  {
> > > +/* wait for the value to be updated */
> > > +sleep(1);
> > Hi Richael,
> >   1 second looks way too much for CPU frequency  swap.
> >   The unit should be ms in the worst case regardless the vendor, IMO.
> > Regards
> > Liang
> Thanks for review. Although I also think this time seems too long,
> it needs more than 700ms when I tested on our arm platform.
Hi Richael, 
   I will suggest you talk with arm cpufreq driver maintainer(kernel). 
   I don't think HW need 700ms to complete frequency changes. 
   cpufreq driver might do something here. intel_pstate driver set the
   threshold as 10ms, according to the kernel source code.  
Regards
Liang
> > > +
> > >  #define TEST_POWER_CONVERT_TO_DECIMAL 10
> > >  FILE *f;
> > >  char fullpath[PATH_MAX];
> > > --
> > > 2.20.1
> > >
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.