diff mbox series

[ovs-dev,v1,1/2] cmap: add thread fence for slot update

Message ID 20191118024519.11367-2-Yanqin.Wei@arm.com
State Accepted
Headers show
Series improve cmap read-write concurrency | expand

Commit Message

Yanqin Wei Nov. 18, 2019, 2:45 a.m. UTC 
bucket update in the cmap lib is protected by a counter. But hash setting
is possible to be moved before counter update. This patch fix this issue.

Reviewed-by: Ola Liljedahl <Ola.Liljedahl@arm.com>
Reviewed-by: Gavin Hu <Gavin.Hu@arm.com>
Signed-off-by: Yanqin Wei <Yanqin.Wei@arm.com>
---
 lib/cmap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Ilya Maximets Oct. 18, 2022, 12:33 p.m. UTC | #1 
On 11/18/19 03:45, Yanqin Wei wrote:
> bucket update in the cmap lib is protected by a counter. But hash setting
> is possible to be moved before counter update. This patch fix this issue.
> 
> Reviewed-by: Ola Liljedahl <Ola.Liljedahl@arm.com>
> Reviewed-by: Gavin Hu <Gavin.Hu@arm.com>
> Signed-off-by: Yanqin Wei <Yanqin.Wei@arm.com>
> ---
>  lib/cmap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/cmap.c b/lib/cmap.c
> index c9eef3f4a..e41c40794 100644
> --- a/lib/cmap.c
> +++ b/lib/cmap.c
> @@ -598,7 +598,9 @@ cmap_set_bucket(struct cmap_bucket *b, int i,
>      uint32_t c;
>  
>      atomic_read_explicit(&b->counter, &c, memory_order_acquire);
> -    atomic_store_explicit(&b->counter, c + 1, memory_order_release);
> +    atomic_store_explicit(&b->counter, c + 1, memory_order_relaxed);
> +    /*need to make sure setting hash is not moved up before counter update*/
> +    atomic_thread_fence(memory_order_release);
>      ovsrcu_set(&b->nodes[i].next, node); /* Also atomic. */
>      b->hashes[i] = hash;
>      atomic_store_explicit(&b->counter, c + 2, memory_order_release);
> 

Looking through the oldest patches in our patchwork instance and this
seems to be a valid bug fix.

Applied now.  Also backported down to 2.17.  Sorry it took that long.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/cmap.c b/lib/cmap.c
index c9eef3f4a..e41c40794 100644
--- a/lib/cmap.c
+++ b/lib/cmap.c
@@ -598,7 +598,9 @@  cmap_set_bucket(struct cmap_bucket *b, int i,
     uint32_t c;
 
     atomic_read_explicit(&b->counter, &c, memory_order_acquire);
-    atomic_store_explicit(&b->counter, c + 1, memory_order_release);
+    atomic_store_explicit(&b->counter, c + 1, memory_order_relaxed);
+    /*need to make sure setting hash is not moved up before counter update*/
+    atomic_thread_fence(memory_order_release);
     ovsrcu_set(&b->nodes[i].next, node); /* Also atomic. */
     b->hashes[i] = hash;
     atomic_store_explicit(&b->counter, c + 2, memory_order_release);