Re: [PATCH 1/2] srcu-3: RCU variant permitting read-side blocking

["Paul E. McKenney" <paulmck@us.ibm.com>]

On Fri, Jul 07, 2006 at 10:58:28AM -0400, Alan Stern wrote:
> On Thu, 6 Jul 2006, Paul E. McKenney wrote:
> 
> > Is this what the two of you are getting at?
> > 
> > #define DEFINE_SRCU_STRUCT(name) \
> > 	DEFINE_PER_CPU(struct srcu_struct_array, name) = { 0, 0 }; \
> > 	struct srcu_struct name = { \
> > 		.completed = 0, \
> > 		.per_cpu_ref = NULL, \
> > 		.mutex = __MUTEX_INITIALIZER(name.mutex) \
> > 	}
> 
> Note that this approach won't work when you need to do something like:
> 
> 	struct xyz {
> 		struct srcu_struct s;
> 	} the_xyz = {
> 		.s = /* What goes here? */
> 	};

Yep, this the same issue leading to my complaint below about not being
able to pass a pointer to the resulting srcu_struct.

> > #define srcu_read_lock(ss) \
> > 	({ \
> > 		if ((ss)->per_cpu_ref != NULL) \
> > 			srcu_read_lock_dynamic(&ss); \
> > 		else { \
> > 			int ret; \
> > 			\
> > 			preempt_disable(); \
> > 			ret = srcu_read_lock_static(&ss, &__get_cpu_var(ss)); \
> > 			preempt_enable(); \
> > 			ret; \
> > 		} \
> > 	})
> > 
> > int srcu_read_lock_dynamic(struct srcu_struct *sp)
> > {
> > 	int idx;
> > 
> > 	preempt_disable();
> > 	idx = sp->completed & 0x1;
> > 	barrier();  /* ensure compiler looks -once- at sp->completed. */
> > 	per_cpu_ptr(sp->per_cpu_ref, smp_processor_id())->c[idx]++;
> > 	srcu_barrier();  /* ensure compiler won't misorder critical section. */
> > 	preempt_enable();
> > 	return idx;
> > }
> > 
> > int srcu_read_lock_static(struct srcu_struct *sp, srcu_struct_array *cp)
> > {
> > 	int idx;
> > 
> > 	idx = sp->completed & 0x1;
> > 	barrier();  /* ensure compiler looks -once- at sp->completed. */
> > 	cp->c[idx]++;
> > 	srcu_barrier();  /* ensure compiler won't misorder critical section. */
> > 	return idx;
> > }
> > 
> > And similarly for srcu_read_unlock()?
> > 
> > I sure hope that there is a better way!!!  For one thing, you cannot pass
> > a pointer in to srcu_read_lock(), since __get_cpu_var's name mangling would
> > fail in that case...
> 
> No, that's not what we had in mind.

Another approach I looked at was statically allocating a struct
percpu_data, but initializing it seems to be problematic.

So here are the three approaches that seem to have some chance
of working:

1.	Your approach of dynamically selecting between the
	per_cpu_ptr() and per_cpu() APIs based on a flag
	within the structure.

2.	Creating a pair of SRCU APIs, reflecting the two
	underlying per-CPU APIs (one for staticly allocated
	per-CPU variables, the other for dynamically allocated
	per-CPU variables).

3.	A compile-time translation layer, making use of
	two different structure types and a bit of gcc
	type comparison.  The idea would be to create
	a srcu_struct_static and a srcu_struct_dynamic
	structure that contained a pointer to the corresponding
	per-CPU variable and an srcu_struct, and to have
	a set of macros that did a typeof comparison, selecting
	the appropriate underlying primitive from the set
	of two.

	This is essentially #2, but with some cpp/typeof
	magic to make it look to the user of SRCU that there
	is but one API.

The goal I believe we are trying to attain with SRCU include:

a.	Minimal read-side overhead.  This goal favors 2 and 3.
	(Yes, blocking is so expensive that the extra check is
	"in the noise" if we block on the read side -- but I
	expect uses where blocking can happen but is extremely
	rare.)

b.	Minimal API expansion.  This goal favors 1 and 3.

c.	Simple and straightforward use of well-understood and
	timeworn features of gcc.  This goal favors 1 and 2.

Based on this breakdown, we have a three-way tie.  I tend to pay less
much attention to (c), which would lead me to choose #2.

Thoughts?  Other important goals?  Better yet, other approaches?

						Thanx, Paul