Skip to contentSkip to conversation

RCU fix

GIT PULL Ingo Molnar 3 participants 5 messages May 31, 2011

lore
Share & tools
lore

Ingo Molnar

author
#0 lore raw

Linus,

Please pull the latest core-urgent-for-linus git tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core-urgent-for-linus

Thanks,

Ingo

------------------>
Peter Zijlstra (1):
rcu: Cure load woes

 kernel/rcutree.c        |   54 ++++++++++++++++++++++++++++++++++++++++-------
 kernel/rcutree_plugin.h |   11 ++++++++-
 2 files changed, 56 insertions(+), 9 deletions(-)
kernel/rcutree.c +46 -8
index 77a7671..89419ff 100644
@@ -1648,7 +1648,6 @@ static int __cpuinit rcu_spawn_one_cpu_kthread(int cpu)
 	if (IS_ERR(t))
 		return PTR_ERR(t);
 	kthread_bind(t, cpu);
-	set_task_state(t, TASK_INTERRUPTIBLE);
 	per_cpu(rcu_cpu_kthread_cpu, cpu) = cpu;
 	WARN_ON_ONCE(per_cpu(rcu_cpu_kthread_task, cpu) != NULL);
 	per_cpu(rcu_cpu_kthread_task, cpu) = t;

@@ -1756,7 +1755,6 @@ static int __cpuinit rcu_spawn_one_node_kthread(struct rcu_state *rsp,
 		if (IS_ERR(t))
 			return PTR_ERR(t);
 		raw_spin_lock_irqsave(&rnp->lock, flags);
-		set_task_state(t, TASK_INTERRUPTIBLE);
 		rnp->node_kthread_task = t;
 		raw_spin_unlock_irqrestore(&rnp->lock, flags);
 		sp.sched_priority = 99;

@@ -1765,6 +1763,8 @@ static int __cpuinit rcu_spawn_one_node_kthread(struct rcu_state *rsp,
 	return rcu_spawn_one_boost_kthread(rsp, rnp, rnp_index);
 }
 
+static void rcu_wake_one_boost_kthread(struct rcu_node *rnp);
+
 /*
  * Spawn all kthreads -- called as soon as the scheduler is running.
  */

@@ -1772,18 +1772,30 @@ static int __init rcu_spawn_kthreads(void)
 {
 	int cpu;
 	struct rcu_node *rnp;
+	struct task_struct *t;
 
 	rcu_kthreads_spawnable = 1;
 	for_each_possible_cpu(cpu) {
 		per_cpu(rcu_cpu_has_work, cpu) = 0;
-		if (cpu_online(cpu))
+		if (cpu_online(cpu)) {
 			(void)rcu_spawn_one_cpu_kthread(cpu);
+			t = per_cpu(rcu_cpu_kthread_task, cpu);
+			if (t)
+				wake_up_process(t);
+		}
 	}
 	rnp = rcu_get_root(rcu_state);
 	(void)rcu_spawn_one_node_kthread(rcu_state, rnp);
+	if (rnp->node_kthread_task)
+		wake_up_process(rnp->node_kthread_task);
 	if (NUM_RCU_NODES > 1) {
-		rcu_for_each_leaf_node(rcu_state, rnp)
+		rcu_for_each_leaf_node(rcu_state, rnp) {
 			(void)rcu_spawn_one_node_kthread(rcu_state, rnp);
+			t = rnp->node_kthread_task;
+			if (t)
+				wake_up_process(t);
+			rcu_wake_one_boost_kthread(rnp);
+		}
 	}
 	return 0;
 }

@@ -2188,14 +2200,14 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp, int preemptible)
 	raw_spin_unlock_irqrestore(&rsp->onofflock, flags);
 }
 
-static void __cpuinit rcu_online_cpu(int cpu)
+static void __cpuinit rcu_prepare_cpu(int cpu)
 {
 	rcu_init_percpu_data(cpu, &rcu_sched_state, 0);
 	rcu_init_percpu_data(cpu, &rcu_bh_state, 0);
 	rcu_preempt_init_percpu_data(cpu);
 }
 
-static void __cpuinit rcu_online_kthreads(int cpu)
+static void __cpuinit rcu_prepare_kthreads(int cpu)
 {
 	struct rcu_data *rdp = per_cpu_ptr(rcu_state->rda, cpu);
 	struct rcu_node *rnp = rdp->mynode;

@@ -2209,6 +2221,31 @@ static void __cpuinit rcu_online_kthreads(int cpu)
 }
 
 /*
+ * kthread_create() creates threads in TASK_UNINTERRUPTIBLE state,
+ * but the RCU threads are woken on demand, and if demand is low this
+ * could be a while triggering the hung task watchdog.
+ *
+ * In order to avoid this, poke all tasks once the CPU is fully
+ * up and running.
+ */
+static void __cpuinit rcu_online_kthreads(int cpu)
+{
+	struct rcu_data *rdp = per_cpu_ptr(rcu_state->rda, cpu);
+	struct rcu_node *rnp = rdp->mynode;
+	struct task_struct *t;
+
+	t = per_cpu(rcu_cpu_kthread_task, cpu);
+	if (t)
+		wake_up_process(t);
+
+	t = rnp->node_kthread_task;
+	if (t)
+		wake_up_process(t);
+
+	rcu_wake_one_boost_kthread(rnp);
+}
+
+/*
  * Handle CPU online/offline notification events.
  */
 static int __cpuinit rcu_cpu_notify(struct notifier_block *self,

@@ -2221,10 +2258,11 @@ static int __cpuinit rcu_cpu_notify(struct notifier_block *self,
 	switch (action) {
 	case CPU_UP_PREPARE:
 	case CPU_UP_PREPARE_FROZEN:
-		rcu_online_cpu(cpu);
-		rcu_online_kthreads(cpu);
+		rcu_prepare_cpu(cpu);
+		rcu_prepare_kthreads(cpu);
 		break;
 	case CPU_ONLINE:
+		rcu_online_kthreads(cpu);
 	case CPU_DOWN_FAILED:
 		rcu_node_kthread_setaffinity(rnp, -1);
 		rcu_cpu_kthread_setrt(cpu, 1);
kernel/rcutree_plugin.h +10 -1
index a767b7d..c8bff30 100644
@@ -1295,7 +1295,6 @@ static int __cpuinit rcu_spawn_one_boost_kthread(struct rcu_state *rsp,
 	if (IS_ERR(t))
 		return PTR_ERR(t);
 	raw_spin_lock_irqsave(&rnp->lock, flags);
-	set_task_state(t, TASK_INTERRUPTIBLE);
 	rnp->boost_kthread_task = t;
 	raw_spin_unlock_irqrestore(&rnp->lock, flags);
 	sp.sched_priority = RCU_KTHREAD_PRIO;

@@ -1303,6 +1302,12 @@ static int __cpuinit rcu_spawn_one_boost_kthread(struct rcu_state *rsp,
 	return 0;
 }
 
+static void __cpuinit rcu_wake_one_boost_kthread(struct rcu_node *rnp)
+{
+	if (rnp->boost_kthread_task)
+		wake_up_process(rnp->boost_kthread_task);
+}
+
 #else /* #ifdef CONFIG_RCU_BOOST */
 
 static void rcu_initiate_boost(struct rcu_node *rnp, unsigned long flags)

@@ -1326,6 +1331,10 @@ static int __cpuinit rcu_spawn_one_boost_kthread(struct rcu_state *rsp,
 	return 0;
 }
 
+static void __cpuinit rcu_wake_one_boost_kthread(struct rcu_node *rnp)
+{
+}
+
 #endif /* #else #ifdef CONFIG_RCU_BOOST */
 
 #ifndef CONFIG_SMP
#1 lore raw
Quoted 4 lines · Ingo Molnar

Please pull the latest core-urgent-for-linus git tree from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core-urgent-for-linus

So remind me again why RCU switched to the stupid threads? It caused problems for rcutiny, it now causes silly problems for rcutree. Why do it? It's just extra complexity and no real advantage afaik.

Linus

#2 lore raw
Quoted 9 lines · Linus Torvalds
Quoted 4 lines · Ingo Molnar

Please pull the latest core-urgent-for-linus git tree from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core-urgent-for-linus

So remind me again why RCU switched to the stupid threads? It caused problems for rcutiny, it now causes silly problems for rcutree. Why do it? It's just extra complexity and no real advantage afaik.

The reason for the switch is to allow threads blocked in TREE_PREEMPT_RCU and TINY_PREEMPT_RCU RCU read-side critical sections to have their priority boosted in order to avoid OOM. People have made these OOMs happen, so this is not longer just a theoretical concern.

Thanx, Paul

#3 lore raw
Quoted 5 lines · Paul E. McKenney

The reason for the switch is to allow threads blocked in TREE_PREEMPT_RCU and TINY_PREEMPT_RCU RCU read-side critical sections to have their priority boosted in order to avoid OOM.  People have made these OOMs happen, so this is not longer just a theoretical concern.

Quite frankly, that doesn't make much sense.

First off, the default for priority boosting is off (and you cannot even select it unless you have RT_MUTEX and PREEMPT_RCU), so why the heck do we still use the threads even when we don't support the boosting at all?

Secondly, if a process is in danger of exhausting the RCU resources, and it is preemptable, why doesn't the rcu_read_unlock() logic just try to force a reschedule and thus an rcu idle period? Using processes and process priorities for this seems to be just stupid.

I dunno. After RCU_TINY showed how fragile it was to use kernel threads for this, and after this subtle issue just re-inforced that conclusion, I just cannot begin to believe that using a thread was the right thing to do. It just seems stupid.

Linus

#4 lore raw
Quoted 8 lines · Linus Torvalds
Quoted 5 lines · Paul E. McKenney

The reason for the switch is to allow threads blocked in TREE_PREEMPT_RCU and TINY_PREEMPT_RCU RCU read-side critical sections to have their priority boosted in order to avoid OOM.  People have made these OOMs happen, so this is not longer just a theoretical concern.

Quoted 6 lines

Quite frankly, that doesn't make much sense.

First off, the default for priority boosting is off (and you cannot even select it unless you have RT_MUTEX and PREEMPT_RCU), so why the heck do we still use the threads even when we don't support the boosting at all?

I considered using softirq in the !RCU_BOOST case, but that makes the code larger and just makes the failure cases we saw less likely. And some of the failure cases could be made to happen from userspace with real-time threads, not just from RCU priority boosting.

But I could of course switch to the dual softirq/kthread approach if needed.

Quoted 4 lines

Secondly, if a process is in danger of exhausting the RCU resources, and it is preemptable, why doesn't the rcu_read_unlock() logic just try to force a reschedule and thus an rcu idle period? Using processes and process priorities for this seems to be just stupid.

This approach does work (and is used) for TINY_RCU and TREE_RCU, but it unfortunately simply does not work for TINY_PREEMPT_RCU and TREE_PREEMPT_RCU. The reason for this is that for the preemptible variants of RCU, a reschedule in not guaranteed to be an RCU quiescent state. Which is why RCU_BOOST depends on PREEMPT_RCU (which is either TINY_PREEMPT_RCU or TREE_PREEMPT_RCU.

Quoted 4 lines

I dunno. After RCU_TINY showed how fragile it was to use kernel threads for this, and after this subtle issue just re-inforced that conclusion, I just cannot begin to believe that using a thread was the right thing to do. It just seems stupid.

Again, at least some of these were things that could be made to happen from userspace with the standard APIs, so those at least did need to be fixed.

Thanx, Paul