[ovs-dev] [PATCH 3/5] lib/ovs-rcu: Fix documentation, add ovsrcu_init().

Ben Pfaff blp at nicira.com
Wed May 28 22:58:26 UTC 2014


On Wed, May 28, 2014 at 03:54:34PM -0700, Jarno Rajahalme wrote:
> 
> On May 23, 2014, at 10:11 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Thu, May 22, 2014 at 05:37:40PM -0700, Jarno Rajahalme wrote:
> >> lib/ovs-rcu.h had some of the comments duplicated.
> >> 
> >> Add ovsrcu_init() that can be used like ovsrcu_set() when the RCU
> >> protected pointer is not yet visible any readers.
> >> 
> >> Use OVS_CONSTRUCTOR to initialize the ovs-rcu module.
> >> 
> >> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> > 
> > I get nervous to start to see modules using OVS_CONSTRUCTOR to do fairly
> > complex work.  Because there is no dependency system for constructors,
> > the order in which they are invoked is unpredictable and can vary from
> > one compiler or system to another.  If we are not perfectly careful to
> > make sure that there are no actual dependencies among them, then that
> > can lead to surprising crashes at startup time.
> 
> Yeah, I have struggled with nasty bugs due to this issue in the past, but already forgot about the pain, apparently :-)
> 
> in general, I like the idea of getting rid of checking for initialization from all the possible entry points.
> 
> We already have a ?module? system for VLOG, maybe we could generalize it to include module initialization with some kind of dependency/priority system?

I agree that it's better to avoid needing to check for
initialization, but I don't have a good idea for how to craft a
dependency or priority system.  Do you have some idea?

> However, I totally see your point, we are calling non-trivial
> functions here, so I am happy to roll this back for now. However,
> I?d like to use the name ?ovsrcu_init? for the new purpose, as it
> rhymes with ?atomic_init?, which has corresponding semantics. You
> fine with using ovsrcu_init_module() instead:

That's fine, thanks.

> > I'm happy with the comment update.
> > 
> 
> :-)

:-)



More information about the dev mailing list