On Wed, May 28, 2014 at 04:48:41PM -0700, Jarno Rajahalme wrote:
>
> On May 28, 2014, at 4:14 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>
> >
> > On May 23, 2014, at 10:23 AM, Ben Pfaff <b...@nicira.com> wrote:
> >
> >> 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 <jrajaha...@nicira.com>
> >>
> >> 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);
> >>
> >
> > It seems to me that moving the fence up one line would solve the problem,
> > no?
>
> I just re-realized that this will go way with the patch 5/5
> anyway. Do you want me to merge patches 4 and 5, instead?
I'd rather not mix these two patches up. Each one is confusing enough
on its own.
I think you're right that moving up the fence solves the issue.
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
