On Wed, Sep 24, 2014 at 11:31:45AM -0700, Jarno Rajahalme wrote:
> We use the 'counter' as a "lock" providing acquire-release
> semantics. Therefore we can use normal, non-atomic access to the
> memory accesses between the atomic accesses to 'counter'. The
> cmap_node.next needs to be RCU, so that can not be changed.
>
> Also rearrange code to benefit from the fact that hash values are
> unique in any bucket.
>
> This patch seems to make cmap_insert() a bit faster.
>
> Signed-off-by: Jarno Rajahalme <[email protected]>
At first, when I read this comment, I thought that it contradicted the
change and the commit message, since atomic reads are no longer needed
to read or write the hashes. It took me a bit longer to understand
that it is talking about the atomic_read_explicit() in read_counter():
/* Need to make sure the counter read is not moved up, before the hash and
* cmap_node_next(). Using atomic_read_explicit with memory_order_acquire
* still allow prior reads to be moved after the barrier.
* atomic_thread_fence prevents all following memory accesses from moving
* prior to preceding loads. */
atomic_thread_fence(memory_order_acquire);
How about editing the comment slightly to make this more obvious, e.g.:
/* Need to make sure the counter read is not moved up, before the hash and
* cmap_node_next(). The atomic_read_explicit() with memory_order_acquire
* in read_counter() still allows prior reads to be moved after the
* barrier. atomic_thread_fence prevents all following memory accesses
* from moving prior to preceding loads. */
Acked-by: Ben Pfaff <[email protected]>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev