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 <jrajaha...@nicira.com>

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 <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to