From: Richael Zhuang <Richael.Zhuang@arm.com>
To: David Hunt <david.hunt@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "zhiminx.huang@intel.com" <zhiminx.huang@intel.com>,
"stable@dpdk.org" <stable@dpdk.org>, nd <nd@arm.com>,
nd <nd@arm.com>
Subject: Re: [dpdk-stable] [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling cur freq
Date: Wed, 14 Jul 2021 09:23:54 +0000 [thread overview]
Message-ID: <AM8PR08MB5796FD61B2F29E5A0DF485B792139@AM8PR08MB5796.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <7fc966da-9eb1-4fe8-2908-d10bc83f4c20@intel.com>
> -----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
############
next prev parent reply other threads:[~2021-07-14 9:24 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210714084407.51979-1-richael.zhuang@arm.com>
2021-07-14 8:44 ` Richael Zhuang
2021-07-14 9:15 ` David Hunt
2021-07-14 9:23 ` Richael Zhuang [this message]
2021-07-14 10:13 ` David Hunt
2021-07-14 10:29 ` Richael Zhuang
[not found] ` <20210714104405.23917-1-richael.zhuang@arm.com>
2021-07-14 10:44 ` [dpdk-stable] [PATCH v2 1/1] test/power: fix CPU frequency check Richael Zhuang
2021-07-14 12:23 ` David Hunt
2021-07-15 1:44 ` Richael Zhuang
2021-07-20 15:20 ` [dpdk-stable] [dpdk-dev] " David Marchand
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=AM8PR08MB5796FD61B2F29E5A0DF485B792139@AM8PR08MB5796.eurprd08.prod.outlook.com \
--to=richael.zhuang@arm.com \
--cc=david.hunt@intel.com \
--cc=dev@dpdk.org \
--cc=nd@arm.com \
--cc=stable@dpdk.org \
--cc=zhiminx.huang@intel.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).