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

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

On Thu, Jul 06, 2006 at 03:50:03PM -0700, Matt Helsley wrote:
> On Thu, 2006-07-06 at 16:28 -0400, Alan Stern wrote:
> > I've been trying to come up with a way to allow SRCU structures to be
> > initialized statically rather than dynamically.  The per-cpu data makes it
> > quite hard.  Not only do you have to use different routines to access
> > static vs. dynamic per-cpu data, there's just no good way to write a
> > static initializer.  This is because the per-cpu data requires its own
> > separate definition, and there's no way to call DEFINE_PER_CPU from within 
> > an initializer.
> > 
> > Here, in outline, is the best I've been able to come up with.  It uses a
> > function pointer member to select the appropriate sort of per-cpu data
> > access.  You would use it like this:
> > 
> > PREDEFINE_SRCU(s);
> > static DEFINE_SRCU(s);
> > ...
> > idx = srcu_read_lock(&s);
> > ... etc ...
> > 
> > Alternative possibilities involve an entire parallel implementation for
> > statically-initialized structures (which seems excessive) or using a
> > runtime test instead of a function pointer to select the dereferencing
> > mechanism.
> > 
> > Can anybody suggest anything better?
> > 
> > Alan Stern
> 
> I started to come up with something similar but did not get as far. I
> suspect the runtime test you're suggesting would look like:
> 
> #include <asm/sections.h>
> 
> ...
> if ((per_cpu_ptr >= __per_cpu_start) && (per_cpu_ptr < __per_cpu_end)) {
>     /* staticly-allocated per-cpu data */
>     ...
> } else {
>     /* dynamically-allocated per-cpu data */
>     ...
> }
> ...
> 
> I think that's easier to read and understand than following a function
> pointer.

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) \
	}

#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...

							Thanx, Paul