patches for DPDK stable branches
 help / color / mirror / Atom feed
From: David Hunt <david.hunt@intel.com>
To: Richael Zhuang <richael.zhuang@arm.com>, dev@dpdk.org
Cc: nd@arm.com, alan.carew@intel.com, stable@dpdk.org,
	Pablo de Lara <pablo.de.lara.guarch@intel.com>
Subject: Re: [dpdk-stable] [PATCH v5 1/2] test/power: add delay before checking cpuinfo cur freq
Date: Tue, 20 Apr 2021 14:15:19 +0100
Message-ID: <7b20f122-ddd2-11fb-68af-dc3ba1da2dbd@intel.com> (raw)
In-Reply-To: <86f24139-a638-ec83-ab52-fa19a28dd658@intel.com>


On 20/4/2021 1:38 PM, David Hunt wrote:
>
> On 15/4/2021 6:59 AM, Richael Zhuang wrote:
>> For some platforms the newly-set frequency may not be effective
>> immediately. If we didn't get the right value from cpuinfo_cur_freq
>> immediately, add 10ms delay each time before rechecking until
>> timeout.
>>
>>  From our test, for some arm platforms, it requires up to 700ms when
>> going from a minimum to a maximum frequency. And it's not the
>> driver/software issue.
>>
>> 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 | 27 ++++++++++++++++++++++-----
>>   1 file changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/app/test/test_power_cpufreq.c 
>> b/app/test/test_power_cpufreq.c
>> index 731c6b4dc..d47b3e0a1 100644
>> --- a/app/test/test_power_cpufreq.c
>> +++ b/app/test/test_power_cpufreq.c
>> @@ -8,6 +8,7 @@
>>   #include <limits.h>
>>   #include <string.h>
>>   #include <inttypes.h>
>> +#include <rte_cycles.h>
>>     #include "test.h"
>>   @@ -44,11 +45,13 @@ static int
>>   check_cur_freq(unsigned lcore_id, uint32_t idx)
>>   {
>>   #define TEST_POWER_CONVERT_TO_DECIMAL 10
>> +#define MAX_LOOP 100
>>       FILE *f;
>>       char fullpath[PATH_MAX];
>>       char buf[BUFSIZ];
>>       uint32_t cur_freq;
>>       int ret = -1;
>> +    int i;
>>         if (snprintf(fullpath, sizeof(fullpath),
>>           TEST_POWER_SYSFILE_CUR_FREQ, lcore_id) < 0) {
>> @@ -58,13 +61,27 @@ check_cur_freq(unsigned lcore_id, uint32_t idx)
>>       if (f == NULL) {
>>           return 0;
>>       }
>> -    if (fgets(buf, sizeof(buf), f) == NULL) {
>> -        goto fail_get_cur_freq;
>> +    for (i = 0; i < MAX_LOOP; i++) {
>> +        fflush(f);
>> +        if (fgets(buf, sizeof(buf), f) == NULL)
>> +            goto fail_all;
>> +
>> +        cur_freq = strtoul(buf, NULL, TEST_POWER_CONVERT_TO_DECIMAL);
>> +        ret = (freqs[idx] == cur_freq ? 0 : -1);
>> +
>> +        if (ret == 0)
>> +            break;
>> +
>> +        if (fseek(f, 0, SEEK_SET) < 0) {
>> +            printf("Fail to set file position indicator to 0\n");
>> +            goto fail_all;
>> +        }
>> +
>> +        /* wait for the value to be updated */
>> +        rte_delay_ms(10);
>>       }
>> -    cur_freq = strtoul(buf, NULL, TEST_POWER_CONVERT_TO_DECIMAL);
>> -    ret = (freqs[idx] == cur_freq ? 0 : -1);
>>   -fail_get_cur_freq:
>> +fail_all:
>>       fclose(f);
>>         return ret;
>
> Hi Richael
>
> On your system, is the current cpu frequency found in cpuinfo_cur_freq 
> or in scaling_cur_freq? On my system, which uses intel_pstate driver, 
> there is no file called cpuinfo_cur_freq, but the test works when I 
> change TEST_POWER_SYSFILE_CUR_FREQ to scaling_cur_freq.
>
> I know that's unrelated to your patch above, but it migth be worth 
> using a file common to all platforms, or else attempting to open one, 
> and if that fails, try open the other.
>
> Rgds,
> Dave.
>

Hi Richael,

     I've tested on other systems, where cpuinfo_cur_freq is present, 
and this patch works fine.

And even though 1 second seems like a very long time to change 
frequency, at leaset on responsive systems it will return quickly 
because of the retry mechanism you've added, and won't cause delays in 
the test.

I'll do a separate patch for checking both cpuinfo_cur_freq and 
scaling_cur_freq.

Reviewed-by: David Hunt <david.hunt@intel.com>





  reply	other threads:[~2021-04-20 13:15 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-06  3:04 [dpdk-stable] [PATCH v1 2/2] test/power: fix a bug in cpufreq autotest Richael Zhuang
2021-04-06  3:41 ` Richael Zhuang
2021-04-07  2:39   ` [dpdk-stable] [PATCH v2 " Richael Zhuang
     [not found]     ` <20210407074636.26891-1-richael.zhuang@arm.com>
2021-04-07  7:46       ` [dpdk-stable] [PATCH v3 1/3] test/power: round cpuinfo cur freq value " Richael Zhuang
     [not found]         ` <20210415053923.3447-1-richael.zhuang@arm.com>
2021-04-15  5:39           ` [dpdk-stable] [PATCH v4 1/2] " Richael Zhuang
2021-04-15  5:39           ` [dpdk-stable] [PATCH v4 2/2] test/power: add delay before checking cpuinfo cur freq Richael Zhuang
     [not found]         ` <20210415055930.3899-1-richael.zhuang@arm.com>
2021-04-15  5:59           ` [dpdk-stable] [PATCH v5 1/2] " Richael Zhuang
2021-04-20 12:38             ` David Hunt
2021-04-20 13:15               ` David Hunt [this message]
2021-04-21  2:41               ` Richael Zhuang
2021-04-15  5:59           ` [dpdk-stable] [PATCH v5 2/2] test/power: round cpuinfo cur freq value in cpufreq autotest Richael Zhuang
2021-04-20 14:01             ` David Hunt
2021-04-07  7:46       ` [dpdk-stable] [PATCH v3 2/3] test/power: fix a bug " Richael Zhuang
2021-04-07  9:58         ` [dpdk-stable] [dpdk-dev] " Burakov, Anatoly
2021-04-08  2:10           ` Richael Zhuang
2021-04-08 14:55             ` Burakov, Anatoly
2021-04-14  8:29               ` Richael Zhuang
2021-04-07  7:46       ` [dpdk-stable] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq Richael Zhuang
2021-04-07 10:14         ` [dpdk-stable] [dpdk-dev] " Liang Ma
2021-04-08  5:10           ` Richael Zhuang
2021-04-08  5:16           ` Richael Zhuang
2021-04-08  7:54             ` Liang Ma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7b20f122-ddd2-11fb-68af-dc3ba1da2dbd@intel.com \
    --to=david.hunt@intel.com \
    --cc=alan.carew@intel.com \
    --cc=dev@dpdk.org \
    --cc=nd@arm.com \
    --cc=pablo.de.lara.guarch@intel.com \
    --cc=richael.zhuang@arm.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git