Acked-by: Jarno Rajahalme <[email protected]>
I still worry about reordering of the non-atomic reads in between the
read_counter() calls, as in principle the acquire barrier does not prevent
loads before such a barrier to be moved after it. Thus, this could happen:
for (;;) {
c = read_even_counter()
check = read_counter()
read hash value
read camp_node pointer
} while (c != check)
While we need something like this:
for (;;) {
c = read_even_counter()
atomic_thread_fence(memory_order_acquire);
read hash value
read camp_node pointer
atomic_thread_fence(memory_order_acquire);
check = read_counter()
} while (c != check)
to guarantee that the hash and node values are read after the even counter and
before the check.
I’ll post a later patch using atomic_thread_fence(memory_model_acquire) to
address this.
Jarno
On May 21, 2014, at 7:43 AM, Ben Pfaff <[email protected]> wrote:
> Release memory ordering only affects visibility of stores, and is not
> allowed on a memory read. Some compilers enforce this, making this code
> fail to compile.
>
> Reported-by: Alex Wang <[email protected]>
> Reported-by: Kmindg G <[email protected]>
> CC: Jarno Rajahalme <[email protected]>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
> lib/cmap.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/cmap.c b/lib/cmap.c
> index cfd66fb..1eb79b4 100644
> --- a/lib/cmap.c
> +++ b/lib/cmap.c
> @@ -245,11 +245,11 @@ cmap_is_empty(const struct cmap *cmap)
> }
>
> static uint32_t
> -read_counter(struct cmap_bucket *bucket, memory_order order)
> +read_counter(struct cmap_bucket *bucket)
> {
> uint32_t counter;
>
> - atomic_read_explicit(&bucket->counter, &counter, order);
> + atomic_read_explicit(&bucket->counter, &counter, memory_order_acquire);
> return counter;
> }
>
> @@ -259,7 +259,7 @@ read_even_counter(struct cmap_bucket *bucket)
> uint32_t counter;
>
> do {
> - counter = read_counter(bucket, memory_order_acquire);
> + counter = read_counter(bucket);
> } while (OVS_UNLIKELY(counter & 1));
>
> return counter;
> @@ -268,7 +268,7 @@ read_even_counter(struct cmap_bucket *bucket)
> static bool
> counter_changed(struct cmap_bucket *b, uint32_t c)
> {
> - return OVS_UNLIKELY(read_counter(b, memory_order_release) != c);
> + return OVS_UNLIKELY(read_counter(b) != c);
> }
>
> /* Searches 'cmap' for an element with the specified 'hash'. If one or more
> is
> --
> 1.9.1
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
