On Jun 2, 2014, at 11:45 AM, Ben Pfaff <[email protected]> wrote: > 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. >
Are we already documenting this somewhere? Jarno > 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >>>> --- >>>> 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 >>>> [email protected] >>>> http://openvswitch.org/mailman/listinfo/dev >>> >> >> _______________________________________________ >> dev mailing list >> [email protected] >> http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
