signed overflow, virtualization optimizations, deadlock avoidance

PATCH New Paul E. McKenney 1 participant 4 messages May 8, 2011 RCU

lore

Share & tools

lore

Paul E. McKenney

author

2011-05-08 13:33 UTC #0 lore raw

cover signed overflow, virtualization optimizations, deadlock avoidance

Hello!

This patchset adds a few more commits to the list:

1. Rework check_cpu_stall to avoid signed overflow.
2. Provide an RCU API for better detection of running in a
guest OS (thanks to Gleb Natapov).
3. Permit rcu_read_unlock() to be called while holding one
of the runqueue locks in response to a lockdep splat, quite possibly from Valdis Kletnieks.

For a testing-only version of this patchset from git, please see the following subject-to-rebase branch:

git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-2.6-rcu.git rcu/testing

Thanx, Paul

 b/include/linux/rcutiny.h |    8 +++++
 b/include/linux/rcutree.h |   10 +++++++
 b/kernel/rcutree.c        |   13 +++++----
 b/kernel/rcutree.h        |    5 ---
 b/kernel/rcutree_plugin.h |   64 ++++++++++++++--------------------------------
 kernel/rcutree.c          |   45 ++++++++++----------------------
 6 files changed, 61 insertions(+), 84 deletions(-)

Paul E. McKenney

author

2011-05-08 13:34 UTC #1 lore raw

PATCH 1/3 rcu: get rid of signed overflow in check_cpu_stall()

Signed integer overflow is undefined by the C standard, so move calculations to unsigned.

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutree.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

kernel/rcutree.c +8 -5

index 78923a5..b2fe2a2 100644
@@ -581,21 +581,24 @@ static void print_cpu_stall(struct rcu_state *rsp)
 
 static void check_cpu_stall(struct rcu_state *rsp, struct rcu_data *rdp)
 {
-	long delta;
+	unsigned long j;
+	unsigned long js;
 	struct rcu_node *rnp;
 
 	if (rcu_cpu_stall_suppress)
 		return;
-	delta = jiffies - ACCESS_ONCE(rsp->jiffies_stall);
+	j = ACCESS_ONCE(jiffies);
+	js = ACCESS_ONCE(rsp->jiffies_stall);
 	rnp = rdp->mynode;
-	if ((ACCESS_ONCE(rnp->qsmask) & rdp->grpmask) && delta >= 0) {
+	if ((ACCESS_ONCE(rnp->qsmask) & rdp->grpmask) && ULONG_CMP_GE(j, js)) {
 
 		/* We haven't checked in, so go dump stack. */
 		print_cpu_stall(rsp);
 
-	} else if (rcu_gp_in_progress(rsp) && delta >= RCU_STALL_RAT_DELAY) {
+	} else if (rcu_gp_in_progress(rsp) &&
+		   ULONG_CMP_GE(j, js + RCU_STALL_RAT_DELAY)) {
 
-		/* They had two time units to dump stack, so complain. */
+		/* They had a few time units to dump stack, so complain. */
 		print_other_cpu_stall(rsp);
 	}
 }

Signature · 1 line

1.7.3.2

Paul E. McKenney

author

2011-05-08 13:34 UTC #2 lore raw

PATCH 2/3 rcu: provide rcu_virt_note_context_switch() function.

In reply to Paul E. McKenney

From: Gleb Natapov <[email protected]>

Provide rcu_virt_note_context_switch() for vitalization use to note quiescent state during guest entry.

Signed-off-by Gleb Natapov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 include/linux/rcutiny.h |    8 ++++++++
 include/linux/rcutree.h |   10 ++++++++++
 kernel/rcutree.c        |    1 +
 3 files changed, 19 insertions(+), 0 deletions(-)

include/linux/rcutiny.h +8 -0

index 30ebd7c..52b3e02 100644
@@ -100,6 +100,14 @@ static inline void rcu_note_context_switch(int cpu)
 }
 
 /*
+ * Take advantage of the fact that there is only one CPU, which
+ * allows us to ignore virtualization-based context switches.
+ */
+static inline void rcu_virt_note_context_switch(int cpu)
+{
+}
+
+/*
  * Return the number of grace periods.
  */
 static inline long rcu_batches_completed(void)

include/linux/rcutree.h +10 -0

index 284dad1..e65d066 100644
@@ -35,6 +35,16 @@ extern void rcu_note_context_switch(int cpu);
 extern int rcu_needs_cpu(int cpu);
 extern void rcu_cpu_stall_reset(void);
 
+/*
+ * Note a virtualization-based context switch.  This is simply a
+ * wrapper around rcu_note_context_switch(), which allows TINY_RCU
+ * to save a few bytes.
+ */
+static inline void rcu_virt_note_context_switch(int cpu)
+{
+	rcu_note_context_switch(cpu);
+}
+
 #ifdef CONFIG_TREE_PREEMPT_RCU
 
 extern void exit_rcu(void);

kernel/rcutree.c +1 -0

index b2fe2a2..54ff7eb 100644
@@ -157,6 +157,7 @@ void rcu_note_context_switch(int cpu)
 	rcu_sched_qs(cpu);
 	rcu_preempt_note_context_switch(cpu);
 }
+EXPORT_SYMBOL_GPL(rcu_note_context_switch);
 
 #ifdef CONFIG_NO_HZ
 DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {

Signature · 1 line

1.7.3.2

Paul E. McKenney

author

2011-05-08 13:34 UTC #3 lore raw

PATCH 3/3 rcu: permit rcu_read_unlock() to be called while holding runqueue locks

In reply to Paul E. McKenney

From: Paul E. McKenney <[email protected]>

Avoid calling into the scheduler while holding core RCU locks. This allows rcu_read_unlock() to be called while holding the runqueue locks, but only as long as there was no chance of the RCU read-side critical section having been preempted. (Otherwise, if RCU priority boosting is enabled, rcu_read_unlock() might call into the scheduler in order to unboost itself, which might allows self-deadlock on the runqueue locks within the scheduler.)

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutree.c        |   44 +++++++++----------------------
 kernel/rcutree.h        |    5 +---
 kernel/rcutree_plugin.h |   64 ++++++++++++++--------------------------------
 3 files changed, 34 insertions(+), 79 deletions(-)

kernel/rcutree.c +13 ~~-31~~

index 54ff7eb..5616b17 100644
@@ -1133,22 +1133,7 @@ static void __rcu_offline_cpu(int cpu, struct rcu_state *rsp)
 		raw_spin_unlock_irqrestore(&rnp->lock, flags);
 	if (need_report & RCU_OFL_TASKS_EXP_GP)
 		rcu_report_exp_rnp(rsp, rnp);
-
-	/*
-	 * If there are no more online CPUs for this rcu_node structure,
-	 * kill the rcu_node structure's kthread.  Otherwise, adjust its
-	 * affinity.
-	 */
-	t = rnp->node_kthread_task;
-	if (t != NULL &&
-	    rnp->qsmaskinit == 0) {
-		raw_spin_lock_irqsave(&rnp->lock, flags);
-		rnp->node_kthread_task = NULL;
-		raw_spin_unlock_irqrestore(&rnp->lock, flags);
-		kthread_stop(t);
-		rcu_stop_boost_kthread(rnp);
-	} else
-		rcu_node_kthread_setaffinity(rnp, -1);
+	rcu_node_kthread_setaffinity(rnp, -1);
 }
 
 /*

@@ -1320,8 +1305,7 @@ static void force_qs_rnp(struct rcu_state *rsp, int (*f)(struct rcu_data *))
 			return;
 		}
 		if (rnp->qsmask == 0) {
-			rcu_initiate_boost(rnp);
-			raw_spin_unlock_irqrestore(&rnp->lock, flags);
+			rcu_initiate_boost(rnp, flags); /* releases rnp->lock */
 			continue;
 		}
 		cpu = rnp->grplo;

@@ -1340,10 +1324,10 @@ static void force_qs_rnp(struct rcu_state *rsp, int (*f)(struct rcu_data *))
 		raw_spin_unlock_irqrestore(&rnp->lock, flags);
 	}
 	rnp = rcu_get_root(rsp);
-	raw_spin_lock_irqsave(&rnp->lock, flags);
-	if (rnp->qsmask == 0)
-		rcu_initiate_boost(rnp);
-	raw_spin_unlock_irqrestore(&rnp->lock, flags);
+	if (rnp->qsmask == 0) {
+		raw_spin_lock_irqsave(&rnp->lock, flags);
+		rcu_initiate_boost(rnp, flags); /* releases rnp->lock. */
+	}
 }
 
 /*

@@ -1497,7 +1481,8 @@ static void invoke_rcu_cpu_kthread(void)
 
 /*
  * Wake up the specified per-rcu_node-structure kthread.
- * The caller must hold ->lock.
+ * Because the per-rcu_node kthreads are immortal, we don't need
+ * to do anything to keep them alive.
  */
 static void invoke_rcu_node_kthread(struct rcu_node *rnp)
 {

@@ -1546,8 +1531,8 @@ static void rcu_cpu_kthread_timer(unsigned long arg)
 
 	raw_spin_lock_irqsave(&rnp->lock, flags);
 	rnp->wakemask |= rdp->grpmask;
-	invoke_rcu_node_kthread(rnp);
 	raw_spin_unlock_irqrestore(&rnp->lock, flags);
+	invoke_rcu_node_kthread(rnp);
 }
 
 /*

@@ -1694,16 +1679,12 @@ static int rcu_node_kthread(void *arg)
 
 	for (;;) {
 		rnp->node_kthread_status = RCU_KTHREAD_WAITING;
-		wait_event_interruptible(rnp->node_wq, rnp->wakemask != 0 ||
-						       kthread_should_stop());
-		if (kthread_should_stop())
-			break;
+		wait_event_interruptible(rnp->node_wq, rnp->wakemask != 0);
 		rnp->node_kthread_status = RCU_KTHREAD_RUNNING;
 		raw_spin_lock_irqsave(&rnp->lock, flags);
 		mask = rnp->wakemask;
 		rnp->wakemask = 0;
-		rcu_initiate_boost(rnp);
-		raw_spin_unlock_irqrestore(&rnp->lock, flags);
+		rcu_initiate_boost(rnp, flags); /* releases rnp->lock. */
 		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask >>= 1) {
 			if ((mask & 0x1) == 0)
 				continue;

@@ -1719,6 +1700,7 @@ static int rcu_node_kthread(void *arg)
 			preempt_enable();
 		}
 	}
+	/* NOTREACHED */
 	rnp->node_kthread_status = RCU_KTHREAD_STOPPED;
 	return 0;
 }

@@ -1738,7 +1720,7 @@ static void rcu_node_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
 	int cpu;
 	unsigned long mask = rnp->qsmaskinit;
 
-	if (rnp->node_kthread_task == NULL || mask == 0)
+	if (rnp->node_kthread_task == NULL)
 		return;
 	if (!alloc_cpumask_var(&cm, GFP_KERNEL))
 		return;

kernel/rcutree.h +1 -4

index a6a9717..93d4a1c 100644
@@ -444,15 +444,12 @@ static void rcu_preempt_send_cbs_to_online(void);
 static void __init __rcu_init_preempt(void);
 static void rcu_needs_cpu_flush(void);
 static void __init rcu_init_boost_waitqueue(struct rcu_node *rnp);
-static void rcu_initiate_boost(struct rcu_node *rnp);
+static void rcu_initiate_boost(struct rcu_node *rnp, unsigned long flags);
 static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp,
 					  cpumask_var_t cm);
 static void rcu_preempt_boost_start_gp(struct rcu_node *rnp);
 static int __cpuinit rcu_spawn_one_boost_kthread(struct rcu_state *rsp,
 						 struct rcu_node *rnp,
 						 int rnp_index);
-#ifdef CONFIG_HOTPLUG_CPU
-static void rcu_stop_boost_kthread(struct rcu_node *rnp);
-#endif /* #ifdef CONFIG_HOTPLUG_CPU */
 
 #endif /* #ifndef RCU_TREE_NONCORE */

kernel/rcutree_plugin.h +20 ~~-44~~

130 lines not on this page · raw message

Signature · 1 line

1.7.3.2