From patchwork Wed Mar 13 19:48:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcin Hajkowski X-Patchwork-Id: 51194 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B01CF4F91; Thu, 14 Mar 2019 15:56:54 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 892C337B0; Wed, 13 Mar 2019 20:50:17 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Mar 2019 12:50:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,475,1544515200"; d="scan'208";a="213891631" Received: from mhajkowx-mobl.ger.corp.intel.com ([10.104.116.179]) by orsmga001.jf.intel.com with ESMTP; 13 Mar 2019 12:50:03 -0700 From: Hajkowski To: david.hunt@intel.com Cc: dev@dpdk.org, Marcin Hajkowski , stable@dpdk.org Date: Wed, 13 Mar 2019 20:48:41 +0100 Message-Id: <20190313194844.11820-2-marcinx.hajkowski@intel.com> X-Mailer: git-send-email 2.20.1.windows.1 In-Reply-To: <20190313194844.11820-1-marcinx.hajkowski@intel.com> References: <20190313194844.11820-1-marcinx.hajkowski@intel.com> MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 14 Mar 2019 15:56:49 +0100 Subject: [dpdk-dev] [PATCH 1/4] power: fix non thread-safe power env modification X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Marcin Hajkowski Due to lack of thread safety in exisiting solution use spinlock mechanism for atomic modification of power environment related data. Fixes: 445c6528b5 ("power: common interface for guest and host") Cc: stable@dpdk.org Signed-off-by: Marcin Hajkowski --- doc/guides/rel_notes/release_19_05.rst | 2 ++ lib/librte_power/rte_power.c | 30 ++++++++++++++++++-------- lib/librte_power/rte_power.h | 2 +- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst index 61a2c7383..65371e4c8 100644 --- a/doc/guides/rel_notes/release_19_05.rst +++ b/doc/guides/rel_notes/release_19_05.rst @@ -120,6 +120,8 @@ API Changes Also, make sure to start the actual text at the margin. ========================================================= + * power: ``rte_power_set_env`` and ``rte_power_unset_env`` functions + have been modified to be thread safe. ABI Changes ----------- diff --git a/lib/librte_power/rte_power.c b/lib/librte_power/rte_power.c index a05fbef94..540d69be7 100644 --- a/lib/librte_power/rte_power.c +++ b/lib/librte_power/rte_power.c @@ -2,7 +2,7 @@ * Copyright(c) 2010-2014 Intel Corporation */ -#include +#include #include "rte_power.h" #include "power_acpi_cpufreq.h" @@ -12,7 +12,7 @@ enum power_management_env global_default_env = PM_ENV_NOT_SET; -volatile uint32_t global_env_cfg_status = 0; +static rte_spinlock_t global_env_cfg_lock = RTE_SPINLOCK_INITIALIZER; /* function pointers */ rte_power_freqs_t rte_power_freqs = NULL; @@ -30,9 +30,15 @@ rte_power_get_capabilities_t rte_power_get_capabilities; int rte_power_set_env(enum power_management_env env) { - if (rte_atomic32_cmpset(&global_env_cfg_status, 0, 1) == 0) { + rte_spinlock_lock(&global_env_cfg_lock); + + if (global_default_env != PM_ENV_NOT_SET) { + rte_spinlock_unlock(&global_env_cfg_lock); return 0; } + + int ret = 0; + if (env == PM_ENV_ACPI_CPUFREQ) { rte_power_freqs = power_acpi_cpufreq_freqs; rte_power_get_freq = power_acpi_cpufreq_get_freq; @@ -73,18 +79,24 @@ rte_power_set_env(enum power_management_env env) } else { RTE_LOG(ERR, POWER, "Invalid Power Management Environment(%d) set\n", env); - rte_power_unset_env(); - return -1; + ret = -1; } - global_default_env = env; - return 0; + + if (ret == 0) + global_default_env = env; + else + global_default_env = PM_ENV_NOT_SET; + + rte_spinlock_unlock(&global_env_cfg_lock); + return ret; } void rte_power_unset_env(void) { - if (rte_atomic32_cmpset(&global_env_cfg_status, 1, 0) != 0) - global_default_env = PM_ENV_NOT_SET; + rte_spinlock_lock(&global_env_cfg_lock); + global_default_env = PM_ENV_NOT_SET; + rte_spinlock_unlock(&global_env_cfg_lock); } enum power_management_env diff --git a/lib/librte_power/rte_power.h b/lib/librte_power/rte_power.h index c5e8f6b5b..54b76b4ee 100644 --- a/lib/librte_power/rte_power.h +++ b/lib/librte_power/rte_power.h @@ -26,7 +26,7 @@ enum power_management_env {PM_ENV_NOT_SET, PM_ENV_ACPI_CPUFREQ, PM_ENV_KVM_VM, /** * Set the default power management implementation. If this is not called prior * to rte_power_init(), then auto-detect of the environment will take place. - * It is not thread safe. + * It is thread safe. * * @param env * env. The environment in which to initialise Power Management for. From patchwork Wed Mar 13 19:48:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcin Hajkowski X-Patchwork-Id: 51195 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 736984F9B; Thu, 14 Mar 2019 15:56:56 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id D7AA737B0 for ; Wed, 13 Mar 2019 20:50:28 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Mar 2019 12:50:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,475,1544515200"; d="scan'208";a="213891703" Received: from mhajkowx-mobl.ger.corp.intel.com ([10.104.116.179]) by orsmga001.jf.intel.com with ESMTP; 13 Mar 2019 12:50:16 -0700 From: Hajkowski To: david.hunt@intel.com Cc: dev@dpdk.org, Marcin Hajkowski Date: Wed, 13 Mar 2019 20:48:42 +0100 Message-Id: <20190313194844.11820-3-marcinx.hajkowski@intel.com> X-Mailer: git-send-email 2.20.1.windows.1 In-Reply-To: <20190313194844.11820-1-marcinx.hajkowski@intel.com> References: <20190313194844.11820-1-marcinx.hajkowski@intel.com> MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 14 Mar 2019 15:56:49 +0100 Subject: [dpdk-dev] [PATCH 2/4] power: return error in set env when power env already set X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Marcin Hajkowski On attempt to set_env in already initialized state notify user by returning error that operation cannot be performed. Signed-off-by: Marcin Hajkowski --- doc/guides/rel_notes/release_19_05.rst | 4 ++++ lib/librte_power/rte_power.c | 3 ++- lib/librte_power/rte_power.h | 3 ++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst index 65371e4c8..0da26b937 100644 --- a/doc/guides/rel_notes/release_19_05.rst +++ b/doc/guides/rel_notes/release_19_05.rst @@ -120,9 +120,13 @@ API Changes Also, make sure to start the actual text at the margin. ========================================================= + * power: ``rte_power_set_env`` and ``rte_power_unset_env`` functions have been modified to be thread safe. + * power: Function ``rte_power_set_env`` modified to return -1 if environment + has been already set. + ABI Changes ----------- diff --git a/lib/librte_power/rte_power.c b/lib/librte_power/rte_power.c index 540d69be7..ed701087d 100644 --- a/lib/librte_power/rte_power.c +++ b/lib/librte_power/rte_power.c @@ -33,8 +33,9 @@ rte_power_set_env(enum power_management_env env) rte_spinlock_lock(&global_env_cfg_lock); if (global_default_env != PM_ENV_NOT_SET) { + RTE_LOG(ERR, POWER, "Power Management Environment already set.\n"); rte_spinlock_unlock(&global_env_cfg_lock); - return 0; + return -1; } int ret = 0; diff --git a/lib/librte_power/rte_power.h b/lib/librte_power/rte_power.h index 54b76b4ee..a3130e709 100644 --- a/lib/librte_power/rte_power.h +++ b/lib/librte_power/rte_power.h @@ -26,7 +26,8 @@ enum power_management_env {PM_ENV_NOT_SET, PM_ENV_ACPI_CPUFREQ, PM_ENV_KVM_VM, /** * Set the default power management implementation. If this is not called prior * to rte_power_init(), then auto-detect of the environment will take place. - * It is thread safe. + * It is thread safe. New env can be set only in unitialized state + * (thus rte_power_unset_env must be called if different env was already set). * * @param env * env. The environment in which to initialise Power Management for. From patchwork Wed Mar 13 19:48:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcin Hajkowski X-Patchwork-Id: 51196 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BD7F15587; Thu, 14 Mar 2019 15:56:57 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id BF8493772 for ; Wed, 13 Mar 2019 20:50:40 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Mar 2019 12:50:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,475,1544515200"; d="scan'208";a="213891756" Received: from mhajkowx-mobl.ger.corp.intel.com ([10.104.116.179]) by orsmga001.jf.intel.com with ESMTP; 13 Mar 2019 12:50:27 -0700 From: Hajkowski To: david.hunt@intel.com Cc: dev@dpdk.org, Marcin Hajkowski Date: Wed, 13 Mar 2019 20:48:43 +0100 Message-Id: <20190313194844.11820-4-marcinx.hajkowski@intel.com> X-Mailer: git-send-email 2.20.1.windows.1 In-Reply-To: <20190313194844.11820-1-marcinx.hajkowski@intel.com> References: <20190313194844.11820-1-marcinx.hajkowski@intel.com> MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 14 Mar 2019 15:56:49 +0100 Subject: [dpdk-dev] [PATCH 3/4] power: reset function pointers on unset env X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Marcin Hajkowski Set all power environment related function pointers to NULL when unset is being made. Signed-off-by: Marcin Hajkowski --- lib/librte_power/rte_power.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/librte_power/rte_power.c b/lib/librte_power/rte_power.c index ed701087d..6b7722727 100644 --- a/lib/librte_power/rte_power.c +++ b/lib/librte_power/rte_power.c @@ -27,6 +27,22 @@ rte_power_freq_change_t rte_power_freq_enable_turbo; rte_power_freq_change_t rte_power_freq_disable_turbo; rte_power_get_capabilities_t rte_power_get_capabilities; +static void +reset_power_function_ptrs(void) +{ + rte_power_freqs = NULL; + rte_power_get_freq = NULL; + rte_power_set_freq = NULL; + rte_power_freq_up = NULL; + rte_power_freq_down = NULL; + rte_power_freq_max = NULL; + rte_power_freq_min = NULL; + rte_power_turbo_status = NULL; + rte_power_freq_enable_turbo = NULL; + rte_power_freq_disable_turbo = NULL; + rte_power_get_capabilities = NULL; +} + int rte_power_set_env(enum power_management_env env) { @@ -85,8 +101,10 @@ rte_power_set_env(enum power_management_env env) if (ret == 0) global_default_env = env; - else + else { global_default_env = PM_ENV_NOT_SET; + reset_power_function_ptrs(); + } rte_spinlock_unlock(&global_env_cfg_lock); return ret; @@ -97,6 +115,7 @@ rte_power_unset_env(void) { rte_spinlock_lock(&global_env_cfg_lock); global_default_env = PM_ENV_NOT_SET; + reset_power_function_ptrs(); rte_spinlock_unlock(&global_env_cfg_lock); } From patchwork Wed Mar 13 19:48:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcin Hajkowski X-Patchwork-Id: 51197 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DD6D45689; Thu, 14 Mar 2019 15:56:58 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 95F313772 for ; Wed, 13 Mar 2019 20:50:53 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Mar 2019 12:50:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,475,1544515200"; d="scan'208";a="213891819" Received: from mhajkowx-mobl.ger.corp.intel.com ([10.104.116.179]) by orsmga001.jf.intel.com with ESMTP; 13 Mar 2019 12:50:39 -0700 From: Hajkowski To: david.hunt@intel.com Cc: dev@dpdk.org, Marcin Hajkowski Date: Wed, 13 Mar 2019 20:48:44 +0100 Message-Id: <20190313194844.11820-5-marcinx.hajkowski@intel.com> X-Mailer: git-send-email 2.20.1.windows.1 In-Reply-To: <20190313194844.11820-1-marcinx.hajkowski@intel.com> References: <20190313194844.11820-1-marcinx.hajkowski@intel.com> MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 14 Mar 2019 15:56:49 +0100 Subject: [dpdk-dev] [PATCH 4/4] power: add UTs for all power env types X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Marcin Hajkowski Test all existing power environment configuration to verify if related data is properly initialized and clean in set/unset scenarios. Signed-off-by: Marcin Hajkowski --- app/test/test_power.c | 155 ++++++++++++++++++++++++++++++++---------- 1 file changed, 120 insertions(+), 35 deletions(-) diff --git a/app/test/test_power.c b/app/test/test_power.c index a0ee21983..8c574e531 100644 --- a/app/test/test_power.c +++ b/app/test/test_power.c @@ -7,6 +7,7 @@ #include #include #include +#include #include "test.h" @@ -23,6 +24,86 @@ test_power(void) #include +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) { @@ -43,43 +124,47 @@ test_power(void) return -1; } - /* verify that function pointers are NULL */ - if (rte_power_freqs != NULL) { - printf("rte_power_freqs should be NULL, environment has not been " - "initialised\n"); - goto fail_all; - } - if (rte_power_get_freq != NULL) { - printf("rte_power_get_freq should be NULL, environment has not been " - "initialised\n"); - goto fail_all; - } - if (rte_power_set_freq != NULL) { - printf("rte_power_set_freq should be NULL, environment has not been " - "initialised\n"); - goto fail_all; - } - if (rte_power_freq_up != NULL) { - printf("rte_power_freq_up should be NULL, environment has not been " - "initialised\n"); - goto fail_all; - } - if (rte_power_freq_down != NULL) { - printf("rte_power_freq_down should be NULL, environment has not been " - "initialised\n"); - goto fail_all; - } - if (rte_power_freq_max != NULL) { - printf("rte_power_freq_max should be NULL, environment has not been " - "initialised\n"); - goto fail_all; - } - if (rte_power_freq_min != NULL) { - printf("rte_power_freq_min should be NULL, environment has not been " - "initialised\n"); + /* Verify that function pointers are NULL */ + if (check_function_ptrs() < 0) goto fail_all; - } + rte_power_unset_env(); + + /* Perform tests for valid environments.*/ + const enum power_management_env envs[] = {PM_ENV_ACPI_CPUFREQ, + PM_ENV_KVM_VM, + PM_ENV_PSTATE_CPUFREQ}; + + const int envs_size = sizeof(envs)/sizeof(enum power_management_env); + + for (int i = 0; i < envs_size; ++i) { + + /* Test setting a valid environment */ + ret = rte_power_set_env(envs[i]); + if (ret != 0) { + printf("Unexpectedly unsucceeded on setting a valid environment\n"); + return -1; + } + + /* Test that the environment has been set */ + env = rte_power_get_env(); + if (env != envs[i]) { + printf("Not expected environment configuration\n"); + 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();