I'd really hope that any serious OVS implementation would be able to
use C11 or GCC or Clang or other compiler-specific techniques to get a
"real" implementation of atomics. Really the pthreads version is just
to make porting easier.
On Mon, Jun 02, 2014 at 11:07:12AM -0700, Jarno Rajahalme wrote:
> As I?m testing for this I notice that the cmap (or, ovs-rcu) with
> ova-atomic-pthreads is painfully slow. The reason for this is the
> the generic pthreads atomics have no memory barrier support, which
> is needed for efficient RCU. As we make more use of ovs-rcu and cmap
> we need to address this in one way or another. it may even become
> necessary to deprecate support for systems for which we have no
> memory barriers.
>
> Thoughts?
>
> Jarno
>
> On Jun 2, 2014, at 10:43 AM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>
> > Thanks for reporting this!
> >
> > I think we should allow the illusion that ovsrcu_init is a function, and
> > it?s parameters are evaluated before the body. Otherwise problems like this
> > are bound to be reintroduced. I?ll post a patch soon.
> >
> > Jarno
> >
> > On Jun 2, 2014, at 3:34 AM, YAMAMOTO Takashi <yamam...@valinux.co.jp> wrote:
> >
> >> In the case of ovs-atomic-pthreads, the previous coding is expanded
> >> into successive atomic_lock__ calls which ends up with deadlock.
> >>
> >> Signed-off-by: YAMAMOTO Takashi <yamam...@valinux.co.jp>
> >> ---
> >> lib/cmap.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/cmap.c b/lib/cmap.c
> >> index a760235..7892d50 100644
> >> --- a/lib/cmap.c
> >> +++ b/lib/cmap.c
> >> @@ -690,7 +690,9 @@ cmap_replace__(struct cmap_impl *impl, struct
> >> cmap_node *node,
> >> 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));
> >> + struct cmap_node *next = cmap_node_next_protected(node);
> >> +
> >> + ovsrcu_init(&replacement->next, next);
> >> }
> >>
> >> struct cmap_node *iter = &b->nodes[slot];
> >> --
> >> 1.8.3.1
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev@openvswitch.org
> >> http://openvswitch.org/mailman/listinfo/dev
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
