Hello!
Rough first cut of patch to provide the call_rcu_sched() needed for

Mathieu's markers implementation.  This is to synchronize_sched()

as call_rcu() is to synchronize_rcu().
Not for inclusion.  Passes light testing, but should be treated with

some caution given that no part of it is more than 24 hours old.

Known/suspected shortcomings:
o	Only lightly tested -- haven't yet tried rcutorture.
o	Need to add call_rcu_sched() testing to rcutorture.
o	If I remember correctly, an rcu_barrier_sched() is required

	(Mathieu?).
o	Interaction of this patch with CPU hotplug should be viewed

	with great suspicion.
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

---
 include/linux/rcuclassic.h |    3

 include/linux/rcupdate.h   |   22 +++

 include/linux/rcupreempt.h |   15 ++

 init/main.c                |    1

 kernel/rcupdate.c          |   20 ---

 kernel/rcupreempt.c        |  256 +++++++++++++++++++++++++++++++++++++++++++--

 6 files changed, 288 insertions(+), 29 deletions(-)
diff -urpNa -X dontdiff linux-2.6.25-rc6/include/linux/rcuclassic.h linux-2.6.25-rc6-call_rcu_sched/include/linux/rcuclassic.h

--- linux-2.6.25-rc6/include/linux/rcuclassic.h	2008-03-16 17:45:16.000000000 -0700

+++ linux-2.6.25-rc6-call_rcu_sched/include/linux/rcuclassic.h	2008-03-21 04:27:31.000000000 -0700

@@ -153,7 +153,10 @@ extern struct lockdep_map rcu_lock_map;
 #define __synchronize_sched() synchronize_rcu()
+#define call_rcu_sched(head, func) call_rcu(head, func)

+

 extern void __rcu_init(void);

+#define rcu_init_sched()	do { } while (0)

 extern void rcu_check_callbacks(int cpu, int user);

 extern void rcu_restart_cpu(int cpu);
diff -urpNa -X dontdiff linux-2.6.25-rc6/include/linux/rcupdate.h linux-2.6.25-rc6-call_rcu_sched/include/linux/rcupdate.h

--- linux-2.6.25-rc6/include/linux/rcupdate.h	2008-03-16 17:45:16.000000000 -0700

+++ linux-2.6.25-rc6-call_rcu_sched/include/linux/rcupdate.h	2008-03-20 21:10:42.000000000 -0700

@@ -42,6 +42,7 @@

 #include &lt...
[ message continues ]

Hello!
Second cut of patch to provide the call_rcu_sched() needed for Mathieu's

markers implementation.  This is again to synchronize_sched() as

call_rcu() is to synchronize_rcu().
Should be fine for experimental use, but not ready for inclusion.
Passes short rcutorture sessions, but should be treated with some caution

given that very little of it is more than 24 hours old.  Fixes since the

first version include a bug that could result in indefinite blocking

(spotted by Gautham Shenoy), better resiliency against CPU-hotplug

operations, and other minor fixes.
Known/suspected shortcomings:
o	Only moderately tested -- only short rcutorture sessions.
o	Need to add call_rcu_sched() testing to rcutorture.
o	If I remember correctly, an rcu_barrier_sched() is required

	(Mathieu?).
o	Interaction of this patch with CPU hotplug should be viewed

	with great suspicion.
o	If there are no synchronize_sched() calls for more than two

	minutes, one can see messages of the form "INFO: task

	rcu_sched_grace:924 blocked for more than 120 seconds."

	Any thoughts on how to avoid this message?  Should I be using

	something other than __wait_event() and wake_up(), which sleep

	uninterruptibly, thus triggering this message?
One other thing -- this patch also fixes a long-standing bug in the

earlier preemptable-RCU implementation of synchronize_rcu() that could

result in loss of concurrent external changes to a task's CPU affinity

mask.  I have lost track of who reported this...
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

---
 include/linux/rcuclassic.h |    3

 include/linux/rcupdate.h   |   22 +++

 include/linux/rcupreempt.h |   15 ++

 init/main.c                |    1

 kernel/rcupdate.c          |   20 ---

 kernel/rcupreempt.c        |  276 +++++++++++++++++++++++++++++++++++++++++++--

 6 files changed, 308 insertions(+), 29 deletions(-)
diff -urpNa -X dontdiff linux-2.6.25-rc6/include/linux/rcuclassic.h linux-2.6.25-rc6-C1-call_rcu_sched/include/linux...
[ message continues ]

Hello!
Third cut of patch to provide the call_rcu_sched().  This is again to

synchronize_sched() as call_rcu() is to synchronize_rcu().
Should be fine for experimental use, but not ready for inclusion.
Passes multi-hour rcutorture sessions with concurrent CPU hotplugging.
Fixes since the first version include a bug that could result in

indefinite blocking (spotted by Gautham Shenoy), better resiliency

against CPU-hotplug operations, and other minor fixes.
Fixes since the second version include reworking grace-period detection

to avoid deadlocks that could happen when running concurrently with

CPU hotplug, adding Mathieu's fix to avoid the softlockup messages,

as well as Mathieu's fix to allow use earlier in boot.
Known/suspected shortcomings:
o	Only moderately tested on x86-64 and POWER -- a few hours of

	rcutorture with concurrent CPU hotplugging.  In particular, I

	still do not trust the sleep/wakeup logic between call_rcu_sched()

	and rcu_sched_grace_period().
o	Need to add call_rcu_sched() testing to rcutorture.
o	Still needs rcu_barrier_sched() -- intending to incorporate

	the version Mathieu provided.
This patch also fixes a long-standing bug in the earlier preemptable-RCU

implementation of synchronize_rcu() that could result in loss of

concurrent external changes to a task's CPU affinity mask.  I still cannot

remember who reported this...
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

---
 include/linux/rcuclassic.h |    3

 include/linux/rcupdate.h   |   22 ++

 include/linux/rcupreempt.h |   42 ++++

 init/main.c                |    1

 kernel/rcupdate.c          |   20 --

 kernel/rcupreempt.c        |  401 ++++++++++++++++++++++++++++++++++++++++-----

 6 files changed, 427 insertions(+), 62 deletions(-)
diff -urpNa -X dontdiff linux-2.6.25-rc6/include/linux/rcuclassic.h linux-2.6.25-rc6-C1-call_rcu_sched/include/linux/rcuclassic.h

--- linux-2.6.25-rc6/inclu...
[ message continues ]

There are lots of creepy macros-which-probably-dont-need-to-be-macros in
That's a pretty big inline.  It only has a single callsite so the compiler

should inline it for us.  And if it grows a second callsite, the inlining
I assume that splitting the irq-disable from the spin_lock is a little
If wake_gp!=0 is common then we could microoptimise straight-line
If the above locking actually correct and needed?  The write to
It's more conventional to omit the blank line after the above form of
but, but, but.  The cpu at `cpu' could have gone offline just before we
I suspect I don't understand any of the RCU code any more.
--

K, removed the "inline".  Though it is not all that big if comments are
How about the following placed before the "if"?
	/*

	 * If this CPU took its interrupt from user mode or from the

	 * idle loop, and this is not a nested interrupt, then

	 * this CPU has to have exited all prior preept-disable

	 * sections of code.  So increment the counter to note this.

	 *

	 * The memory barrier is needed to handle the case where

	 * writes from a preempt-disable section of code gets reordered

	 * into schedule() by this CPU's write buffer.  So the memory

	 * barrier makes sure that the rcu_qsctr_inc() is seen by other

	 * CPUs to happen after any such write.

	 */
And I guess rcuclassic.c needs a similar comment in its version of

rcu_check_callbacks().  I will add this in a separate patch in this
As Gautham pointed out, we cannot allow the task to be preempted between

the time that we pick up rdp and the time that we acquire rdp->lock.
Hmmm...  Wait a minute... This should set "cpu"'s rdp->rcu_sched_sleeping

to 1, not some random CPU's.  Guess I should fix that, thank you!!!
The new code is as follows:
	rdp = RCU_DATA_CPU(cpu);

	spin_lock_irqsave(&rdp->lock, flags);

	rdp->rcu_sched_sleeping = 1;
If wake_gp!=0 is common, then that means that this section of code

is rarely invoked.  In which case I would guess that we should avoid
OK.  I am guessing the one after the "do".  The others are the ones
Yes, but the code sequence in call_rcu_sched() that is protected by

the same lock would be shocked and disappointed if that write showed
Also no point in its being a multi-line comment block.  Changed to

single-line comment block.  And deleted the blank lines following
Indeed.  I added the following to the comment:
			 * The current CPU might have already gone

			 * offline (between the for_each_offline_cpu and

			 * the spin_lock_irqsave), but in that case all its

			 * callback lists will be empty, so no harm done.
I added the following...
[ message continues ]

RCU_DATA_ME() eventually calls smp_processor_id(), which yells if we

call it from a irq-enabled or a preempt-enabled context. Hence the
In that case the CPU_DEAD callback should have migrated the rcu-lists to
--

Thanks and Regards

gautham

--

|

But local variable rdp might be pointing at the now-offlined CPU's data? -------/

--

Right. But then rdp wouldn't contain anything useful at this point.

So, we may only end up taking the rdp->lock, observe that there's nothing to do,

and move on.
Is there something else that I am missing?
--

Thanks and Regards

gautham

--

erm, I guess that'll work OK.
There were intentions to release the per-cpu memory during unplug, but

nobody has threatened to do that for a while.
--

That would be a bit painful in a number of situations...  :-/
							Thanx, Paul

--

Hi Paul,
Thanks for this work, I'll give it a try (I'm just back from a weekend

away from the city). Yes, my code needs a rcu_barrier_sched() so it can

wait for call_rcu_sched completion before it tries to re-use the data

structures at the next modification of the same marker.
I think rcu_barrier_sched should be quite straightforward to implement

if we derive it from kernel/rcupdate.c:rcu_barrier. Actually, couldn't

we just rename rcu_barrier into something else made static (_rcu_barrier)

and call it with a different parameter telling which of call_rcu or

call_rcu_sched to use ?
Something like this :
Add rcu_barrier_sched
Adds rcu_barrier_sched, which uses call_rcu_sched. It wait for each in flight

call_rcu_sched to be completed before it returns.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

---

 include/linux/rcupdate.h |    1 +

 kernel/rcupdate.c        |   40 +++++++++++++++++++++++++++++++++-------

 2 files changed, 34 insertions(+), 7 deletions(-)
Index: linux-2.6-lttng/include/linux/rcupdate.h

===================================================================

--- linux-2.6-lttng.orig/include/linux/rcupdate.h	2008-03-24 00:13:26.000000000 -0400

+++ linux-2.6-lttng/include/linux/rcupdate.h	2008-03-24 00:13:36.000000000 -0400

@@ -260,6 +260,7 @@ extern void call_rcu_bh(struct rcu_head

 /* Exported common interfaces */

 extern void synchronize_rcu(void);

 extern void rcu_barrier(void);

+extern void rcu_barrier_sched(void);

 extern long rcu_batches_completed(void);

 extern long rcu_batches_completed_bh(void);
Index: linux-2.6-lttng/kernel/rcupdate.c

===================================================================

--- linux-2.6-lttng.orig/kernel/rcupdate.c	2008-03-24 00:07:15.000000000 -0400

+++ linux-2.6-lttng/kernel/rcupdate.c	2008-03-24 00:17:01.000000000 -0400

@@ -45,6 +45,11 @@

 #include <linux/mutex.h>

 #include <linux/module.h>
+enum rcu_barrier {

+	RCU_BARRIER_STD,

+	RCU_BARRIER_SCHED,

+};

+

 static D...
[ message continues ]

I was thinking in terms of a cpp macro sort of like synchronize_rcu_xxx(),

but will look at this as well.  In any case, I agree with sharing the

mechanism between the two -- unless there is some screaming reason why

we need to be able to do an rcu_barrier() and an rcu_sched_barrier()
There are definitely some problems here...  Though I am seeing them
Don't we have to do something here to clear signal state if we are

ever to block again?  Maybe something like the following?
		flush_signals(current):
Fixing the bug or losing track?  ;-)
--

Initialize call_rcu_sched sooner so it can be used in module_init stage.
(needed for LTTng)
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

---

 init/main.c |    2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6-lttng/init/main.c

===================================================================

--- linux-2.6-lttng.orig/init/main.c	2008-03-25 08:49:02.000000000 -0400

+++ linux-2.6-lttng/init/main.c	2008-03-25 08:49:20.000000000 -0400

@@ -738,13 +738,13 @@ static void __init do_initcalls(void)

  */

 static void __init do_basic_setup(void)

 {

+	rcu_init_sched();

 	/* drivers will send hotplug events */

 	init_workqueues();

 	usermodehelper_init();

 	driver_init();

 	init_irq_proc();

 	do_initcalls();

-	rcu_init_sched();

 }
 static int __initdata nosoftlockup;

--

Mathieu Desnoyers

Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal

OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

--

This passes smoke tests, so have incorporated it.  Thank you, Mathieu!!!
--

Sorry for the misleading line : "Fix call_rcu_sched wait" was the title
Good point, I would add

if (ret < 0)

  flush_signals(current);
Fixing it of course :)
New version of the fix-call-rcu-sched-wait.patch file below.
Mathieu
Could you use __wait_event_interruptible and wake_up_interruptible

instead ? softlockup.c only seems to complain when uninterruptible tasks

are not scheduled for 2 minutes. I guess that when we receive a signal

we could simply go through another loop.
- Changelog

Reset signal state upon wakeup.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

---

 kernel/rcupreempt.c |   11 ++++++++---

 1 file changed, 8 insertions(+), 3 deletions(-)
Index: linux-2.6-lttng/kernel/rcupreempt.c

===================================================================

--- linux-2.6-lttng.orig/kernel/rcupreempt.c	2008-03-24 00:26:27.000000000 -0400

+++ linux-2.6-lttng/kernel/rcupreempt.c	2008-03-24 09:57:28.000000000 -0400

@@ -1074,7 +1074,7 @@ void call_rcu_sched(struct rcu_head *hea

 		rcu_ctrlblk.sched_sleep = rcu_sched_not_sleeping;

 		spin_unlock_irqrestore(&rcu_ctrlblk.schedlock, flags);

 		if (wake_gp)

-			wake_up(&rcu_ctrlblk.sched_wq);

+			wake_up_interruptible(&rcu_ctrlblk.sched_wq);

 	}

 }

 EXPORT_SYMBOL_GPL(call_rcu_sched);

@@ -1097,6 +1097,7 @@ rcu_sched_grace_period(void *arg)

 	int couldsleep;		/* might sleep after current pass. */

 	int couldsleepnext = 0; /* might sleep after next pass. */

 	int cpu;

+	int ret;

 	long err;

 	unsigned long flags;

 	int needsoftirq;

@@ -1242,8 +1243,12 @@ retry:
 		rcu_ctrlblk.sched_sleep = rcu_sched_sleeping;

 		spin_unlock_irqrestore(&rcu_ctrlblk.schedlock, flags);

-		__wait_event(rcu_ctrlblk.sched_wq,

-			     rcu_ctrlblk.sched_sleep != rcu_sched_sleeping);

+		ret = 0;

+		__wait_event_interruptible(rcu_ctrlblk.sched_wq,

+			     rcu_ctrlblk.sched_sleep != rcu_sched_sleeping,

+			     ret);

+		if (ret < 0)

+			flush_signals(current);

 		couldsleepnex...
[ message continues ]