diff mbox series

[ovs-dev] libs: Replace weak with strong compare and exchange

Message ID 20211020074750.2023-1-anton.ivanov@cambridgegreys.com
State Accepted
Headers show
Series [ovs-dev] libs: Replace weak with strong compare and exchange | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Anton Ivanov Oct. 20, 2021, 7:47 a.m. UTC 
From: Anton Ivanov <anton.ivanov@cambridgegreys.com>

Weak may fail for reasons unrelated to the comparison (not
on x86, where these are practically equivalent).

Thus, it can and should be used only in cases where there will
be a repeat of the event which runs the code (in this case
semaphore post).

On non-x86 architectures using weak may result in a failed
comparison without any more sem post events - a hang. Reported
for arm, likely for other platforms where weak may fail.

Reported-by: wentao.jia@easystack.cn

Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com>
---
 lib/ovn-parallel-hmap.c | 2 +-
 ovs                     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Numan Siddique Oct. 21, 2021, 1:02 a.m. UTC | #1 
On Wed, Oct 20, 2021 at 3:48 AM <anton.ivanov@cambridgegreys.com> wrote:
>
> From: Anton Ivanov <anton.ivanov@cambridgegreys.com>
>
> Weak may fail for reasons unrelated to the comparison (not
> on x86, where these are practically equivalent).
>
> Thus, it can and should be used only in cases where there will
> be a repeat of the event which runs the code (in this case
> semaphore post).
>
> On non-x86 architectures using weak may result in a failed
> comparison without any more sem post events - a hang. Reported
> for arm, likely for other platforms where weak may fail.
>
> Reported-by: wentao.jia@easystack.cn
>
> Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com>

Thanks.   I applied this patch to the main branch and backported to
branch-21.09 and branch-21.06.

Numan

> ---
>  lib/ovn-parallel-hmap.c | 2 +-
>  ovs                     | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c
> index b8c7ac786..56ceed8e8 100644
> --- a/lib/ovn-parallel-hmap.c
> +++ b/lib/ovn-parallel-hmap.c
> @@ -267,7 +267,7 @@ ovn_run_pool_callback(struct worker_pool *pool,
>               * (most likely acq_rel) to ensure that the main thread
>               * sees all of the results produced by the worker.
>               */
> -            if (atomic_compare_exchange_weak(
> +            if (atomic_compare_exchange_strong(
>                      &pool->controls[index].finished,
>                      &test,
>                      false)) {
> --
> 2.20.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c
index b8c7ac786..56ceed8e8 100644
--- a/lib/ovn-parallel-hmap.c
+++ b/lib/ovn-parallel-hmap.c
@@ -267,7 +267,7 @@  ovn_run_pool_callback(struct worker_pool *pool,
              * (most likely acq_rel) to ensure that the main thread
              * sees all of the results produced by the worker.
              */
-            if (atomic_compare_exchange_weak(
+            if (atomic_compare_exchange_strong(
                     &pool->controls[index].finished,
                     &test,
                     false)) {