From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id CD11E7E5D for ; Thu, 25 Sep 2014 19:43:35 +0200 (CEST) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1XXDAn-0007LL-0W; Thu, 25 Sep 2014 13:49:52 -0400 Date: Thu, 25 Sep 2014 13:49:43 -0400 From: Neil Horman To: "Carew, Alan" Message-ID: <20140925174943.GI32725@hmsreliant.think-freely.org> References: <1411410879-28872-1-git-send-email-alan.carew@intel.com> <1411579576-21786-1-git-send-email-alan.carew@intel.com> <1411579576-21786-8-git-send-email-alan.carew@intel.com> <20140925101003.GA32725@hmsreliant.think-freely.org> <0E29434AEE0C3A4180987AB476A6F6306D278070@IRSMSX109.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0E29434AEE0C3A4180987AB476A6F6306D278070@IRSMSX109.ger.corp.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 07/10] librte_power common interface for Guest and Host X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Sep 2014 17:43:36 -0000 On Thu, Sep 25, 2014 at 05:06:11PM +0000, Carew, Alan wrote: > > -----Original Message----- > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > Sent: Thursday, September 25, 2014 11:10 AM > > To: Carew, Alan > > Cc: dev@dpdk.org > > Subject: Re: [PATCH v2 07/10] librte_power common interface for Guest and > > Host > > > > On Wed, Sep 24, 2014 at 06:26:13PM +0100, Alan Carew wrote: > > > Moved the current librte_power implementation to rte_power_acpi_cpufreq, > > with > > > renaming of functions only. > > > Added rte_power_kvm_vm implmentation to support Power Management > > from a VM. > > > > > > librte_power now hides the implementation based on the environment used. > > > A new call rte_power_set_env() can explicidly set the environment, if not > > > called then auto-detection takes place. > > > > > > rte_power_kvm_vm is subset of the librte_power APIs, the following is > > supported: > > > rte_power_init(unsigned lcore_id) > > > rte_power_exit(unsigned lcore_id) > > > rte_power_freq_up(unsigned lcore_id) > > > rte_power_freq_down(unsigned lcore_id) > > > rte_power_freq_min(unsigned lcore_id) > > > rte_power_freq_max(unsigned lcore_id) > > > > > > The other unsupported APIs return -ENOTSUP > > > > > > Signed-off-by: Alan Carew > > > --- > > > lib/librte_power/rte_power.c | 540 ++++------------------------- > > > lib/librte_power/rte_power.h | 120 +++++-- > > > lib/librte_power/rte_power_acpi_cpufreq.c | 545 > > ++++++++++++++++++++++++++++++ > > > lib/librte_power/rte_power_acpi_cpufreq.h | 192 +++++++++++ > > > lib/librte_power/rte_power_common.h | 39 +++ > > > lib/librte_power/rte_power_kvm_vm.c | 160 +++++++++ > > > lib/librte_power/rte_power_kvm_vm.h | 179 ++++++++++ > > > 7 files changed, 1273 insertions(+), 502 deletions(-) > > > create mode 100644 lib/librte_power/rte_power_acpi_cpufreq.c > > > create mode 100644 lib/librte_power/rte_power_acpi_cpufreq.h > > > create mode 100644 lib/librte_power/rte_power_common.h > > > create mode 100644 lib/librte_power/rte_power_kvm_vm.c > > > create mode 100644 lib/librte_power/rte_power_kvm_vm.h > > > > > > diff --git a/lib/librte_power/rte_power.c b/lib/librte_power/rte_power.c > > > index 856da9a..998ed1c 100644 > > > --- a/lib/librte_power/rte_power.c > > > +++ b/lib/librte_power/rte_power.c > > > @@ -31,515 +31,113 @@ > > > * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > DAMAGE. > > > */ > > > > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > - > > > -#include > > > #include > > > > > > #include "rte_power.h" > > > +#include "rte_power_acpi_cpufreq.h" > > > +#include "rte_power_kvm_vm.h" > > > +#include "rte_power_common.h" > > > > > > -#ifdef RTE_LIBRTE_POWER_DEBUG > > > -#define POWER_DEBUG_TRACE(fmt, args...) do { \ > > > - RTE_LOG(ERR, POWER, "%s: " fmt, __func__, ## args); \ > > > - } while (0) > > > -#else > > > -#define POWER_DEBUG_TRACE(fmt, args...) > > > -#endif > > > - > > > -#define FOPEN_OR_ERR_RET(f, retval) do { \ > > > - if ((f) == NULL) { \ > > > - RTE_LOG(ERR, POWER, "File not openned\n"); \ > > > - return (retval); \ > > > - } \ > > > -} while(0) > > > - > > > -#define FOPS_OR_NULL_GOTO(ret, label) do { \ > > > - if ((ret) == NULL) { \ > > > - RTE_LOG(ERR, POWER, "fgets returns nothing\n"); \ > > > - goto label; \ > > > - } \ > > > -} while(0) > > > - > > > -#define FOPS_OR_ERR_GOTO(ret, label) do { \ > > > - if ((ret) < 0) { \ > > > - RTE_LOG(ERR, POWER, "File operations failed\n"); \ > > > - goto label; \ > > > - } \ > > > -} while(0) > > > - > > > -#define STR_SIZE 1024 > > > -#define POWER_CONVERT_TO_DECIMAL 10 > > > +enum power_management_env global_default_env = PM_ENV_NOT_SET; > > > > > > -#define POWER_GOVERNOR_USERSPACE "userspace" > > > -#define POWER_SYSFILE_GOVERNOR \ > > > - "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_governor" > > > -#define POWER_SYSFILE_AVAIL_FREQ \ > > > - > > "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_available_frequencie > > s" > > > -#define POWER_SYSFILE_SETSPEED \ > > > - "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_setspeed" > > > +volatile uint32_t global_env_cfg_status = 0; > > > > > > -enum power_state { > > > - POWER_IDLE = 0, > > > - POWER_ONGOING, > > > - POWER_USED, > > > - POWER_UNKNOWN > > > -}; > > > +/* function pointers */ > > > +rte_power_freqs_t rte_power_freqs = NULL; > > > +rte_power_get_freq_t rte_power_get_freq = NULL; > > > +rte_power_set_freq_t rte_power_set_freq = NULL; > > > +rte_power_freq_change_t rte_power_freq_up = NULL; > > > +rte_power_freq_change_t rte_power_freq_down = NULL; > > > +rte_power_freq_change_t rte_power_freq_max = NULL; > > > +rte_power_freq_change_t rte_power_freq_min = NULL; > > > > > > -/** > > > - * Power info per lcore. > > > - */ > > > -struct rte_power_info { > > > - unsigned lcore_id; /**< Logical core id */ > > > - uint32_t freqs[RTE_MAX_LCORE_FREQS]; /**< Frequency array */ > > > - uint32_t nb_freqs; /**< number of available freqs */ > > > - FILE *f; /**< FD of scaling_setspeed */ > > > - char governor_ori[32]; /**< Original governor name */ > > > - uint32_t curr_idx; /**< Freq index in freqs array */ > > > - volatile uint32_t state; /**< Power in use state */ > > > -} __rte_cache_aligned; > > > - > > > -static struct rte_power_info lcore_power_info[RTE_MAX_LCORE]; > > > - > > > -/** > > > - * It is to set specific freq for specific logical core, according to the index > > > - * of supported frequencies. > > > - */ > > > -static int > > > -set_freq_internal(struct rte_power_info *pi, uint32_t idx) > > > +int > > > +rte_power_set_env(enum power_management_env env) > > > { > > > - if (idx >= RTE_MAX_LCORE_FREQS || idx >= pi->nb_freqs) { > > > - RTE_LOG(ERR, POWER, "Invalid frequency index %u, which " > > > - "should be less than %u\n", idx, pi->nb_freqs); > > > - return -1; > > > - } > > > - > > > - /* Check if it is the same as current */ > > > - if (idx == pi->curr_idx) > > > + if (rte_atomic32_cmpset(&global_env_cfg_status, 0, 1) == 0) { > > > return 0; > > > - > > 1 Nit here. If an invalid environment value is passed in on the first config > > attempt here, you won't ever be able to set it. Maybe add some logic to return > > us to an initial state if a value env isn't selected? > > > > Neil > > Hi Neil, > > I should have called it out in the commit, but there's also a rte_power_unset_env() > function that resets the environment that allows for retrying a different environment. > rte_power_unset_env() is also called when an invalid configuration is set. > > Thanks, > Alan. > Ok, that seems like an odd interface too me, but it works as well as anything else. Thanks! Neil >