d/numam-dpdk
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

power: make ethdev power management thread unsafe

Browse Source 
Currently, we expect that only one callback can be active at any given
moment, for a particular queue configuration, which is relatively easy
to implement in a thread-safe way. However, we're about to add support
for multiple queues per lcore, which will greatly increase the
possibility of various race conditions.

We could have used something like an RCU for this use case, but absent
of a pressing need for thread safety we'll go the easy way and just
mandate that the API's are to be called when all affected ports are
stopped, and document this limitation. This greatly simplifies the
`rte_power_monitor`-related code.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Tested-by: David Hunt <david.hunt@intel.com>
This commit is contained in:
Anatoly Burakov 2021-07-09 16:08:14 +00:00 committed by David Marchand
parent 66834f2974
commit 209fd58545
4 changed files with 66 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/guides/rel_notes/release_21_08.rst
View File

@ -147,6 +147,10 @@ API Changes
* eal: ``rte_power_monitor`` and the ``rte_power_monitor_cond`` struct changed
to use a callback mechanism.
* rte_power: The experimental PMD power management API is no longer considered
to be thread safe; all Rx queues affected by the API will now need to be
stopped before making any changes to the power management scheme.
ABI Changes
-----------

3
lib/power/meson.build
View File

@ -22,4 +22,7 @@ headers = files(
'rte_power_pmd_mgmt.h',
'rte_power_guest_channel.h',
)
if cc.has_argument('-Wno-cast-qual')
cflags += '-Wno-cast-qual'
endif
deps += ['timer', 'ethdev']

133
lib/power/rte_power_pmd_mgmt.c
View File

@ -40,8 +40,6 @@ struct pmd_queue_cfg {
/**< Callback mode for this queue */
const struct rte_eth_rxtx_callback *cur_cb;
/**< Callback instance */
volatile bool umwait_in_progress;
/**< are we currently sleeping? */
uint64_t empty_poll_stats;
/**< Number of empty polls */
} __rte_cache_aligned;
@ -92,30 +90,11 @@ clb_umwait(uint16_t port_id, uint16_t qidx, struct rte_mbuf **pkts __rte_unused,
struct rte_power_monitor_cond pmc;
uint16_t ret;
/*
* we might get a cancellation request while being
* inside the callback, in which case the wakeup
* wouldn't work because it would've arrived too early.
*
* to get around this, we notify the other thread that
* we're sleeping, so that it can spin until we're done.
* unsolicited wakeups are perfectly safe.
*/
q_conf->umwait_in_progress = true;
rte_atomic_thread_fence(__ATOMIC_SEQ_CST);
/* check if we need to cancel sleep */
if (q_conf->pwr_mgmt_state == PMD_MGMT_ENABLED) {
/* use monitoring condition to sleep */
ret = rte_eth_get_monitor_addr(port_id, qidx,
&pmc);
if (ret == 0)
rte_power_monitor(&pmc, UINT64_MAX);
}
q_conf->umwait_in_progress = false;
rte_atomic_thread_fence(__ATOMIC_SEQ_CST);
/* use monitoring condition to sleep */
ret = rte_eth_get_monitor_addr(port_id, qidx,
&pmc);
if (ret == 0)
rte_power_monitor(&pmc, UINT64_MAX);
}
} else
q_conf->empty_poll_stats = 0;
@ -177,12 +156,24 @@ clb_scale_freq(uint16_t port_id, uint16_t qidx,
return nb_rx;
}
static int
queue_stopped(const uint16_t port_id, const uint16_t queue_id)
{
struct rte_eth_rxq_info qinfo;
if (rte_eth_rx_queue_info_get(port_id, queue_id, &qinfo) < 0)
return -1;
return qinfo.queue_state == RTE_ETH_QUEUE_STATE_STOPPED;
}
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;
struct rte_eth_dev_info info;
rte_rx_callback_fn clb;
int ret;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
@ -203,6 +194,14 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id,
goto end;
}
/* check if the queue is stopped */
ret = queue_stopped(port_id, queue_id);
if (ret != 1) {
/* error means invalid queue, 0 means queue wasn't stopped */
ret = ret < 0 ? -EINVAL : -EBUSY;
goto end;
}
queue_cfg = &port_cfg[port_id][queue_id];
if (queue_cfg->pwr_mgmt_state != PMD_MGMT_DISABLED) {
@ -232,17 +231,7 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id,
ret = -ENOTSUP;
goto end;
}
/* initialize data before enabling the callback */
queue_cfg->empty_poll_stats = 0;
queue_cfg->cb_mode = mode;
queue_cfg->umwait_in_progress = false;
queue_cfg->pwr_mgmt_state = PMD_MGMT_ENABLED;
/* ensure we update our state before callback starts */
rte_atomic_thread_fence(__ATOMIC_SEQ_CST);
queue_cfg->cur_cb = rte_eth_add_rx_callback(port_id, queue_id,
clb_umwait, NULL);
clb = clb_umwait;
break;
}
case RTE_POWER_MGMT_TYPE_SCALE:
@ -269,16 +258,7 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id,
ret = -ENOTSUP;
goto end;
}
/* 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;
/* this is not necessary here, but do it anyway */
rte_atomic_thread_fence(__ATOMIC_SEQ_CST);
queue_cfg->cur_cb = rte_eth_add_rx_callback(port_id,
queue_id, clb_scale_freq, NULL);
clb = clb_scale_freq;
break;
}
case RTE_POWER_MGMT_TYPE_PAUSE:
@ -286,18 +266,21 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id,
if (global_data.tsc_per_us == 0)
calc_tsc();
/* 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;
/* this is not necessary here, but do it anyway */
rte_atomic_thread_fence(__ATOMIC_SEQ_CST);
queue_cfg->cur_cb = rte_eth_add_rx_callback(port_id, queue_id,
clb_pause, NULL);
clb = clb_pause;
break;
default:
RTE_LOG(DEBUG, POWER, "Invalid power management type\n");
ret = -EINVAL;
goto end;
}
/* 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);
ret = 0;
end:
return ret;
@ -308,12 +291,20 @@ rte_power_ethdev_pmgmt_queue_disable(unsigned int lcore_id,
uint16_t port_id, uint16_t queue_id)
{
struct pmd_queue_cfg *queue_cfg;
int ret;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
if (lcore_id >= RTE_MAX_LCORE || queue_id >= RTE_MAX_QUEUES_PER_PORT)
return -EINVAL;
/* check if the queue is stopped */
ret = queue_stopped(port_id, queue_id);
if (ret != 1) {
/* error means invalid queue, 0 means queue wasn't stopped */
return ret < 0 ? -EINVAL : -EBUSY;
}
/* no need to check queue id as wrong queue id would not be enabled */
queue_cfg = &port_cfg[port_id][queue_id];
@ -323,27 +314,8 @@ rte_power_ethdev_pmgmt_queue_disable(unsigned int lcore_id,
/* stop any callbacks from progressing */
queue_cfg->pwr_mgmt_state = PMD_MGMT_DISABLED;
/* ensure we update our state before continuing */
rte_atomic_thread_fence(__ATOMIC_SEQ_CST);
switch (queue_cfg->cb_mode) {
case RTE_POWER_MGMT_TYPE_MONITOR:
{
bool exit = false;
do {
/*
* we may request cancellation while the other thread
* has just entered the callback but hasn't started
* sleeping yet, so keep waking it up until we know it's
* done sleeping.
*/
if (queue_cfg->umwait_in_progress)
rte_power_monitor_wakeup(lcore_id);
else
exit = true;
} while (!exit);
}
/* fall-through */
case RTE_POWER_MGMT_TYPE_MONITOR: /* fall-through */
case RTE_POWER_MGMT_TYPE_PAUSE:
rte_eth_remove_rx_callback(port_id, queue_id,
queue_cfg->cur_cb);
@ -356,10 +328,11 @@ rte_power_ethdev_pmgmt_queue_disable(unsigned int lcore_id,
break;
}
/*
* we don't free the RX callback here because it is unsafe to do so
* unless we know for a fact that all data plane threads have stopped.
* the API doc mandates that the user stops all processing on affected
* ports before calling any of these API's, so we can assume that the
* callbacks can be freed. we're intentionally casting away const-ness.
*/
queue_cfg->cur_cb = NULL;
rte_free((void *)queue_cfg->cur_cb);
return 0;
}

6
lib/power/rte_power_pmd_mgmt.h
View File

@ -43,6 +43,9 @@ enum rte_power_pmd_mgmt_type {
*
* @note This function is not thread-safe.
*
* @warning This function must be called when all affected Ethernet queues are
* stopped and no Rx/Tx is in progress!
*
* @param lcore_id
* The lcore the Rx queue will be polled from.
* @param port_id
@ -69,6 +72,9 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id,
*
* @note This function is not thread-safe.
*
* @warning This function must be called when all affected Ethernet queues are
* stopped and no Rx/Tx is in progress!
*
* @param lcore_id
* The lcore the Rx queue is polled from.
* @param port_id