[v2] l3fwd-power: make interrupt wakeup log thread safe
Checks
Commit Message
Currently, the interrupt status notification prevents log spam by
remembering whether previous interrupt wakeup was due to traffic or due
to timeout expiring. However, it is a single variable that can
potentially be accessed from multiple threads, so it is not thread-safe.
Fix it by having per-lcore interrupt status.
Fixes: f4d1e19c293d ("examples/l3fwd-power: add Rx interrupt timeout")
Cc: anatoly.burakov@intel.com
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
Notes:
v2:
- Fix confusing variable naming
examples/l3fwd-power/main.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
Comments
Hi Anatoly,
On 2/10/2020 1:07 PM, Anatoly Burakov wrote:
> Currently, the interrupt status notification prevents log spam by
> remembering whether previous interrupt wakeup was due to traffic or due
> to timeout expiring. However, it is a single variable that can
> potentially be accessed from multiple threads, so it is not thread-safe.
>
> Fix it by having per-lcore interrupt status.
>
> Fixes: f4d1e19c293d ("examples/l3fwd-power: add Rx interrupt timeout")
> Cc: anatoly.burakov@intel.com
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
Makes sense. Thanks.
Acked-by: David Hunt <david.hunt@intel.com>
Tested-by: Zhang, XiX <xix.zhang@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> Sent: Friday, October 2, 2020 8:07 PM
> To: dev@dpdk.org
> Cc: Hunt, David <david.hunt@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [dpdk-dev] [PATCH v2] l3fwd-power: make interrupt wakeup log
> thread safe
On Fri, Oct 9, 2020 at 8:27 AM Xie, WeiX <weix.xie@intel.com> wrote:
>
> Tested-by: Zhang, XiX <xix.zhang@intel.com>
A bit surprised to read a Tested-by: token sent for someone else.
Is this a typo?
Besides, it should be "Xi Zhang", no comma.
Tested-by: Zhang, XiX <xix.zhang@intel.com>
The tag is correct and I send the email on behalf of him, his mailbox can't receive dev patch. Thx.
Regards,
Xie Wei
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Hunt
> Sent: Wednesday, October 7, 2020 11:06 PM
> To: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] l3fwd-power: make interrupt wakeup log
> thread safe
>
> Hi Anatoly,
>
> On 2/10/2020 1:07 PM, Anatoly Burakov wrote:
> > Currently, the interrupt status notification prevents log spam by
> > remembering whether previous interrupt wakeup was due to traffic or
> > due to timeout expiring. However, it is a single variable that can
> > potentially be accessed from multiple threads, so it is not thread-safe.
> >
> > Fix it by having per-lcore interrupt status.
> >
> > Fixes: f4d1e19c293d ("examples/l3fwd-power: add Rx interrupt timeout")
> > Cc: anatoly.burakov@intel.com
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
>
>
> Makes sense. Thanks.
>
> Acked-by: David Hunt <david.hunt@intel.com>
>
>
>
On Fri, Oct 2, 2020 at 2:07 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> Currently, the interrupt status notification prevents log spam by
> remembering whether previous interrupt wakeup was due to traffic or due
> to timeout expiring. However, it is a single variable that can
> potentially be accessed from multiple threads, so it is not thread-safe.
>
> Fix it by having per-lcore interrupt status.
>
> Fixes: f4d1e19c293d ("examples/l3fwd-power: add Rx interrupt timeout")
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: David Hunt <david.hunt@intel.com>
Tested-by: Xi Zhang <xix.zhang@intel.com>
Applied, thanks.
@@ -821,20 +821,23 @@ power_freq_scaleup_heuristic(unsigned lcore_id,
* 0 on success
*/
static int
-sleep_until_rx_interrupt(int num)
+sleep_until_rx_interrupt(int num, int lcore)
{
/*
* we want to track when we are woken up by traffic so that we can go
- * back to sleep again without log spamming.
+ * back to sleep again without log spamming. Avoid cache line sharing
+ * to prevent threads stepping on each others' toes.
*/
- static bool timeout;
+ static struct {
+ bool wakeup;
+ } __rte_cache_aligned status[RTE_MAX_LCORE];
struct rte_epoll_event event[num];
int n, i;
uint16_t port_id;
uint8_t queue_id;
void *data;
- if (!timeout) {
+ if (status[lcore].wakeup) {
RTE_LOG(INFO, L3FWD_POWER,
"lcore %u sleeps until interrupt triggers\n",
rte_lcore_id());
@@ -851,7 +854,7 @@ sleep_until_rx_interrupt(int num)
" port %d queue %d\n",
rte_lcore_id(), port_id, queue_id);
}
- timeout = n == 0;
+ status[lcore].wakeup = n != 0;
return 0;
}
@@ -1050,7 +1053,8 @@ static int main_intr_loop(__rte_unused void *dummy)
if (intr_en) {
turn_on_off_intr(qconf, 1);
sleep_until_rx_interrupt(
- qconf->n_rx_queue);
+ qconf->n_rx_queue,
+ lcore_id);
turn_on_off_intr(qconf, 0);
/**
* start receiving packets immediately
@@ -1473,7 +1477,8 @@ main_legacy_loop(__rte_unused void *dummy)
if (intr_en) {
turn_on_off_intr(qconf, 1);
sleep_until_rx_interrupt(
- qconf->n_rx_queue);
+ qconf->n_rx_queue,
+ lcore_id);
turn_on_off_intr(qconf, 0);
/**
* start receiving packets immediately