patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v1 2/2] test/power: fix a bug in cpufreq autotest
@ 2021-04-06  3:04 Richael Zhuang
  2021-04-06  3:41 ` Richael Zhuang
  0 siblings, 1 reply; 13+ messages in thread
From: Richael Zhuang @ 2021-04-06  3:04 UTC (permalink / raw)
  To: dev; +Cc: lukaszx.krakowiak, stable, David Hunt

For platforms that don't support turbo boost,rte_power_turbo_status()
returns "-ENOTSUP" (like power_kvm_vm_turbo_status()). So don't allow
check_power_turbo() to continue if rte_power_turbo_status(TEST_POWER_LCORE_ID)!=1

Fixes: aeaeaf5f2d62 ("test/power: add cases for turbo feature")
Cc: lukaszx.krakowiak@intel.com
Cc: stable@dpdk.org

CustomizedGitHooks: yes
Change-Id: I5702d5c647e941253c556ae2be8c1edeefacb618
Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
---
 app/test/test_power_cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
index c4f4541ac..7a93bc90a 100644
--- a/app/test/test_power_cpufreq.c
+++ b/app/test/test_power_cpufreq.c
@@ -389,7 +389,7 @@ check_power_turbo(void)
 {
        int ret;

-       if (rte_power_turbo_status(TEST_POWER_LCORE_ID) == 0) {
+       if (rte_power_turbo_status(TEST_POWER_LCORE_ID) != 1) {
                printf("Turbo not available on lcore %u, skipping test\n",
                                TEST_POWER_LCORE_ID);
                return 0;
--
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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH v1 2/2] test/power: fix a bug in cpufreq autotest
  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
  0 siblings, 1 reply; 13+ messages in thread
From: Richael Zhuang @ 2021-04-06  3:41 UTC (permalink / raw)
  To: dev; +Cc: lukaszx.krakowiak, stable, David Hunt

For platforms that don't support turbo boost,rte_power_turbo_status()
returns "-ENOTSUP" (like power_kvm_vm_turbo_status()). So don't allow
check_power_turbo() to continue if rte_power_turbo_status(TEST_POWER_LCORE_ID)!=1

Fixes: aeaeaf5f2d62 ("test/power: add cases for turbo feature")
Cc: lukaszx.krakowiak@intel.com
Cc: stable@dpdk.org

Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
---
 app/test/test_power_cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
index c4f4541ac..7a93bc90a 100644
--- a/app/test/test_power_cpufreq.c
+++ b/app/test/test_power_cpufreq.c
@@ -389,7 +389,7 @@ check_power_turbo(void)
 {
        int ret;

-       if (rte_power_turbo_status(TEST_POWER_LCORE_ID) == 0) {
+       if (rte_power_turbo_status(TEST_POWER_LCORE_ID) != 1) {
                printf("Turbo not available on lcore %u, skipping test\n",
                                TEST_POWER_LCORE_ID);
                return 0;
--
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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH v2 2/2] test/power: fix a bug in cpufreq autotest
  2021-04-06  3:41 ` Richael Zhuang
@ 2021-04-07  2:39   ` Richael Zhuang
       [not found]     ` <20210407074636.26891-1-richael.zhuang@arm.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Richael Zhuang @ 2021-04-07  2:39 UTC (permalink / raw)
  To: dev; +Cc: lukaszx.krakowiak, stable, David Hunt

For platforms that don't support turbo boost,rte_power_turbo_status()
returns "-ENOTSUP" (like power_kvm_vm_turbo_status()). So don't allow
check_power_turbo() to continue if rte_power_turbo_status(TEST_POWER_LCORE_ID)!=1

Fixes: aeaeaf5f2d62 ("test/power: add cases for turbo feature")
Cc: lukaszx.krakowiak@intel.com
Cc: stable@dpdk.org

Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
---
 app/test/test_power_cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
index c4f4541ac..7a93bc90a 100644
--- a/app/test/test_power_cpufreq.c
+++ b/app/test/test_power_cpufreq.c
@@ -389,7 +389,7 @@ check_power_turbo(void)
 {
 	int ret;
 
-	if (rte_power_turbo_status(TEST_POWER_LCORE_ID) == 0) {
+	if (rte_power_turbo_status(TEST_POWER_LCORE_ID) != 1) {
 		printf("Turbo not available on lcore %u, skipping test\n",
 				TEST_POWER_LCORE_ID);
 		return 0;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH v3 1/3] test/power: round cpuinfo cur freq value in cpufreq autotest
       [not found]     ` <20210407074636.26891-1-richael.zhuang@arm.com>
@ 2021-04-07  7:46       ` Richael Zhuang
  2021-04-07  7:46       ` [dpdk-stable] [PATCH v3 2/3] test/power: fix a bug " Richael Zhuang
  2021-04-07  7:46       ` [dpdk-stable] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq Richael Zhuang
  2 siblings, 0 replies; 13+ messages in thread
From: Richael Zhuang @ 2021-04-07  7:46 UTC (permalink / raw)
  To: dev; +Cc: alan.carew, stable, David Hunt, Pablo de Lara

The value in "/sys/.../cpuinfo_cur_freq" may not be exactly the
same as what was set. For example, if "2400000" is written to
"/sys/.../cpufreq/scaling_setspeed" to set the frequency, then the
value in "/sys/.../cpuinfo_cur_freq" may be "2401222". So need to
round the value.

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 | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
index 731c6b4dc..1f4d8bb05 100644
--- a/app/test/test_power_cpufreq.c
+++ b/app/test/test_power_cpufreq.c
@@ -34,6 +34,10 @@ test_power_caps(void)
 #define TEST_POWER_LCORE_INVALID ((unsigned)RTE_MAX_LCORE)
 #define TEST_POWER_FREQS_NUM_MAX ((unsigned)RTE_MAX_LCORE_FREQS)
 
+/* macros used for rounding frequency to nearest 100000 */
+#define TEST_FREQ_ROUNDING_DELTA 50000
+#define TEST_ROUND_FREQ_TO_N_100000 100000
+
 #define TEST_POWER_SYSFILE_CUR_FREQ \
 	"/sys/devices/system/cpu/cpu%u/cpufreq/cpuinfo_cur_freq"
 
@@ -62,7 +66,17 @@ check_cur_freq(unsigned lcore_id, uint32_t idx)
 		goto fail_get_cur_freq;
 	}
 	cur_freq = strtoul(buf, NULL, TEST_POWER_CONVERT_TO_DECIMAL);
-	ret = (freqs[idx] == cur_freq ? 0 : -1);
+
+	/* convert the frequency to nearest 100000 value
+	 * Ex: if cur_freq=1396789 then freq_conv=1400000
+	 * Ex: if cur_freq=800030 then freq_conv=800000
+	 */
+	unsigned int freq_conv = 0;
+	freq_conv = (cur_freq + TEST_FREQ_ROUNDING_DELTA)
+				/ TEST_ROUND_FREQ_TO_N_100000;
+	freq_conv = freq_conv * TEST_ROUND_FREQ_TO_N_100000;
+
+	ret = (freqs[idx] == freq_conv ? 0 : -1);
 
 fail_get_cur_freq:
 	fclose(f);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH v3 2/3] test/power: fix a bug in cpufreq autotest
       [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
@ 2021-04-07  7:46       ` Richael Zhuang
  2021-04-07  9:58         ` [dpdk-stable] [dpdk-dev] " Burakov, Anatoly
  2021-04-07  7:46       ` [dpdk-stable] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq Richael Zhuang
  2 siblings, 1 reply; 13+ messages in thread
From: Richael Zhuang @ 2021-04-07  7:46 UTC (permalink / raw)
  To: dev; +Cc: lukaszx.krakowiak, stable, David Hunt

For platforms that don't support turbo boost,rte_power_turbo_status()
returns "-ENOTSUP" (like power_kvm_vm_turbo_status()). So don't allow
check_power_turbo() to continue if rte_power_turbo_status(TEST_POWER_LCORE_ID)!=1

Fixes: aeaeaf5f2d62 ("test/power: add cases for turbo feature")
Cc: lukaszx.krakowiak@intel.com
Cc: stable@dpdk.org

Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
---
 app/test/test_power_cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
index 1f4d8bb05..cda74bd8a 100644
--- a/app/test/test_power_cpufreq.c
+++ b/app/test/test_power_cpufreq.c
@@ -386,7 +386,7 @@ check_power_turbo(void)
 {
 	int ret;
 
-	if (rte_power_turbo_status(TEST_POWER_LCORE_ID) == 0) {
+	if (rte_power_turbo_status(TEST_POWER_LCORE_ID) != 1) {
 		printf("Turbo not available on lcore %u, skipping test\n",
 				TEST_POWER_LCORE_ID);
 		return 0;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq
       [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
  2021-04-07  7:46       ` [dpdk-stable] [PATCH v3 2/3] test/power: fix a bug " Richael Zhuang
@ 2021-04-07  7:46       ` Richael Zhuang
  2021-04-07 10:14         ` [dpdk-stable] [dpdk-dev] " Liang Ma
  2 siblings, 1 reply; 13+ messages in thread
From: Richael Zhuang @ 2021-04-07  7:46 UTC (permalink / raw)
  To: dev; +Cc: alan.carew, stable, David Hunt, Pablo de Lara

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);
+
 #define TEST_POWER_CONVERT_TO_DECIMAL 10
 	FILE *f;
 	char fullpath[PATH_MAX];
-- 
2.20.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v3 2/3] test/power: fix a bug in cpufreq autotest
  2021-04-07  7:46       ` [dpdk-stable] [PATCH v3 2/3] test/power: fix a bug " Richael Zhuang
@ 2021-04-07  9:58         ` Burakov, Anatoly
  2021-04-08  2:10           ` Richael Zhuang
  0 siblings, 1 reply; 13+ messages in thread
From: Burakov, Anatoly @ 2021-04-07  9:58 UTC (permalink / raw)
  To: Richael Zhuang, dev; +Cc: lukaszx.krakowiak, stable, David Hunt

On 07-Apr-21 8:46 AM, Richael Zhuang wrote:
> For platforms that don't support turbo boost,rte_power_turbo_status()
> returns "-ENOTSUP" (like power_kvm_vm_turbo_status()). So don't allow
> check_power_turbo() to continue if rte_power_turbo_status(TEST_POWER_LCORE_ID)!=1
> 
> Fixes: aeaeaf5f2d62 ("test/power: add cases for turbo feature")
> Cc: lukaszx.krakowiak@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> ---
>   app/test/test_power_cpufreq.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
> index 1f4d8bb05..cda74bd8a 100644
> --- a/app/test/test_power_cpufreq.c
> +++ b/app/test/test_power_cpufreq.c
> @@ -386,7 +386,7 @@ check_power_turbo(void)
>   {
>   	int ret;
>   
> -	if (rte_power_turbo_status(TEST_POWER_LCORE_ID) == 0) {
> +	if (rte_power_turbo_status(TEST_POWER_LCORE_ID) != 1) {
>   		printf("Turbo not available on lcore %u, skipping test\n",
>   				TEST_POWER_LCORE_ID);
>   		return 0;
> 

If what you're really checking is -ENOTSUP, maybe check for that? 
Because otherwise it seems like you're making unwarranted assumptions 
about why the call failed...

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq
  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         ` Liang Ma
  2021-04-08  5:10           ` Richael Zhuang
  2021-04-08  5:16           ` Richael Zhuang
  0 siblings, 2 replies; 13+ messages in thread
From: Liang Ma @ 2021-04-07 10:14 UTC (permalink / raw)
  To: Richael Zhuang; +Cc: dev, alan.carew, stable, David Hunt, Pablo de Lara

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
> +
>  #define TEST_POWER_CONVERT_TO_DECIMAL 10
>  	FILE *f;
>  	char fullpath[PATH_MAX];
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v3 2/3] test/power: fix a bug in cpufreq autotest
  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
  0 siblings, 1 reply; 13+ messages in thread
From: Richael Zhuang @ 2021-04-08  2:10 UTC (permalink / raw)
  To: Burakov, Anatoly, dev; +Cc: lukaszx.krakowiak, stable, David Hunt

Hi,
Thanks for your comments.
My change  is to make  the check_power_turbo()  to continue only when rte_power_turbo_status(TEST_POWER_LCORE_ID) returns 1 which means turbo is available.

-----Original Message-----
From: Burakov, Anatoly <anatoly.burakov@intel.com>
Sent: Wednesday, April 7, 2021 5:59 PM
To: Richael Zhuang <Richael.Zhuang@arm.com>; dev@dpdk.org
Cc: lukaszx.krakowiak@intel.com; stable@dpdk.org; David Hunt <david.hunt@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 2/3] test/power: fix a bug in cpufreq autotest

On 07-Apr-21 8:46 AM, Richael Zhuang wrote:
> For platforms that don't support turbo boost,rte_power_turbo_status()
> returns "-ENOTSUP" (like power_kvm_vm_turbo_status()). So don't allow
> check_power_turbo() to continue if
> rte_power_turbo_status(TEST_POWER_LCORE_ID)!=1
>
> Fixes: aeaeaf5f2d62 ("test/power: add cases for turbo feature")
> Cc: lukaszx.krakowiak@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> ---
>   app/test/test_power_cpufreq.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/app/test/test_power_cpufreq.c
> b/app/test/test_power_cpufreq.c index 1f4d8bb05..cda74bd8a 100644
> --- a/app/test/test_power_cpufreq.c
> +++ b/app/test/test_power_cpufreq.c
> @@ -386,7 +386,7 @@ check_power_turbo(void)
>   {
>   int ret;
>
> -if (rte_power_turbo_status(TEST_POWER_LCORE_ID) == 0) {
> +if (rte_power_turbo_status(TEST_POWER_LCORE_ID) != 1) {
>   printf("Turbo not available on lcore %u, skipping test\n",
>   TEST_POWER_LCORE_ID);
>   return 0;
>

If what you're really checking is -ENOTSUP, maybe check for that?
Because otherwise it seems like you're making unwarranted assumptions about why the call failed...

--
Thanks,
Anatoly
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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq
  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
  1 sibling, 0 replies; 13+ messages in thread
From: Richael Zhuang @ 2021-04-08  5:10 UTC (permalink / raw)
  To: Liang Ma; +Cc: dev, alan.carew, stable, David Hunt, Pablo de Lara, nd

Hi Liang,

> -----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.
> > +
> >  #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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq
  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
  1 sibling, 1 reply; 13+ messages in thread
From: Richael Zhuang @ 2021-04-08  5:16 UTC (permalink / raw)
  To: Liang Ma; +Cc: dev, alan.carew, stable, David Hunt, Pablo de Lara, nd, nd

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.
> > +
> >  #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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v3 3/3] test/power: add delay before checking cpuinfo cur freq
  2021-04-08  5:16           ` Richael Zhuang
@ 2021-04-08  7:54             ` Liang Ma
  0 siblings, 0 replies; 13+ messages in thread
From: Liang Ma @ 2021-04-08  7:54 UTC (permalink / raw)
  To: Richael Zhuang; +Cc: dev, alan.carew, stable, David Hunt, Pablo de Lara, nd

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v3 2/3] test/power: fix a bug in cpufreq autotest
  2021-04-08  2:10           ` Richael Zhuang
@ 2021-04-08 14:55             ` Burakov, Anatoly
  0 siblings, 0 replies; 13+ messages in thread
From: Burakov, Anatoly @ 2021-04-08 14:55 UTC (permalink / raw)
  To: Richael Zhuang, dev; +Cc: lukaszx.krakowiak, stable, David Hunt

On 08-Apr-21 3:10 AM, Richael Zhuang wrote:
> Hi,
> Thanks for your comments.
> My change  is to make  the check_power_turbo()  to continue only when rte_power_turbo_status(TEST_POWER_LCORE_ID) returns 1 which means turbo is available.

Sure, but the code reads like if the turbo status isn't available, then 
the code just treats it as "unsupported", where in reality it could've 
been supported, but failed for some other reason.

> 
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Wednesday, April 7, 2021 5:59 PM
> To: Richael Zhuang <Richael.Zhuang@arm.com>; dev@dpdk.org
> Cc: lukaszx.krakowiak@intel.com; stable@dpdk.org; David Hunt <david.hunt@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/3] test/power: fix a bug in cpufreq autotest
> 
> On 07-Apr-21 8:46 AM, Richael Zhuang wrote:
>> For platforms that don't support turbo boost,rte_power_turbo_status()
>> returns "-ENOTSUP" (like power_kvm_vm_turbo_status()). So don't allow
>> check_power_turbo() to continue if
>> rte_power_turbo_status(TEST_POWER_LCORE_ID)!=1
>>
>> Fixes: aeaeaf5f2d62 ("test/power: add cases for turbo feature")
>> Cc: lukaszx.krakowiak@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
>> ---
>>    app/test/test_power_cpufreq.c | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/app/test/test_power_cpufreq.c
>> b/app/test/test_power_cpufreq.c index 1f4d8bb05..cda74bd8a 100644
>> --- a/app/test/test_power_cpufreq.c
>> +++ b/app/test/test_power_cpufreq.c
>> @@ -386,7 +386,7 @@ check_power_turbo(void)
>>    {
>>    int ret;
>>
>> -if (rte_power_turbo_status(TEST_POWER_LCORE_ID) == 0) {
>> +if (rte_power_turbo_status(TEST_POWER_LCORE_ID) != 1) {
>>    printf("Turbo not available on lcore %u, skipping test\n",
>>    TEST_POWER_LCORE_ID);
>>    return 0;
>>
> 
> If what you're really checking is -ENOTSUP, maybe check for that?
> Because otherwise it seems like you're making unwarranted assumptions about why the call failed...
> 
> --
> Thanks,
> Anatoly
> 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.
> 


-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-04-08 14:55 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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-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

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://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/ http://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