[dpdk-stable] [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling cur freq

[dpdk-stable] [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling cur freq

[Richael Zhuang]

For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
same. For acpi_cpufreq and cppc_cpufreq, we should check
cpuinfo_cur_freq. So here checking cpuinfo_cur_freq before
scaling_cur_freq to make sure it works for all cpufreq drivers.

Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
Cc: david.hunt@intel.com
Cc: stable@dpdk.org

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

diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
index b8fc53925c..f56abb6f86 100644
--- a/app/test/test_power_cpufreq.c
+++ b/app/test/test_power_cpufreq.c
@@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx, bool turbo)
 	int i;
 
 	if (snprintf(fullpath, sizeof(fullpath),
-		TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
+		TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
 		return 0;
 	}
 	f = fopen(fullpath, "r");
 	if (f == NULL) {
 		if (snprintf(fullpath, sizeof(fullpath),
-			TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
+			TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
 			return 0;
 		}
 		f = fopen(fullpath, "r");
-- 
2.20.1

Re: [dpdk-stable] [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling cur freq

[David Hunt]

On 14/7/2021 9:44 AM, Richael Zhuang wrote:
> For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
> scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
> And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
> same. For acpi_cpufreq and cppc_cpufreq, we should check
> cpuinfo_cur_freq. So here checking cpuinfo_cur_freq before
> scaling_cur_freq to make sure it works for all cpufreq drivers.
>
> Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
> Cc: david.hunt@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> ---
>   app/test/test_power_cpufreq.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
> index b8fc53925c..f56abb6f86 100644
> --- a/app/test/test_power_cpufreq.c
> +++ b/app/test/test_power_cpufreq.c
> @@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx, bool turbo)
>   	int i;
>   
>   	if (snprintf(fullpath, sizeof(fullpath),
> -		TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
> +		TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
>   		return 0;
>   	}
>   	f = fopen(fullpath, "r");
>   	if (f == NULL) {
>   		if (snprintf(fullpath, sizeof(fullpath),
> -			TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
> +			TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
>   			return 0;
>   		}
>   		f = fopen(fullpath, "r");


Hi Richael, I don't think this patch fixes anything. If the scaling file 
is not available, it will then attempt to open the cpuinfo file. 
Changing the order does not address the underlying issue.

It looks like the test is failing in check_cur_req, which is only 
rounding for cppc driver. I think it also needs to round for the other 
drivers. I've just checked intel_pstate driver now, and it needs the 
rounding. I would think that acpi driver also needs it. I'll do a bit 
more investigation and see if I can  change to acpi and attempt to 
confirm that all drivers need the rounding.

Rgds,
Dave.

Re: [dpdk-stable] [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling cur freq

[Richael Zhuang]

> -----Original Message-----
> From: David Hunt <david.hunt@intel.com>
> Sent: Wednesday, July 14, 2021 5:15 PM
> To: Richael Zhuang <Richael.Zhuang@arm.com>; dev@dpdk.org
> Cc: zhiminx.huang@intel.com; stable@dpdk.org
> Subject: Re: [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling
> cur freq
> 
> 
> On 14/7/2021 9:44 AM, Richael Zhuang wrote:
> > For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
> > scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
> > And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
> > same. For acpi_cpufreq and cppc_cpufreq, we should check
> > cpuinfo_cur_freq. So here checking cpuinfo_cur_freq before
> > scaling_cur_freq to make sure it works for all cpufreq drivers.
> >
> > Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
> > Cc: david.hunt@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> > ---
> >   app/test/test_power_cpufreq.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test/test_power_cpufreq.c
> > b/app/test/test_power_cpufreq.c index b8fc53925c..f56abb6f86 100644
> > --- a/app/test/test_power_cpufreq.c
> > +++ b/app/test/test_power_cpufreq.c
> > @@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx,
> bool turbo)
> >   	int i;
> >
> >   	if (snprintf(fullpath, sizeof(fullpath),
> > -		TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
> > +		TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
> >   		return 0;
> >   	}
> >   	f = fopen(fullpath, "r");
> >   	if (f == NULL) {
> >   		if (snprintf(fullpath, sizeof(fullpath),
> > -			TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0)
> {
> > +			TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0)
> {
> >   			return 0;
> >   		}
> >   		f = fopen(fullpath, "r");
> 
> 
> Hi Richael, I don't think this patch fixes anything. If the scaling file is not
> available, it will then attempt to open the cpuinfo file.
> Changing the order does not address the underlying issue.
> 
> It looks like the test is failing in check_cur_req, which is only rounding for
> cppc driver. I think it also needs to round for the other drivers. I've just
> checked intel_pstate driver now, and it needs the rounding. I would think
> that acpi driver also needs it. I'll do a bit more investigation and see if I
> can  change to acpi and attempt to confirm that all drivers need the rounding.
> 
> Rgds,
> Dave.
> 
> 
Hi David,
For acpi_cpufreq and cppc_cpufreq, both two files exist. So with the current code, it will check the scaling_cur_freq. But I think for this two drivers, it should check cpuinfo_cur_freq but not scaling_cur_freq. From my system, for acpi cpufreq, the value in cpuinfo_cur_freq and scaling_cur_freq are not the same.  So if not changing the check sequence, the result is:
##########
RTE>>power_cpufreq_autotest
POWER: Env isn't set yet!
POWER: Attempting to initialise ACPI cpufreq power management...
POWER: Initialized successfully for lcore 2 power management
POWER: Power management of lcore 2 has exited from 'userspace' mode and been set back to the original
POWER: Lcore id 128 can not exceeds 127
POWER: Initialized successfully for lcore 2 power management
POWER: Power management of lcore 2 is in use
POWER: Invalid lcore ID
POWER: NULL buffer supplied
POWER: Buffer size is not enough
POWER: Invalid lcore ID
POWER: Power management of lcore 2 has exited from 'userspace' mode and been set back to the original
Test Failed
RTE>>quit
#############

But after changing the check sequence, the result is OK from my test:
######
RTE>>power_cpufreq_autotest
POWER: Env isn't set yet!
POWER: Attempting to initialise ACPI cpufreq power management...
POWER: Initialized successfully for lcore 2 power management
POWER: Power management of lcore 2 has exited from 'userspace' mode and been set back to the original
POWER: Lcore id 128 can not exceeds 127
POWER: Initialized successfully for lcore 2 power management
POWER: Power management of lcore 2 is in use
POWER: Invalid lcore ID
POWER: NULL buffer supplied
POWER: Buffer size is not enough
POWER: Invalid lcore ID
POWER: Invalid lcore ID
POWER: Invalid frequency index 64, which should be less than 3
POWER: Invalid frequency index 3, which should be less than 3
POWER: Failed to enable turbo on lcore 2
POWER: Invalid lcore ID
POWER: Invalid lcore ID
POWER: Invalid lcore ID
POWER: Invalid lcore ID
Turbo not available on lcore 2, skipping test
POWER: Power management of lcore 2 has exited from 'userspace' mode and been set back to the original
POWER: Power management of lcore 2 is not used
POWER: Lcore id 128 can not exceeds 127
Test OK
############

Re: [dpdk-stable] [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling cur freq

[David Hunt]

On 14/7/2021 10:23 AM, Richael Zhuang wrote:
>
>> -----Original Message-----
>> From: David Hunt <david.hunt@intel.com>
>> Sent: Wednesday, July 14, 2021 5:15 PM
>> To: Richael Zhuang <Richael.Zhuang@arm.com>; dev@dpdk.org
>> Cc: zhiminx.huang@intel.com; stable@dpdk.org
>> Subject: Re: [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling
>> cur freq
>>
>>
>> On 14/7/2021 9:44 AM, Richael Zhuang wrote:
>>> For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
>>> scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
>>> And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
>>> same. For acpi_cpufreq and cppc_cpufreq, we should check
>>> cpuinfo_cur_freq. So here checking cpuinfo_cur_freq before
>>> scaling_cur_freq to make sure it works for all cpufreq drivers.
>>>
>>> Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
>>> Cc: david.hunt@intel.com
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
>>> ---
>>>    app/test/test_power_cpufreq.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/app/test/test_power_cpufreq.c
>>> b/app/test/test_power_cpufreq.c index b8fc53925c..f56abb6f86 100644
>>> --- a/app/test/test_power_cpufreq.c
>>> +++ b/app/test/test_power_cpufreq.c
>>> @@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx,
>> bool turbo)
>>>    	int i;
>>>
>>>    	if (snprintf(fullpath, sizeof(fullpath),
>>> -		TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
>>> +		TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
>>>    		return 0;
>>>    	}
>>>    	f = fopen(fullpath, "r");
>>>    	if (f == NULL) {
>>>    		if (snprintf(fullpath, sizeof(fullpath),
>>> -			TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0)
>> {
>>> +			TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0)
>> {
>>>    			return 0;
>>>    		}
>>>    		f = fopen(fullpath, "r");
>>
>> Hi Richael, I don't think this patch fixes anything. If the scaling file is not
>> available, it will then attempt to open the cpuinfo file.
>> Changing the order does not address the underlying issue.
>>
>> It looks like the test is failing in check_cur_req, which is only rounding for
>> cppc driver. I think it also needs to round for the other drivers. I've just
>> checked intel_pstate driver now, and it needs the rounding. I would think
>> that acpi driver also needs it. I'll do a bit more investigation and see if I
>> can  change to acpi and attempt to confirm that all drivers need the rounding.
>>
>> Rgds,
>> Dave.
>>
>>
> Hi David,
> For acpi_cpufreq and cppc_cpufreq, both two files exist. So with the current code, it will check the scaling_cur_freq. But I think for this two drivers, it should check cpuinfo_cur_freq but not scaling_cur_freq. From my system, for acpi cpufreq, the value in cpuinfo_cur_freq and scaling_cur_freq are not the same.


Ah OK. Now I see different the values in scaling_cur_freq and 
cpuinfo_cur_freq on a system here with acpi driver. The cpuinfo_cur_freq 
is already rounded, but the scaling_cur_freq is not.

# cat /sys/devices/system/cpu/cpu2/cpufreq/scaling_cur_freq
800259
# cat /sys/devices/system/cpu/cpu2/cpufreq/cpuinfo_cur_freq
800000

So I think it would make sense to include the re-order of the filenames 
as you suggest, but maybe also add rounding for the pstate driver, as 
that is needed.

-               if (env == PM_ENV_CPPC_CPUFREQ) {
+               if ((env == PM_ENV_CPPC_CPUFREQ) || (env == 
PM_ENV_PSTATE_CPUFREQ)) {

That avoids the complication of the acpi_cpufreq driver when turbo is 
enabled (2301000 for any turbo freq).

I've just run the test with the two changes (reorder filenames and round 
on pstate), and the power_cpufreq_autotest passes on both intel_pstate 
and acpi-cpufreq driver systems.

Regards,
Dave.



>    So if not changing the check sequence, the result is:
> ##########
> RTE>>power_cpufreq_autotest
> POWER: Env isn't set yet!
> POWER: Attempting to initialise ACPI cpufreq power management...
> POWER: Initialized successfully for lcore 2 power management
> POWER: Power management of lcore 2 has exited from 'userspace' mode and been set back to the original
> POWER: Lcore id 128 can not exceeds 127
> POWER: Initialized successfully for lcore 2 power management
> POWER: Power management of lcore 2 is in use
> POWER: Invalid lcore ID
> POWER: NULL buffer supplied
> POWER: Buffer size is not enough
> POWER: Invalid lcore ID
> POWER: Power management of lcore 2 has exited from 'userspace' mode and been set back to the original
> Test Failed
> RTE>>quit
> #############
>
> But after changing the check sequence, the result is OK from my test:
> ######
> RTE>>power_cpufreq_autotest
> POWER: Env isn't set yet!
> POWER: Attempting to initialise ACPI cpufreq power management...
> POWER: Initialized successfully for lcore 2 power management
> POWER: Power management of lcore 2 has exited from 'userspace' mode and been set back to the original
> POWER: Lcore id 128 can not exceeds 127
> POWER: Initialized successfully for lcore 2 power management
> POWER: Power management of lcore 2 is in use
> POWER: Invalid lcore ID
> POWER: NULL buffer supplied
> POWER: Buffer size is not enough
> POWER: Invalid lcore ID
> POWER: Invalid lcore ID
> POWER: Invalid frequency index 64, which should be less than 3
> POWER: Invalid frequency index 3, which should be less than 3
> POWER: Failed to enable turbo on lcore 2
> POWER: Invalid lcore ID
> POWER: Invalid lcore ID
> POWER: Invalid lcore ID
> POWER: Invalid lcore ID
> Turbo not available on lcore 2, skipping test
> POWER: Power management of lcore 2 has exited from 'userspace' mode and been set back to the original
> POWER: Power management of lcore 2 is not used
> POWER: Lcore id 128 can not exceeds 127
> Test OK
> ############

Re: [dpdk-stable] [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling cur freq

[Richael Zhuang]

> -----Original Message-----
> From: David Hunt <david.hunt@intel.com>
> Sent: Wednesday, July 14, 2021 6:14 PM
> To: Richael Zhuang <Richael.Zhuang@arm.com>; dev@dpdk.org
> Cc: zhiminx.huang@intel.com; stable@dpdk.org; nd <nd@arm.com>
> Subject: Re: [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling
> cur freq
> 
> 
> On 14/7/2021 10:23 AM, Richael Zhuang wrote:
> >
> >> -----Original Message-----
> >> From: David Hunt <david.hunt@intel.com>
> >> Sent: Wednesday, July 14, 2021 5:15 PM
> >> To: Richael Zhuang <Richael.Zhuang@arm.com>; dev@dpdk.org
> >> Cc: zhiminx.huang@intel.com; stable@dpdk.org
> >> Subject: Re: [PATCH v1 1/1] test/power: check cpuinfo cur freq before
> >> scaling cur freq
> >>
> >>
> >> On 14/7/2021 9:44 AM, Richael Zhuang wrote:
> >>> For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
> >>> scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
> >>> And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
> >>> same. For acpi_cpufreq and cppc_cpufreq, we should check
> >>> cpuinfo_cur_freq. So here checking cpuinfo_cur_freq before
> >>> scaling_cur_freq to make sure it works for all cpufreq drivers.
> >>>
> >>> Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
> >>> Cc: david.hunt@intel.com
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> >>> ---
> >>>    app/test/test_power_cpufreq.c | 4 ++--
> >>>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/app/test/test_power_cpufreq.c
> >>> b/app/test/test_power_cpufreq.c index b8fc53925c..f56abb6f86 100644
> >>> --- a/app/test/test_power_cpufreq.c
> >>> +++ b/app/test/test_power_cpufreq.c
> >>> @@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t
> >>> idx,
> >> bool turbo)
> >>>    	int i;
> >>>
> >>>    	if (snprintf(fullpath, sizeof(fullpath),
> >>> -		TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
> >>> +		TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
> >>>    		return 0;
> >>>    	}
> >>>    	f = fopen(fullpath, "r");
> >>>    	if (f == NULL) {
> >>>    		if (snprintf(fullpath, sizeof(fullpath),
> >>> -			TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0)
> >> {
> >>> +			TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0)
> >> {
> >>>    			return 0;
> >>>    		}
> >>>    		f = fopen(fullpath, "r");
> >>
> >> Hi Richael, I don't think this patch fixes anything. If the scaling
> >> file is not available, it will then attempt to open the cpuinfo file.
> >> Changing the order does not address the underlying issue.
> >>
> >> It looks like the test is failing in check_cur_req, which is only
> >> rounding for cppc driver. I think it also needs to round for the
> >> other drivers. I've just checked intel_pstate driver now, and it
> >> needs the rounding. I would think that acpi driver also needs it.
> >> I'll do a bit more investigation and see if I can  change to acpi and attempt
> to confirm that all drivers need the rounding.
> >>
> >> Rgds,
> >> Dave.
> >>
> >>
> > Hi David,
> > For acpi_cpufreq and cppc_cpufreq, both two files exist. So with the
> current code, it will check the scaling_cur_freq. But I think for this two drivers,
> it should check cpuinfo_cur_freq but not scaling_cur_freq. From my system,
> for acpi cpufreq, the value in cpuinfo_cur_freq and scaling_cur_freq are not
> the same.
> 
> 
> Ah OK. Now I see different the values in scaling_cur_freq and
> cpuinfo_cur_freq on a system here with acpi driver. The cpuinfo_cur_freq is
> already rounded, but the scaling_cur_freq is not.
> 
> # cat /sys/devices/system/cpu/cpu2/cpufreq/scaling_cur_freq
> 800259
> # cat /sys/devices/system/cpu/cpu2/cpufreq/cpuinfo_cur_freq
> 800000
> 
> So I think it would make sense to include the re-order of the filenames as you
> suggest, but maybe also add rounding for the pstate driver, as that is needed.
> 
> -               if (env == PM_ENV_CPPC_CPUFREQ) {
> +               if ((env == PM_ENV_CPPC_CPUFREQ) || (env ==
> PM_ENV_PSTATE_CPUFREQ)) {
> 
> That avoids the complication of the acpi_cpufreq driver when turbo is
> enabled (2301000 for any turbo freq).
> 
> I've just run the test with the two changes (reorder filenames and round
> on pstate), and the power_cpufreq_autotest passes on both intel_pstate
> and acpi-cpufreq driver systems.
> 
> Regards,
> Dave.
> 
> 
Hi David,
For I don't have a platform with pstate available now,  I can’t  verify it by myself. Thanks for your test and conclusion, I will include" env ==
 PM_ENV_PSTATE_CPUFREQ".

> 
> >    So if not changing the check sequence, the result is:
> > ##########
> > RTE>>power_cpufreq_autotest
> > POWER: Env isn't set yet!
> > POWER: Attempting to initialise ACPI cpufreq power management...
> > POWER: Initialized successfully for lcore 2 power management
> > POWER: Power management of lcore 2 has exited from 'userspace' mode
> and been set back to the original
> > POWER: Lcore id 128 can not exceeds 127
> > POWER: Initialized successfully for lcore 2 power management
> > POWER: Power management of lcore 2 is in use
> > POWER: Invalid lcore ID
> > POWER: NULL buffer supplied
> > POWER: Buffer size is not enough
> > POWER: Invalid lcore ID
> > POWER: Power management of lcore 2 has exited from 'userspace' mode
> and been set back to the original
> > Test Failed
> > RTE>>quit
> > #############
> >
> > But after changing the check sequence, the result is OK from my test:
> > ######
> > RTE>>power_cpufreq_autotest
> > POWER: Env isn't set yet!
> > POWER: Attempting to initialise ACPI cpufreq power management...
> > POWER: Initialized successfully for lcore 2 power management
> > POWER: Power management of lcore 2 has exited from 'userspace' mode
> and been set back to the original
> > POWER: Lcore id 128 can not exceeds 127
> > POWER: Initialized successfully for lcore 2 power management
> > POWER: Power management of lcore 2 is in use
> > POWER: Invalid lcore ID
> > POWER: NULL buffer supplied
> > POWER: Buffer size is not enough
> > POWER: Invalid lcore ID
> > POWER: Invalid lcore ID
> > POWER: Invalid frequency index 64, which should be less than 3
> > POWER: Invalid frequency index 3, which should be less than 3
> > POWER: Failed to enable turbo on lcore 2
> > POWER: Invalid lcore ID
> > POWER: Invalid lcore ID
> > POWER: Invalid lcore ID
> > POWER: Invalid lcore ID
> > Turbo not available on lcore 2, skipping test
> > POWER: Power management of lcore 2 has exited from 'userspace' mode
> and been set back to the original
> > POWER: Power management of lcore 2 is not used
> > POWER: Lcore id 128 can not exceeds 127
> > Test OK
> > ############

[dpdk-stable] [PATCH v2 1/1] test/power: fix CPU frequency check

[Richael Zhuang]

For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
same. For acpi_cpufreq and cppc_cpufreq, we should check
cpuinfo_cur_freq but not scaling_cur_freq. So here change the
check sequence to make sure it works for all cpufreq drivers.
Besides, add rounding for pstate driver.

Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
Cc: david.hunt@intel.com
Cc: stable@dpdk.org

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

diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
index b8fc53925c..1a9549527e 100644
--- a/app/test/test_power_cpufreq.c
+++ b/app/test/test_power_cpufreq.c
@@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx, bool turbo)
 	int i;
 
 	if (snprintf(fullpath, sizeof(fullpath),
-		TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
+		TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
 		return 0;
 	}
 	f = fopen(fullpath, "r");
 	if (f == NULL) {
 		if (snprintf(fullpath, sizeof(fullpath),
-			TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
+			TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
 			return 0;
 		}
 		f = fopen(fullpath, "r");
@@ -85,7 +85,7 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx, bool turbo)
 		freq_conv = cur_freq;
 
 		env = rte_power_get_env();
-		if (env == PM_ENV_CPPC_CPUFREQ) {
+		if (env == PM_ENV_CPPC_CPUFREQ || env == PM_ENV_PSTATE_CPUFREQ) {
 			/* 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
-- 
2.20.1

Re: [dpdk-stable] [PATCH v2 1/1] test/power: fix CPU frequency check

[David Hunt]

Hi Richael,

On 14/7/2021 11:44 AM, Richael Zhuang wrote:
> For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
> scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
> And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
> same. For acpi_cpufreq and cppc_cpufreq, we should check
> cpuinfo_cur_freq but not scaling_cur_freq. So here change the
> check sequence to make sure it works for all cpufreq drivers.
> Besides, add rounding for pstate driver.
>
> Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
> Cc: david.hunt@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> ---
>   app/test/test_power_cpufreq.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/app/test/test_power_cpufreq.c b/app/test/test_power_cpufreq.c
> index b8fc53925c..1a9549527e 100644
> --- a/app/test/test_power_cpufreq.c
> +++ b/app/test/test_power_cpufreq.c
> @@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx, bool turbo)
>   	int i;
>   
>   	if (snprintf(fullpath, sizeof(fullpath),
> -		TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
> +		TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
>   		return 0;
>   	}
>   	f = fopen(fullpath, "r");
>   	if (f == NULL) {
>   		if (snprintf(fullpath, sizeof(fullpath),
> -			TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
> +			TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
>   			return 0;
>   		}
>   		f = fopen(fullpath, "r");
> @@ -85,7 +85,7 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx, bool turbo)
>   		freq_conv = cur_freq;
>   
>   		env = rte_power_get_env();
> -		if (env == PM_ENV_CPPC_CPUFREQ) {
> +		if (env == PM_ENV_CPPC_CPUFREQ || env == PM_ENV_PSTATE_CPUFREQ) {
>   			/* 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


Looks good now. I ran the power_cpufreq_autotest test on both the 
acpi-cpufreq and pstate drivers, and both passed.

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

Re: [dpdk-stable] [PATCH v2 1/1] test/power: fix CPU frequency check

[Richael Zhuang]

> -----Original Message-----
> From: David Hunt <david.hunt@intel.com>
> Sent: Wednesday, July 14, 2021 8:23 PM
> To: Richael Zhuang <Richael.Zhuang@arm.com>; dev@dpdk.org
> Cc: zhiminx.huang@intel.com; stable@dpdk.org
> Subject: Re: [PATCH v2 1/1] test/power: fix CPU frequency check
> 
> Hi Richael,
> 
> On 14/7/2021 11:44 AM, Richael Zhuang wrote:
> > For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
> > scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
> > And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
> > same. For acpi_cpufreq and cppc_cpufreq, we should check
> > cpuinfo_cur_freq but not scaling_cur_freq. So here change the check
> > sequence to make sure it works for all cpufreq drivers.
> > Besides, add rounding for pstate driver.
> >
> > Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
> > Cc: david.hunt@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> > ---
> >   app/test/test_power_cpufreq.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/app/test/test_power_cpufreq.c
> > b/app/test/test_power_cpufreq.c index b8fc53925c..1a9549527e 100644
> > --- a/app/test/test_power_cpufreq.c
> > +++ b/app/test/test_power_cpufreq.c
> > @@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx,
> bool turbo)
> >   	int i;
> >
> >   	if (snprintf(fullpath, sizeof(fullpath),
> > -		TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
> > +		TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
> >   		return 0;
> >   	}
> >   	f = fopen(fullpath, "r");
> >   	if (f == NULL) {
> >   		if (snprintf(fullpath, sizeof(fullpath),
> > -			TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0)
> {
> > +			TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0)
> {
> >   			return 0;
> >   		}
> >   		f = fopen(fullpath, "r");
> > @@ -85,7 +85,7 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx,
> bool turbo)
> >   		freq_conv = cur_freq;
> >
> >   		env = rte_power_get_env();
> > -		if (env == PM_ENV_CPPC_CPUFREQ) {
> > +		if (env == PM_ENV_CPPC_CPUFREQ || env ==
> PM_ENV_PSTATE_CPUFREQ) {
> >   			/* 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
> 
> 
> Looks good now. I ran the power_cpufreq_autotest test on both the acpi-
> cpufreq and pstate drivers, and both passed.
> 
> Acked-by: David Hunt <david.hunt@intel.com>
> 
Thank you for testing it!
> 

Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/1] test/power: fix CPU frequency check

[David Marchand]

On Wed, Jul 14, 2021 at 2:23 PM David Hunt <david.hunt@intel.com> wrote:
> On 14/7/2021 11:44 AM, Richael Zhuang wrote:
> > For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
> > scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
> > And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
> > same. For acpi_cpufreq and cppc_cpufreq, we should check
> > cpuinfo_cur_freq but not scaling_cur_freq. So here change the
> > check sequence to make sure it works for all cpufreq drivers.
> > Besides, add rounding for pstate driver.
> >
> > Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
> Acked-by: David Hunt <david.hunt@intel.com>

Applied, thanks.


-- 
David Marchand