From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 05FB4A0A0F; Wed, 30 Jun 2021 11:52:28 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1E8BC40141; Wed, 30 Jun 2021 11:52:28 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id A4C9A40040 for ; Wed, 30 Jun 2021 11:52:26 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10030"; a="206499125" X-IronPort-AV: E=Sophos;i="5.83,311,1616482800"; d="scan'208";a="206499125" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Jun 2021 02:52:24 -0700 X-IronPort-AV: E=Sophos;i="5.83,311,1616482800"; d="scan'208";a="457153053" Received: from dhunt5-mobl5.ger.corp.intel.com (HELO [10.252.20.191]) ([10.252.20.191]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Jun 2021 02:52:23 -0700 To: Anatoly Burakov , dev@dpdk.org Cc: konstantin.ananyev@intel.com, ciara.loftus@intel.com References: <8f5d030a77aa2f0e95e9680cb911b4e8db30c879.1624981670.git.anatoly.burakov@intel.com> From: David Hunt Message-ID: <04557f14-b238-3daa-2753-b3437e59e640@intel.com> Date: Wed, 30 Jun 2021 10:52:21 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <8f5d030a77aa2f0e95e9680cb911b4e8db30c879.1624981670.git.anatoly.burakov@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB Subject: Re: [dpdk-dev] [PATCH v5 5/7] power: support callbacks for multiple Rx queues X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" Hi Anatoly, On 29/6/2021 4:48 PM, Anatoly Burakov wrote: > Currently, there is a hard limitation on the PMD power management > support that only allows it to support a single queue per lcore. This is > not ideal as most DPDK use cases will poll multiple queues per core. > > The PMD power management mechanism relies on ethdev Rx callbacks, so it > is very difficult to implement such support because callbacks are > effectively stateless and have no visibility into what the other ethdev > devices are doing. This places limitations on what we can do within the > framework of Rx callbacks, but the basics of this implementation are as > follows: > > - Replace per-queue structures with per-lcore ones, so that any device > polled from the same lcore can share data > - Any queue that is going to be polled from a specific lcore has to be > added to the list of queues to poll, so that the callback is aware of > other queues being polled by the same lcore > - Both the empty poll counter and the actual power saving mechanism is > shared between all queues polled on a particular lcore, and is only > activated when all queues in the list were polled and were determined > to have no traffic. > - The limitation on UMWAIT-based polling is not removed because UMWAIT > is incapable of monitoring more than one address. > > Also, while we're at it, update and improve the docs. > > Signed-off-by: Anatoly Burakov > --- > > Notes: > v5: > - Remove the "power save queue" API and replace it with mechanism suggested by > Konstantin > > v3: > - Move the list of supported NICs to NIC feature table > > v2: > - Use a TAILQ for queues instead of a static array > - Address feedback from Konstantin > - Add additional checks for stopped queues > > doc/guides/nics/features.rst | 10 + > doc/guides/prog_guide/power_man.rst | 65 ++-- > doc/guides/rel_notes/release_21_08.rst | 3 + > lib/power/rte_power_pmd_mgmt.c | 431 ++++++++++++++++++------- > 4 files changed, 373 insertions(+), 136 deletions(-) > --snip-- > int > rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id, > uint16_t queue_id, enum rte_power_pmd_mgmt_type mode) > { > - struct pmd_queue_cfg *queue_cfg; > + const union queue qdata = {.portid = port_id, .qid = queue_id}; > + struct pmd_core_cfg *lcore_cfg; > + struct queue_list_entry *queue_cfg; > struct rte_eth_dev_info info; > rte_rx_callback_fn clb; > int ret; > @@ -202,9 +401,19 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id, > goto end; > } > > - queue_cfg = &port_cfg[port_id][queue_id]; > + lcore_cfg = &lcore_cfgs[lcore_id]; > > - if (queue_cfg->pwr_mgmt_state != PMD_MGMT_DISABLED) { > + /* check if other queues are stopped as well */ > + ret = cfg_queues_stopped(lcore_cfg); > + if (ret != 1) { > + /* error means invalid queue, 0 means queue wasn't stopped */ > + ret = ret < 0 ? -EINVAL : -EBUSY; > + goto end; > + } > + > + /* if callback was already enabled, check current callback type */ > + if (lcore_cfg->pwr_mgmt_state != PMD_MGMT_DISABLED && > + lcore_cfg->cb_mode != mode) { > ret = -EINVAL; > goto end; > } > @@ -214,53 +423,20 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id, > > switch (mode) { > case RTE_POWER_MGMT_TYPE_MONITOR: > - { > - struct rte_power_monitor_cond dummy; > - > - /* check if rte_power_monitor is supported */ > - if (!global_data.intrinsics_support.power_monitor) { > - RTE_LOG(DEBUG, POWER, "Monitoring intrinsics are not supported\n"); > - ret = -ENOTSUP; > + /* check if we can add a new queue */ > + ret = check_monitor(lcore_cfg, &qdata); > + if (ret < 0) > goto end; > - } > > - /* check if the device supports the necessary PMD API */ > - if (rte_eth_get_monitor_addr(port_id, queue_id, > - &dummy) == -ENOTSUP) { > - RTE_LOG(DEBUG, POWER, "The device does not support rte_eth_get_monitor_addr\n"); > - ret = -ENOTSUP; > - goto end; > - } > clb = clb_umwait; > break; > - } > case RTE_POWER_MGMT_TYPE_SCALE: > - { > - enum power_management_env env; > - /* only PSTATE and ACPI modes are supported */ > - if (!rte_power_check_env_supported(PM_ENV_ACPI_CPUFREQ) && > - !rte_power_check_env_supported( > - PM_ENV_PSTATE_CPUFREQ)) { > - RTE_LOG(DEBUG, POWER, "Neither ACPI nor PSTATE modes are supported\n"); > - ret = -ENOTSUP; > + /* check if we can add a new queue */ > + ret = check_scale(lcore_id); > + if (ret < 0) > goto end; > - } > - /* ensure we could initialize the power library */ > - if (rte_power_init(lcore_id)) { > - ret = -EINVAL; > - goto end; > - } > - /* ensure we initialized the correct env */ > - env = rte_power_get_env(); > - if (env != PM_ENV_ACPI_CPUFREQ && > - env != PM_ENV_PSTATE_CPUFREQ) { > - RTE_LOG(DEBUG, POWER, "Neither ACPI nor PSTATE modes were initialized\n"); > - ret = -ENOTSUP; > - goto end; > - } > clb = clb_scale_freq; > break; > - } > case RTE_POWER_MGMT_TYPE_PAUSE: > /* figure out various time-to-tsc conversions */ > if (global_data.tsc_per_us == 0) > @@ -273,13 +449,23 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id, > ret = -EINVAL; > goto end; > } > + /* add this queue to the list */ > + ret = queue_list_add(lcore_cfg, &qdata); > + if (ret < 0) { > + RTE_LOG(DEBUG, POWER, "Failed to add queue to list: %s\n", > + strerror(-ret)); > + goto end; > + } > + /* new queue is always added last */ > + queue_cfg = TAILQ_LAST(&lcore_cfgs->head, queue_list_head); Need to ensure that queue_cfg gets set here, otherwise we'll get a segfault below. > > /* initialize data before enabling the callback */ > - queue_cfg->empty_poll_stats = 0; > - queue_cfg->cb_mode = mode; > - queue_cfg->pwr_mgmt_state = PMD_MGMT_ENABLED; > - queue_cfg->cur_cb = rte_eth_add_rx_callback(port_id, queue_id, > - clb, NULL); > + if (lcore_cfg->n_queues == 1) { > + lcore_cfg->cb_mode = mode; > + lcore_cfg->pwr_mgmt_state = PMD_MGMT_ENABLED; > + } > + queue_cfg->cb = rte_eth_add_rx_callback(port_id, queue_id, > + clb, queue_cfg); --snip--