From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 66CCCA0518 for ; Fri, 24 Jul 2020 14:07:30 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5D8CF1C134; Fri, 24 Jul 2020 14:07:30 +0200 (CEST) Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by dpdk.org (Postfix) with ESMTP id 535BF1BFE7 for ; Fri, 24 Jul 2020 14:07:29 +0200 (CEST) Received: by mail-wm1-f66.google.com with SMTP id t142so1682845wmt.4 for ; Fri, 24 Jul 2020 05:07:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=yZsUOSbjUD9m9RnfUJ8CsoAGZIXVHn2muljeckG1TEc=; b=ZalGuTyMYMZ6fo/ol9Wb6NcMgu4oLFNSUQCga1bPeXSXOOVgQrynhYfzPN8BwY8fCB 8D3E6ak6YMfLiGU7HNkd/YPsjzRRGAGEbwkJ8aMpfabOIFUykMSqYbVdzukb9q7xXu2Z wp/+/e4gN4iDkqtBWPI/lFk60iuLHEoGkR4/m3zSbZ6RItGdWZM0odvAouYiNlAhNRm3 IqfCo1GwVhso77eO0tdo6jE2DVXIhHigWO5F16GT+L+hPiNdY0tPlviCAOkR2yS6i04p X+KrrUwPgeMQnRYnBipdzYPo7H1un0drHyoy1J69LwE28asNrc9+5coigaxgaDgjKSvI ll4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=yZsUOSbjUD9m9RnfUJ8CsoAGZIXVHn2muljeckG1TEc=; b=sB1k73fY+I5PLKpq4OkFc4IqZLpIz2uHwuSfgTM6eogRyGhhKmRDsIWeerf90BlH/J DJbmwV7yenWfN7nOtPN2KaDAKChW1yOS1L9GlH6TERAI+fmxu2FNgCr5A5oBQ3U4CZbx l/b2y1One5r5ZrDl0h+NWZafxSb3/qRLzL058fW3oY9eR3vPNNeAWrNmrTj4AgyuRrRK 5GP3ZOrR3OfI3RJTcoRdFhKcIIMFd+7Axb/ewBRG46GLtfGA4LVfkpQ1J+m61CBobW+j x5LRAHkYQNNiKL0gS4DPVgSbVCOqC4kXXPGJaWZ2+J0SEE/TBxFlBmD2RQtbIH1IQmbk L+RA== X-Gm-Message-State: AOAM533Vyl+A2ERddLoyyt+sffMJt66TYgZkA700OLFqCU0PnuumdVRN cx2E/1Z9x/ZaOMbCFKtRo0PaQicXgDEzqw== X-Google-Smtp-Source: ABdhPJxbvz4K+L+2rpuo2JSby9YymCILba72ux+p4Dxr7JzE2LTy5/7TaIiV4ec49bKoB64AHJ22vw== X-Received: by 2002:a1c:3546:: with SMTP id c67mr8600259wma.102.1595592449034; Fri, 24 Jul 2020 05:07:29 -0700 (PDT) Received: from localhost ([88.98.246.218]) by smtp.gmail.com with ESMTPSA id w7sm7254570wmc.32.2020.07.24.05.07.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Jul 2020 05:07:28 -0700 (PDT) From: luca.boccassi@gmail.com To: Phil Yang Cc: Dharmik Thakkar , Ruifeng Wang , Erik Gabriel Carrillo , dpdk stable Date: Fri, 24 Jul 2020 12:58:59 +0100 Message-Id: <20200724120030.1863487-101-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200724120030.1863487-1-luca.boccassi@gmail.com> References: <20200724120030.1863487-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'eventdev: use C11 atomics for lcore timer armed flag' has been queued to stable release 19.11.4 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, FYI, your patch has been queued to stable release 19.11.4 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 07/26/20. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Thanks. Luca Boccassi --- >From 3939291be17174407903296e4f47986515e3e9fc Mon Sep 17 00:00:00 2001 From: Phil Yang Date: Tue, 7 Jul 2020 23:54:51 +0800 Subject: [PATCH] eventdev: use C11 atomics for lcore timer armed flag [ upstream commit 1028d63eb214c5961b2df9d4b5662dab082b839e ] The in_use flag is a per core variable which is not shared between lcores in the normal case and the access of this variable should be ordered on the same core. However, if non-EAL thread pick the highest lcore to insert timers into, there is the possibility of conflicts on this flag between threads. Then the atomic compare-and-swap operation is needed. Use the C11 atomics instead of the generic rte_atomic operations to avoid the unnecessary barrier on aarch64. Signed-off-by: Phil Yang Reviewed-by: Dharmik Thakkar Reviewed-by: Ruifeng Wang Acked-by: Erik Gabriel Carrillo --- lib/librte_eventdev/rte_event_timer_adapter.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c b/lib/librte_eventdev/rte_event_timer_adapter.c index 76885972e..678be9f0a 100644 --- a/lib/librte_eventdev/rte_event_timer_adapter.c +++ b/lib/librte_eventdev/rte_event_timer_adapter.c @@ -550,7 +550,7 @@ struct swtim { uint32_t timer_data_id; /* Track which cores have actually armed a timer */ struct { - rte_atomic16_t v; + uint16_t v; } __rte_cache_aligned in_use[RTE_MAX_LCORE]; /* Track which cores' timer lists should be polled */ unsigned int poll_lcores[RTE_MAX_LCORE]; @@ -602,7 +602,8 @@ swtim_callback(struct rte_timer *tim) "with immediate expiry value"); } - if (unlikely(rte_atomic16_test_and_set(&sw->in_use[lcore].v))) { + if (unlikely(sw->in_use[lcore].v == 0)) { + sw->in_use[lcore].v = 1; n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1, __ATOMIC_RELAXED); __atomic_store_n(&sw->poll_lcores[n_lcores], lcore, @@ -830,7 +831,7 @@ swtim_init(struct rte_event_timer_adapter *adapter) /* Initialize the variables that track in-use timer lists */ for (i = 0; i < RTE_MAX_LCORE; i++) - rte_atomic16_init(&sw->in_use[i].v); + sw->in_use[i].v = 0; /* Initialize the timer subsystem and allocate timer data instance */ ret = rte_timer_subsystem_init(); @@ -1013,6 +1014,8 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter, struct rte_timer *tim, *tims[nb_evtims]; uint64_t cycles; int n_lcores; + /* Timer list for this lcore is not in use. */ + uint16_t exp_state = 0; #ifdef RTE_LIBRTE_EVENTDEV_DEBUG /* Check that the service is running. */ @@ -1031,8 +1034,12 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter, /* If this is the first time we're arming an event timer on this lcore, * mark this lcore as "in use"; this will cause the service * function to process the timer list that corresponds to this lcore. + * The atomic compare-and-swap operation can prevent the race condition + * on in_use flag between multiple non-EAL threads. */ - if (unlikely(rte_atomic16_test_and_set(&sw->in_use[lcore_id].v))) { + if (unlikely(__atomic_compare_exchange_n(&sw->in_use[lcore_id].v, + &exp_state, 1, 0, + __ATOMIC_RELAXED, __ATOMIC_RELAXED))) { EVTIM_LOG_DBG("Adding lcore id = %u to list of lcores to poll", lcore_id); n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1, -- 2.20.1 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2020-07-24 12:53:52.442169935 +0100 +++ 0101-eventdev-use-C11-atomics-for-lcore-timer-armed-flag.patch 2020-07-24 12:53:48.343007503 +0100 @@ -1,8 +1,10 @@ -From 1028d63eb214c5961b2df9d4b5662dab082b839e Mon Sep 17 00:00:00 2001 +From 3939291be17174407903296e4f47986515e3e9fc Mon Sep 17 00:00:00 2001 From: Phil Yang Date: Tue, 7 Jul 2020 23:54:51 +0800 Subject: [PATCH] eventdev: use C11 atomics for lcore timer armed flag +[ upstream commit 1028d63eb214c5961b2df9d4b5662dab082b839e ] + The in_use flag is a per core variable which is not shared between lcores in the normal case and the access of this variable should be ordered on the same core. However, if non-EAL thread pick the highest @@ -13,8 +15,6 @@ Use the C11 atomics instead of the generic rte_atomic operations to avoid the unnecessary barrier on aarch64. -Cc: stable@dpdk.org - Signed-off-by: Phil Yang Reviewed-by: Dharmik Thakkar Reviewed-by: Ruifeng Wang @@ -24,10 +24,10 @@ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c b/lib/librte_eventdev/rte_event_timer_adapter.c -index 370ea40a7..4ed301382 100644 +index 76885972e..678be9f0a 100644 --- a/lib/librte_eventdev/rte_event_timer_adapter.c +++ b/lib/librte_eventdev/rte_event_timer_adapter.c -@@ -554,7 +554,7 @@ struct swtim { +@@ -550,7 +550,7 @@ struct swtim { uint32_t timer_data_id; /* Track which cores have actually armed a timer */ struct { @@ -36,7 +36,7 @@ } __rte_cache_aligned in_use[RTE_MAX_LCORE]; /* Track which cores' timer lists should be polled */ unsigned int poll_lcores[RTE_MAX_LCORE]; -@@ -606,7 +606,8 @@ swtim_callback(struct rte_timer *tim) +@@ -602,7 +602,8 @@ swtim_callback(struct rte_timer *tim) "with immediate expiry value"); } @@ -46,7 +46,7 @@ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1, __ATOMIC_RELAXED); __atomic_store_n(&sw->poll_lcores[n_lcores], lcore, -@@ -834,7 +835,7 @@ swtim_init(struct rte_event_timer_adapter *adapter) +@@ -830,7 +831,7 @@ swtim_init(struct rte_event_timer_adapter *adapter) /* Initialize the variables that track in-use timer lists */ for (i = 0; i < RTE_MAX_LCORE; i++) @@ -55,7 +55,7 @@ /* Initialize the timer subsystem and allocate timer data instance */ ret = rte_timer_subsystem_init(); -@@ -1017,6 +1018,8 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter, +@@ -1013,6 +1014,8 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter, struct rte_timer *tim, *tims[nb_evtims]; uint64_t cycles; int n_lcores; @@ -64,7 +64,7 @@ #ifdef RTE_LIBRTE_EVENTDEV_DEBUG /* Check that the service is running. */ -@@ -1035,8 +1038,12 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter, +@@ -1031,8 +1034,12 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter, /* If this is the first time we're arming an event timer on this lcore, * mark this lcore as "in use"; this will cause the service * function to process the timer list that corresponds to this lcore.