[v1,3/4] test/power: removed function pointer validations
Checks
Commit Message
After refactoring the power library, power management operations are now
consistently supported regardless of the operating environment, making
function pointer checks unnecessary and thus removed from applications.
Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
---
app/test/test_power.c | 95 -----------------------------------
app/test/test_power_cpufreq.c | 52 -------------------
app/test/test_power_kvm_vm.c | 36 -------------
3 files changed, 183 deletions(-)
Comments
On 20/07/2024 17:50, Sivaprasad Tummala wrote:
> After refactoring the power library, power management operations are now
> consistently supported regardless of the operating environment, making
> function pointer checks unnecessary and thus removed from applications.
>
> Signed-off-by: Sivaprasad Tummala<sivaprasad.tummala@amd.com>
> ---
> app/test/test_power.c | 95 -----------------------------------
> app/test/test_power_cpufreq.c | 52 -------------------
> app/test/test_power_kvm_vm.c | 36 -------------
> 3 files changed, 183 deletions(-)
>
Hi Sivaprasad,
Nice work on the patch-set.
There's just four function pointer checks remaining that my compiler is
complaining about. They are in examples/l3fwd-power/main.c (lines 443,
452, 1350, 1353). It would be nice to have these removed as well, seeing
as the functions are now inlines and don't need these checks.
I'm running the patch set through some tests here, will keep you posted
on progress.
Rgds,
Dave.
---snip---
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Dave,
Inline..
From: Hunt, David <david.hunt@intel.com>
Sent: Monday, July 22, 2024 4:20 PM
To: Tummala, Sivaprasad <Sivaprasad.Tummala@amd.com>; anatoly.burakov@intel.com; jerinj@marvell.com; lihuisong@huawei.com; david.marchand@redhat.com; Yigit, Ferruh <Ferruh.Yigit@amd.com>; konstantin.ananyev@huawei.com
Cc: dev@dpdk.org
Subject: Re: [PATCH v1 3/4] test/power: removed function pointer validations
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On 20/07/2024 17:50, Sivaprasad Tummala wrote:
After refactoring the power library, power management operations are now
consistently supported regardless of the operating environment, making
function pointer checks unnecessary and thus removed from applications.
Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com><mailto:sivaprasad.tummala@amd.com>
---
app/test/test_power.c | 95 -----------------------------------
app/test/test_power_cpufreq.c | 52 -------------------
app/test/test_power_kvm_vm.c | 36 -------------
3 files changed, 183 deletions(-)
Hi Sivaprasad,
Nice work on the patch-set.
There's just four function pointer checks remaining that my compiler is complaining about. They are in examples/l3fwd-power/main.c (lines 443, 452, 1350, 1353). It would be nice to have these removed as well, seeing as the functions are now inlines and don't need these checks.
[Siva] ACK. Will fix this in next version.
I'm running the patch set through some tests here, will keep you posted on progress.
Rgds,
Dave.
---snip---
@@ -24,86 +24,6 @@ test_power(void)
#include <rte_power.h>
-static int
-check_function_ptrs(void)
-{
- enum power_management_env env = rte_power_get_env();
-
- const bool not_null_expected = !(env == PM_ENV_NOT_SET);
-
- const char *inject_not_string1 = not_null_expected ? " not" : "";
- const char *inject_not_string2 = not_null_expected ? "" : " not";
-
- if ((rte_power_freqs == NULL) == not_null_expected) {
- printf("rte_power_freqs should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_get_freq == NULL) == not_null_expected) {
- printf("rte_power_get_freq should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_set_freq == NULL) == not_null_expected) {
- printf("rte_power_set_freq should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_freq_up == NULL) == not_null_expected) {
- printf("rte_power_freq_up should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_freq_down == NULL) == not_null_expected) {
- printf("rte_power_freq_down should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_freq_max == NULL) == not_null_expected) {
- printf("rte_power_freq_max should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_freq_min == NULL) == not_null_expected) {
- printf("rte_power_freq_min should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_turbo_status == NULL) == not_null_expected) {
- printf("rte_power_turbo_status should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_freq_enable_turbo == NULL) == not_null_expected) {
- printf("rte_power_freq_enable_turbo should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_freq_disable_turbo == NULL) == not_null_expected) {
- printf("rte_power_freq_disable_turbo should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
- if ((rte_power_get_capabilities == NULL) == not_null_expected) {
- printf("rte_power_get_capabilities should%s be NULL, environment has%s been "
- "initialised\n", inject_not_string1,
- inject_not_string2);
- return -1;
- }
-
- return 0;
-}
-
static int
test_power(void)
{
@@ -124,10 +44,6 @@ test_power(void)
return -1;
}
- /* Verify that function pointers are NULL */
- if (check_function_ptrs() < 0)
- goto fail_all;
-
rte_power_unset_env();
/* Perform tests for valid environments.*/
@@ -154,22 +70,11 @@ test_power(void)
return -1;
}
- /* Verify that function pointers are NOT NULL */
- if (check_function_ptrs() < 0)
- goto fail_all;
-
rte_power_unset_env();
- /* Verify that function pointers are NULL */
- if (check_function_ptrs() < 0)
- goto fail_all;
-
}
return 0;
-fail_all:
- rte_power_unset_env();
- return -1;
}
#endif
@@ -519,58 +519,6 @@ test_power_cpufreq(void)
goto fail_all;
}
- /* verify that function pointers are not NULL */
- if (rte_power_freqs == NULL) {
- printf("rte_power_freqs should not be NULL, environment has not been "
- "initialised\n");
- goto fail_all;
- }
- if (rte_power_get_freq == NULL) {
- printf("rte_power_get_freq should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
- if (rte_power_set_freq == NULL) {
- printf("rte_power_set_freq should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
- if (rte_power_freq_up == NULL) {
- printf("rte_power_freq_up should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
- if (rte_power_freq_down == NULL) {
- printf("rte_power_freq_down should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
- if (rte_power_freq_max == NULL) {
- printf("rte_power_freq_max should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
- if (rte_power_freq_min == NULL) {
- printf("rte_power_freq_min should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
- if (rte_power_turbo_status == NULL) {
- printf("rte_power_turbo_status should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
- if (rte_power_freq_enable_turbo == NULL) {
- printf("rte_power_freq_enable_turbo should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
- if (rte_power_freq_disable_turbo == NULL) {
- printf("rte_power_freq_disable_turbo should not be NULL, environment has not "
- "been initialised\n");
- goto fail_all;
- }
-
ret = rte_power_exit(TEST_POWER_LCORE_ID);
if (ret < 0) {
printf("Cannot exit power management for lcore %u\n",
@@ -47,42 +47,6 @@ test_power_kvm_vm(void)
return -1;
}
- /* verify that function pointers are not NULL */
- if (rte_power_freqs == NULL) {
- printf("rte_power_freqs should not be NULL, environment has not been "
- "initialised\n");
- return -1;
- }
- if (rte_power_get_freq == NULL) {
- printf("rte_power_get_freq should not be NULL, environment has not "
- "been initialised\n");
- return -1;
- }
- if (rte_power_set_freq == NULL) {
- printf("rte_power_set_freq should not be NULL, environment has not "
- "been initialised\n");
- return -1;
- }
- if (rte_power_freq_up == NULL) {
- printf("rte_power_freq_up should not be NULL, environment has not "
- "been initialised\n");
- return -1;
- }
- if (rte_power_freq_down == NULL) {
- printf("rte_power_freq_down should not be NULL, environment has not "
- "been initialised\n");
- return -1;
- }
- if (rte_power_freq_max == NULL) {
- printf("rte_power_freq_max should not be NULL, environment has not "
- "been initialised\n");
- return -1;
- }
- if (rte_power_freq_min == NULL) {
- printf("rte_power_freq_min should not be NULL, environment has not "
- "been initialised\n");
- return -1;
- }
/* Test initialisation of an out of bounds lcore */
ret = rte_power_init(TEST_POWER_VM_LCORE_OUT_OF_BOUNDS);
if (ret != -1) {