[ltt-dev] [PATCH RFC urcu] add pthread_atfork interfaces for call_rcu

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> Hello!
> 
> Please see below for an RFC patch for creating pthread_atfork()-friendly
> interfaces for call_rcu().
> 
> However, I ran into a problem when trying to test this.  I cannot use
> this except with rcu-bp, since only rcu-bp can be made fork()-safe.
> OK, easy enough to create the corresponding interfaces for the other
> flavors of RCU.  But this led to a couple of issues:
> 
> 1.	Do we intend to support mixing multiple flavors of RCU in the
> 	same program?  Trying this with the current implementations
> 	results in symbol collisions (multiple different versions
> 	of synchronize_rcu(), for example).  It is possible to use
> 	preprocessor magic to handle this, but thought I should check
> 	on the intent before attempting this.

In the case of URCU bp, I indend to statically link UST to it. But I
agree it's a hack. Would you rather prefix all symbols with names
specific to their own flavor ? It makes it slightly harder to change
flavors in an application, although it lessens the risk of bugs due to
linking with the from urcu flavor (mismatch between headers and urcu
library).

> 2.	Do we intend to support use of RCU from library functions?
> 	If the answer is "yes", then we are pretty much stuck supporting
> 	multiple flavors of RCU in the same program, because different
> 	library functions might reasonably choose different of the
> 	RCU implementations.

Indeed, although we might want to wonder if we want to allow only the
static linking case from libraries or do we want to support dynamic
linking too ?

> 
> 3.	Do we intend to support use of UST on programs that use
> 	userspace-rcu?  If so, we again might need to tolerate mixing
> 	of RCU flavors, given that UST uses rcu-bp and the programs
> 	in questions might reasonably make a different choice.

Yes, although the static vs dynamic linking question remains.

> 
> 4.	Do we intend to support use of a given flavor of RCU from
> 	multiple library modules, such that a given program might
> 	link any number of the RCU-using library modules?  If so,
> 	we need initialization to be idempotent.

Actually, we'd need to refcount the library here, right ?

> 
> 5.	Do we intend to support use of a given flavor of RCU from
> 	multiple library modules, some of which might fork() and others
> 	of which might not?  If so, we need to pull the handling of
> 	pthread_atfork() into the userspace-rcu library -- otherwise,
> 	multiple users of RCU might call pthread_atfork(), resulting
> 	in self-deadlock at fork() time.
> 
> 	Perhaps a flag on rcu_register_thread() indicating that the
> 	thread registering itself might fork()?  Or, for compatibility,
> 	a new rcu_register_thread_flags() that has a bitfield with
> 	options.  The first thread that indicated that it might fork
> 	would then cause userspace-rcu to do the required calls to
> 	pthread_atfork().

Looks like a good idea. In the case of urcu-bp (which might eventually
use call rcu), I would like to leave the pre/post fork symbols available
too, so the register thread could specify, for these, that the pre/post
fork hooks should *not* be automatically invoked, leaving this to the
caller.

So the standard behavior for all urcu flavors, urcu defer and urcu
call_rcu would be not to call any of these callbacks.

If the application/user library needs to support fork()s, it would call
rcu_register_thread_flags() (and the similar functions for urcu defer
and call_rcu()) to indicate that forks must be supported, which will
register pthread atfork handlers (this should be refcounted).

In the case of urcu-bp, it would provide the callbacks that can be used
to deal with forks. We can keep track of which callbacks have been
invoked during execution with a counter incremented as pre-fork,
decremented post-fork, so if we have two "clients" calling pre fork
hooks, only the first one will be executed, the second will be skipped.
The post fork hooks would decrement the counter.

call_rcu should provide both the symbols of pre/post fork hooks (with
the counter scheme, like urcu-bp), and also the ability to register with
thread_flags, so pthread_atfork would be used to handle forks(). Note
that in a combination where we have some "clients" using the
pthread_atfork() scheme, and others using direct calls to the pre/post
fork symbols, the counter scheme would ensure that the hooks only get
called once. The execution order of the UST fork symbol override is
that the UST code executes first before the fork, and last after the
fork, and the original libc symbol is called in the middle (which will
trigger atfork).

Does what I'm proposing here make sense ?

> 
> I suspect that the answer is "yes" for all of the above questions.
> 
> Does this all make sense, or am I confused?
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> Provide pthread_atfork-friendly interfaces
> 
> Provides call_rcu_before_fork() and call_rcu_after_fork_parent() to
> go with the existing call_rcu_after_fork_child().

For the patch below, it might be better to also implement the flag
scheme using atfork in addition to exporting the symbol, and to add the
counter scheme to keep track of multiple calls.

> 
> Signed-off-by: Paul E. McKenney <paulmck at linux.vnet.ibm.com>
> 
> diff --git a/README b/README
> index f7f0dec..56e98d7 100644
> --- a/README
> +++ b/README
> @@ -204,3 +204,7 @@ Interaction with fork()
>  	liburcu-bp, which is designed to handle fork() by calling
>  	rcu_bp_before_fork, rcu_bp_after_fork_parent and
>  	rcu_bp_after_fork_child.
> +
> +	Applications that use call_rcu() are required to invoke
> +	call_rcu_after_fork_child() from the child process after a
> +	successful fork() system call that is not followed by exec().

Not true for the atfork scheme.

> diff --git a/urcu-call-rcu.c b/urcu-call-rcu.c
> index bb56dbb..665f20c 100644
> --- a/urcu-call-rcu.c
> +++ b/urcu-call-rcu.c
> @@ -566,13 +566,40 @@ void free_all_cpu_call_rcu_data(void)
>  }
>  
>  /*
> + * Acquire the call_rcu_mutex in order to ensure that the child sees
> + * all of the call_rcu() data structures in a consistent state.
> + * Suitable for pthread_atfork() and friends.
> + */
> +void call_rcu_before_fork(void)
> +{
> +	call_rcu_lock(&call_rcu_mutex);
> +}
> +
> +/*
> + * Clean up call_rcu data structures in the parent of a successful fork()
> + * that is not followed by exec() in the child.  Suitable for
> + * pthread_atfork() and friends.
> + */
> +void call_rcu_after_fork_parent(void)
> +{
> +	call_rcu_unlock(&call_rcu_mutex);
> +}
> +
> +/*
>   * Clean up call_rcu data structures in the child of a successful fork()
> - * that is not followed by exec().
> + * that is not followed by exec().  Suitable for pthread_atfork() and
> + * friends.
>   */
>  void call_rcu_after_fork_child(void)
>  {
>  	struct call_rcu_data *crdp;
>  
> +	/* Re-initialize the mutex. */
> +	if (pthread_mutex_init(&call_rcu_mutex, NULL) != 0) {

Silly question: why reinitialize the mutex rather than unlock it ?

Thanks,

Mathieu

> +		perror("pthread_mutex_init");
> +		exit(-1);
> +	}
> +
>  	/*
>  	 * Allocate a new default call_rcu_data structure in order
>  	 * to get a working call_rcu thread to go with it.
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com