On Thu, May 22, 2014 at 05:37:41PM -0700, Jarno Rajahalme wrote:
> Add cmap_replace() and cmap_first(), as well as CMAP_FOR_EACH_SAFE and
> CMAP_FOR_EACH_CONTINUE to make porting existing hmap using code a bit
> easier.
>
> CMAP_FOR_EACH_SAFE is useful in RCU postponed destructors, when it is
> known that additional postponing is not needed.
>
> Signed-off-by: Jarno Rajahalme <[email protected]>
I am not sure that ovsrcu_init() may be used in cmap_replace__() here.
I believe that we need to make sure that the store to replacement->next
becomes visible before the store to b->nodes[slot]. I am not sure that
a single release fence is sufficient, because I believe that the write
to replacement->next could still get reordered after the write to
b->nodes[slot]. After all, both are on the same side of the release
fence:
/* The pointer to 'node' is changed to point to 'replacement',
* which is the next node if no replacement node is given. */
if (!replacement) {
replacement = cmap_node_next_protected(node);
} else {
/* 'replacement' takes the position of 'node' in the list. */
ovsrcu_init(&replacement->next, cmap_node_next_protected(node));
}
if (b->nodes[slot] == node) {
b->nodes[slot] = replacement;
atomic_thread_fence(memory_order_release);
The purpose of CMAP_FOR_EACH_SAFE in cmap is different enough from the
typical use to deserve an explanatory comment. (At least, the comment
claiming that there is no CMAP_FOR_EACH_SAFE macro needs to be fixed.)
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
