public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3
@ 2011-12-03 18:34 Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 1/7] rcu: Don't check irq nesting from rcu idle entry/exit Paul E. McKenney
                   ` (6 more replies)
  0 siblings, 7 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet, darren,
	patches

Hello!

This patchset adds to the earlier sets:

	https://lkml.org/lkml/2011/11/2/363
	https://lkml.org/lkml/2011/11/15/302
	https://lkml.org/lkml/2011/11/28/588

This fourth set adds more infrastructure for Frederic's user-mode nohz
work, additional improvements to RCU_FAST_NO_HZ, and a few bug fixes.
The patches are as follows:

1,2.	More support for user-mode nohz (courtesy of Frederic Weisbecker).
3.	Ignore RCU callback batch limits if the CPU is otherwise idle.
	This allows a CPU to more quickly clear a backlog of callbacks,
	and thus more quickly enter dyntick-idle mode.
4.	Adaptive dyntick-entry approach that more efficiently handles
	the various possible grace-period begin/end scenarios.
5.	Remove a redundant variable declaration.
6.	Allow rcutorture to avoid attempting to offline unhotpluggable CPUs.
7.	Fix RCU-lockdep splats involving RCU read-side critical sections
	that partially overlap irq-disable code sections (courtesy of
	Yong Zhang).

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

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

							Thanx, Paul

------------------------------------------------------------------------

 b/drivers/base/cpu.c      |   12 ++++++++
 b/include/linux/cpu.h     |    1 
 b/kernel/rcutorture.c     |    4 +-
 b/kernel/rcutree.c        |   18 +++++--------
 b/kernel/rcutree.h        |    1 
 b/kernel/rcutree_plugin.h |   14 ++++++++++
 kernel/rcutree.c          |   12 ++++----
 kernel/rcutree_plugin.h   |   62 ++++++++++++++++++++++++++++++++++++----------
 8 files changed, 92 insertions(+), 32 deletions(-)


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH RFC tip/core/rcu 1/7] rcu: Don't check irq nesting from rcu idle entry/exit
  2011-12-03 18:34 [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3 Paul E. McKenney
@ 2011-12-03 18:34 ` Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 2/7] rcu: Irq nesting is always 0 on rcu_enter_idle_common Paul E. McKenney
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet, darren,
	patches, Frederic Weisbecker, Paul E. McKenney

From: Frederic Weisbecker <fweisbec@gmail.com>

Because tasks do not nest, rcu_idle_enter() and rcu_idle_exit() do
not need to check for nesting.  This commit therefore moves nesting
checks from rcu_idle_enter_common() to rcu_irq_exit() and from
rcu_idle_exit_common() to rcu_irq_enter().

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index bf085d7..860c02c 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -350,10 +350,6 @@ static int rcu_implicit_offline_qs(struct rcu_data *rdp)
  */
 static void rcu_idle_enter_common(struct rcu_dynticks *rdtp, long long oldval)
 {
-	if (rdtp->dynticks_nesting) {
-		trace_rcu_dyntick("--=", oldval, rdtp->dynticks_nesting);
-		return;
-	}
 	trace_rcu_dyntick("Start", oldval, rdtp->dynticks_nesting);
 	if (!is_idle_task(current)) {
 		struct task_struct *idle = idle_task(smp_processor_id());
@@ -426,7 +422,10 @@ void rcu_irq_exit(void)
 	oldval = rdtp->dynticks_nesting;
 	rdtp->dynticks_nesting--;
 	WARN_ON_ONCE(rdtp->dynticks_nesting < 0);
-	rcu_idle_enter_common(rdtp, oldval);
+	if (rdtp->dynticks_nesting)
+		trace_rcu_dyntick("--=", oldval, rdtp->dynticks_nesting);
+	else
+		rcu_idle_enter_common(rdtp, oldval);
 	local_irq_restore(flags);
 }
 
@@ -439,10 +438,6 @@ void rcu_irq_exit(void)
  */
 static void rcu_idle_exit_common(struct rcu_dynticks *rdtp, long long oldval)
 {
-	if (oldval) {
-		trace_rcu_dyntick("++=", oldval, rdtp->dynticks_nesting);
-		return;
-	}
 	smp_mb__before_atomic_inc();  /* Force ordering w/previous sojourn. */
 	atomic_inc(&rdtp->dynticks);
 	/* CPUs seeing atomic_inc() must see later RCU read-side crit sects */
@@ -518,7 +513,10 @@ void rcu_irq_enter(void)
 	oldval = rdtp->dynticks_nesting;
 	rdtp->dynticks_nesting++;
 	WARN_ON_ONCE(rdtp->dynticks_nesting == 0);
-	rcu_idle_exit_common(rdtp, oldval);
+	if (oldval)
+		trace_rcu_dyntick("++=", oldval, rdtp->dynticks_nesting);
+	else
+		rcu_idle_exit_common(rdtp, oldval);
 	local_irq_restore(flags);
 }
 
-- 
1.7.3.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH RFC tip/core/rcu 2/7] rcu: Irq nesting is always 0 on rcu_enter_idle_common
  2011-12-03 18:34 [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3 Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 1/7] rcu: Don't check irq nesting from rcu idle entry/exit Paul E. McKenney
@ 2011-12-03 18:34 ` Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 3/7] rcu: Keep invoking callbacks if CPU otherwise idle Paul E. McKenney
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet, darren,
	patches, Frederic Weisbecker, Paul E. McKenney

From: Frederic Weisbecker <fweisbec@gmail.com>

Because tasks don't nest, the ->dyntick_nesting must always be zero upon
entry to rcu_idle_enter_common().  Therefore, pass "0" rather than the
counter itself.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 860c02c..c0ed376 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -350,12 +350,11 @@ static int rcu_implicit_offline_qs(struct rcu_data *rdp)
  */
 static void rcu_idle_enter_common(struct rcu_dynticks *rdtp, long long oldval)
 {
-	trace_rcu_dyntick("Start", oldval, rdtp->dynticks_nesting);
+	trace_rcu_dyntick("Start", oldval, 0);
 	if (!is_idle_task(current)) {
 		struct task_struct *idle = idle_task(smp_processor_id());
 
-		trace_rcu_dyntick("Error on entry: not idle task",
-				   oldval, rdtp->dynticks_nesting);
+		trace_rcu_dyntick("Error on entry: not idle task", oldval, 0);
 		ftrace_dump(DUMP_ALL);
 		WARN_ONCE(1, "Current pid: %d comm: %s / Idle pid: %d comm: %s",
 			  current->pid, current->comm,
-- 
1.7.3.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH RFC tip/core/rcu 3/7] rcu: Keep invoking callbacks if CPU otherwise idle
  2011-12-03 18:34 [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3 Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 1/7] rcu: Don't check irq nesting from rcu idle entry/exit Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 2/7] rcu: Irq nesting is always 0 on rcu_enter_idle_common Paul E. McKenney
@ 2011-12-03 18:34 ` Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 4/7] rcu: Adaptive dyntick-idle preparation Paul E. McKenney
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet, darren,
	patches, Paul E. McKenney, Paul E. McKenney

From: Paul E. McKenney <paul.mckenney@linaro.org>

The rcu_do_batch() function that invokes callbacks for TREE_RCU and
TREE_PREEMPT_RCU normally throttles callback invocation to avoid degrading
scheduling latency.  However, as long as the CPU would otherwise be idle,
there is no downside to continuing to invoke any callbacks that have passed
through their grace periods.  In fact, processing such callbacks in a
timely manner has the benefit of increasing the probability that the
CPU can enter the power-saving dyntick-idle mode.

Therefore, this commit allows callback invocation to continue beyond the
preset limit as long as the scheduler does not have some other task to
run and as long as context is that of the idle task or the relevant
RCU kthread.

Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c        |    5 ++++-
 kernel/rcutree.h        |    1 +
 kernel/rcutree_plugin.h |   14 ++++++++++++++
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index c0ed376..4ec4b14 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1403,7 +1403,10 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp)
 		debug_rcu_head_unqueue(list);
 		__rcu_reclaim(rsp->name, list);
 		list = next;
-		if (++count >= bl)
+		/* Stop only if limit reached and CPU has something to do. */
+		if (++count >= bl &&
+		    (need_resched() ||
+		     (!is_idle_task(current) && !rcu_is_callbacks_kthread())))
 			break;
 	}
 
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index 9bcfbc9..fddff92 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -455,6 +455,7 @@ static void __init __rcu_init_preempt(void);
 static void rcu_initiate_boost(struct rcu_node *rnp, unsigned long flags);
 static void rcu_preempt_boost_start_gp(struct rcu_node *rnp);
 static void invoke_rcu_callbacks_kthread(void);
+static bool rcu_is_callbacks_kthread(void);
 #ifdef CONFIG_RCU_BOOST
 static void rcu_preempt_do_callbacks(void);
 static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp,
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index dbcea6b..adb6e66 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -1337,6 +1337,15 @@ static void invoke_rcu_callbacks_kthread(void)
 }
 
 /*
+ * Is the current CPU running the RCU-callbacks kthread?
+ * Caller must have preemption disabled.
+ */
+static bool rcu_is_callbacks_kthread(void)
+{
+	return __get_cpu_var(rcu_cpu_kthread_task) == current;
+}
+
+/*
  * Set the affinity of the boost kthread.  The CPU-hotplug locks are
  * held, so no one should be messing with the existence of the boost
  * kthread.
@@ -1780,6 +1789,11 @@ static void invoke_rcu_callbacks_kthread(void)
 	WARN_ON_ONCE(1);
 }
 
+static bool rcu_is_callbacks_kthread(void)
+{
+	return false;
+}
+
 static void rcu_preempt_boost_start_gp(struct rcu_node *rnp)
 {
 }
-- 
1.7.3.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH RFC tip/core/rcu 4/7] rcu: Adaptive dyntick-idle preparation
  2011-12-03 18:34 [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3 Paul E. McKenney
                   ` (2 preceding siblings ...)
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 3/7] rcu: Keep invoking callbacks if CPU otherwise idle Paul E. McKenney
@ 2011-12-03 18:34 ` Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 5/7] rcu: remove redundant rcu_cpu_stall_suppress declaration Paul E. McKenney
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet, darren,
	patches, Paul E. McKenney, Paul E. McKenney

From: Paul E. McKenney <paul.mckenney@linaro.org>

If there are other CPUs active at a given point in time, then there is a
limit to what a given CPU can do to advance the current RCU grace period.
Beyond this limit, attempting to force the RCU grace period forward will
do nothing but consume energy burning CPU cycles.

Therefore, this commit takes an adaptive approach to RCU_FAST_NO_HZ
preparations for idle.  It pushes the RCU core state machine for
two cycles unconditionally, and then it will push from zero to three
additional cycles, but only as long as the RCU core has work for this
CPU to do immediately.  The rcu_pending() function is used to check
whether the RCU core has such work.

Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree_plugin.h |   54 +++++++++++++++++++++++++++++++++++++---------
 1 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index adb6e66..8cd9efe 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -1994,8 +1994,40 @@ static void rcu_prepare_for_idle(int cpu)
 
 #else /* #if !defined(CONFIG_RCU_FAST_NO_HZ) */
 
-#define RCU_NEEDS_CPU_FLUSHES 5		/* Allow for callback self-repost. */
+/*
+ * This code is invoked when a CPU goes idle, at which point we want
+ * to have the CPU do everything required for RCU so that it can enter
+ * the energy-efficient dyntick-idle mode.  This is handled by a
+ * state machine implemented by rcu_prepare_for_idle() below.
+ *
+ * The following three proprocessor symbols control this state machine:
+ *
+ * RCU_IDLE_FLUSHES gives the maximum number of times that we will attempt
+ *	to satisfy RCU.  Beyond this point, it is better to incur a periodic
+ *	scheduling-clock interrupt than to loop through the state machine
+ *	at full power.
+ * RCU_IDLE_OPT_FLUSHES gives the number of RCU_IDLE_FLUSHES that are
+ *	optional if RCU does not need anything immediately from this
+ *	CPU, even if this CPU still has RCU callbacks queued.  The first
+ *	times through the state machine are mandatory: we need to give
+ *	the state machine a chance to communicate a quiescent state
+ *	to the RCU core.
+ * RCU_IDLE_GP_DELAY gives the number of jiffies that a CPU is permitted
+ *	to sleep in dyntick-idle mode with RCU callbacks pending.  This
+ *	is sized to be roughly one RCU grace period.  Those energy-efficiency
+ *	benchmarkers who might otherwise be tempted to set this to a large
+ *	number, be warned: Setting RCU_IDLE_GP_DELAY too high can hang your
+ *	system.  And if you are -that- concerned about energy efficiency,
+ *	just power the system down and be done with it!
+ *
+ * The values below work well in practice.  If future workloads require
+ * adjustment, they can be converted into kernel config parameters, though
+ * making the state machine smarter might be a better option.
+ */
+#define RCU_IDLE_FLUSHES 5		/* Number of dyntick-idle tries. */
+#define RCU_IDLE_OPT_FLUSHES 3		/* Optional dyntick-idle tries. */
 #define RCU_IDLE_GP_DELAY 6		/* Roughly one grace period. */
+
 static DEFINE_PER_CPU(int, rcu_dyntick_drain);
 static DEFINE_PER_CPU(unsigned long, rcu_dyntick_holdoff);
 static DEFINE_PER_CPU(struct hrtimer, rcu_idle_gp_timer);
@@ -2110,17 +2142,17 @@ static void rcu_prepare_for_idle(int cpu)
 	/* Check and update the rcu_dyntick_drain sequencing. */
 	if (per_cpu(rcu_dyntick_drain, cpu) <= 0) {
 		/* First time through, initialize the counter. */
-		per_cpu(rcu_dyntick_drain, cpu) = RCU_NEEDS_CPU_FLUSHES;
-	} else if (--per_cpu(rcu_dyntick_drain, cpu) <= 0) {
+		per_cpu(rcu_dyntick_drain, cpu) = RCU_IDLE_FLUSHES;
+	} else if (per_cpu(rcu_dyntick_drain, cpu) <= RCU_IDLE_OPT_FLUSHES &&
+		   !rcu_pending(cpu)) {
 		/* Can we go dyntick-idle despite still having callbacks? */
-		if (!rcu_pending(cpu)) {
-			trace_rcu_prep_idle("Dyntick with callbacks");
-			per_cpu(rcu_dyntick_holdoff, cpu) = jiffies - 1;
-			hrtimer_start(&per_cpu(rcu_idle_gp_timer, cpu),
-				      rcu_idle_gp_wait, HRTIMER_MODE_REL);
-			return; /* Nothing more to do immediately. */
-		}
-
+		trace_rcu_prep_idle("Dyntick with callbacks");
+		per_cpu(rcu_dyntick_drain, cpu) = 0;
+		per_cpu(rcu_dyntick_holdoff, cpu) = jiffies - 1;
+		hrtimer_start(&per_cpu(rcu_idle_gp_timer, cpu),
+			      rcu_idle_gp_wait, HRTIMER_MODE_REL);
+		return; /* Nothing more to do immediately. */
+	} else if (--per_cpu(rcu_dyntick_drain, cpu) <= 0) {
 		/* We have hit the limit, so time to give up. */
 		per_cpu(rcu_dyntick_holdoff, cpu) = jiffies;
 		local_irq_restore(flags);
-- 
1.7.3.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH RFC tip/core/rcu 5/7] rcu: remove redundant rcu_cpu_stall_suppress declaration
  2011-12-03 18:34 [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3 Paul E. McKenney
                   ` (3 preceding siblings ...)
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 4/7] rcu: Adaptive dyntick-idle preparation Paul E. McKenney
@ 2011-12-03 18:34 ` Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 6/7] driver-core/cpu: Add cpu_is_hotpluggable() for rcutorture error analysis Paul E. McKenney
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Paul E. McKenney
  6 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet, darren,
	patches, Paul E. McKenney

No point in having two identical rcu_cpu_stall_suppress declarations,
so remove the more obscure of the two.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 4ec4b14..2b2e1a9 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -642,8 +642,6 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp)
 
 #endif /* #ifdef CONFIG_SMP */
 
-int rcu_cpu_stall_suppress __read_mostly;
-
 static void record_gp_stall_check_time(struct rcu_state *rsp)
 {
 	rsp->gp_start = jiffies;
-- 
1.7.3.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH RFC tip/core/rcu 6/7] driver-core/cpu: Add cpu_is_hotpluggable() for rcutorture error analysis
  2011-12-03 18:34 [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3 Paul E. McKenney
                   ` (4 preceding siblings ...)
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 5/7] rcu: remove redundant rcu_cpu_stall_suppress declaration Paul E. McKenney
@ 2011-12-03 18:34 ` Paul E. McKenney
  2011-12-03 21:06   ` Josh Triplett
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Paul E. McKenney
  6 siblings, 1 reply; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet, darren,
	patches, Paul E. McKenney, Paul E. McKenney

From: Paul E. McKenney <paul.mckenney@linaro.org>

The rcutorture test now can automatically exercise CPU hotplug and
collect success statistics, which can be correlated with other rcutorture
activity.  This permits rcutorture to completely exercise RCU regardless
of what sort of userspace and filesystem layout is in use.  Unfortunately,
rcutorture is happy to attempt to offline CPUs that cannot be offlined,
for example, CPU 0 in both the x86 and ARM architectures.  Although this
allows rcutorture testing to proceed normally, it confounds attempts at
error analysis due to the resulting flood of spurious CPU-hotplug errors.

Therefore, this commit creates a cpu_is_hotpluggable() function that
allows rcutorture to avoid attempting to offline CPUs that are not
hotpluggable, which in turn allows rcutorture to avoid reporting spurious
CPU-hotplug errors.  Note that this function is EXPORT_SYMBOL_GPL()
to allow rcutorture to use it when compiled as a kernel module.
This commit also includes modifications to rcutorture to use this
new function.

Suggested-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 drivers/base/cpu.c  |   12 +++++++++++-
 include/linux/cpu.h |    1 +
 kernel/rcutorture.c |    4 ++--
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 251acea..17f9b58 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -23,6 +23,7 @@ struct sysdev_class cpu_sysdev_class = {
 EXPORT_SYMBOL(cpu_sysdev_class);
 
 static DEFINE_PER_CPU(struct sys_device *, cpu_sys_devices);
+static DEFINE_PER_CPU(bool, is_hotpluggable);
 
 #ifdef CONFIG_HOTPLUG_CPU
 static ssize_t show_online(struct sys_device *dev, struct sysdev_attribute *attr,
@@ -76,6 +77,7 @@ void unregister_cpu(struct cpu *cpu)
 
 	sysdev_unregister(&cpu->sysdev);
 	per_cpu(cpu_sys_devices, logical_cpu) = NULL;
+	per_cpu(is_hotpluggable, logical_cpu) = 0;
 	return;
 }
 
@@ -224,8 +226,10 @@ int __cpuinit register_cpu(struct cpu *cpu, int num)
 
 	error = sysdev_register(&cpu->sysdev);
 
-	if (!error && cpu->hotpluggable)
+	if (!error && cpu->hotpluggable) {
 		register_cpu_control(cpu);
+		per_cpu(is_hotpluggable, num) = 1;
+	}
 	if (!error)
 		per_cpu(cpu_sys_devices, num) = &cpu->sysdev;
 	if (!error)
@@ -238,6 +242,12 @@ int __cpuinit register_cpu(struct cpu *cpu, int num)
 	return error;
 }
 
+bool cpu_is_hotpluggable(int num)
+{
+	return per_cpu(is_hotpluggable, num);
+}
+EXPORT_SYMBOL_GPL(cpu_is_hotpluggable);
+
 struct sys_device *get_cpu_sysdev(unsigned cpu)
 {
 	if (cpu < nr_cpu_ids && cpu_possible(cpu))
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 6cb60fd..be140ef 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -26,6 +26,7 @@ struct cpu {
 };
 
 extern int register_cpu(struct cpu *cpu, int num);
+extern bool cpu_is_hotpluggable(int num);
 extern struct sys_device *get_cpu_sysdev(unsigned cpu);
 
 extern int cpu_add_sysdev_attr(struct sysdev_attribute *attr);
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 1e422ae..186ead9 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -1388,7 +1388,7 @@ rcu_torture_onoff(void *arg)
 	WARN_ON(maxcpu < 0);
 	while (!kthread_should_stop()) {
 		cpu = (rcu_random(&rand) >> 4) % (maxcpu + 1);
-		if (cpu_online(cpu)) {
+		if (cpu_online(cpu) && cpu_is_hotpluggable(cpu)) {
 			if (verbose)
 				printk(KERN_ALERT "%s" TORTURE_FLAG
 				       "rcu_torture_onoff task: offlining %d\n",
@@ -1402,7 +1402,7 @@ rcu_torture_onoff(void *arg)
 					       torture_type, cpu);
 				n_offline_successes++;
 			}
-		} else {
+		} else if (cpu_is_hotpluggable(cpu)) {
 			if (verbose)
 				printk(KERN_ALERT "%s" TORTURE_FLAG
 				       "rcu_torture_onoff task: onlining %d\n",
-- 
1.7.3.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-03 18:34 [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3 Paul E. McKenney
                   ` (5 preceding siblings ...)
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 6/7] driver-core/cpu: Add cpu_is_hotpluggable() for rcutorture error analysis Paul E. McKenney
@ 2011-12-03 18:34 ` Paul E. McKenney
  2011-12-05  9:19   ` Yong Zhang
  2011-12-05  9:41   ` Peter Zijlstra
  6 siblings, 2 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet, darren,
	patches, Yong Zhang, Paul E. McKenney

From: Yong Zhang <yong.zhang0@gmail.com>

RCU-lockdep will issue warnings given the following use pattern:

	rcu_read_lock();
	local_irq_disable();
	rcu_read_unlock();
	local_irq_enable();

However, this use pattern is legal except for the scheduler's runqueue
and priority-inheritance locks (and any other locks that the scheduler
might use during priority-inheritance operations).

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree_plugin.h |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 8cd9efe..2020e8a 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -401,8 +401,11 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
 
 #ifdef CONFIG_RCU_BOOST
 		/* Unboost if we were boosted. */
-		if (rbmp)
+		if (rbmp) {
+			local_irq_save(flags);
 			rt_mutex_unlock(rbmp);
+			local_irq_restore(flags);
+		}
 #endif /* #ifdef CONFIG_RCU_BOOST */
 
 		/*
@@ -1233,9 +1236,10 @@ static int rcu_boost(struct rcu_node *rnp)
 	lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class,
 				   "rcu_boost_mutex");
 	t->rcu_boost_mutex = &mtx;
-	raw_spin_unlock_irqrestore(&rnp->lock, flags);
+	raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */
 	rt_mutex_lock(&mtx);  /* Side effect: boosts task t's priority. */
 	rt_mutex_unlock(&mtx);  /* Keep lockdep happy. */
+	local_irq_restore(flags);
 
 	return rnp->exp_tasks != NULL || rnp->boost_tasks != NULL;
 }
-- 
1.7.3.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 6/7] driver-core/cpu: Add cpu_is_hotpluggable() for rcutorture error analysis
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 6/7] driver-core/cpu: Add cpu_is_hotpluggable() for rcutorture error analysis Paul E. McKenney
@ 2011-12-03 21:06   ` Josh Triplett
  2011-12-03 23:14     ` Paul E. McKenney
  0 siblings, 1 reply; 32+ messages in thread
From: Josh Triplett @ 2011-12-03 21:06 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches, Paul E. McKenney

On Sat, Dec 03, 2011 at 10:34:41AM -0800, Paul E. McKenney wrote:
> The rcutorture test now can automatically exercise CPU hotplug and
> collect success statistics, which can be correlated with other rcutorture
> activity.  This permits rcutorture to completely exercise RCU regardless
> of what sort of userspace and filesystem layout is in use.  Unfortunately,
> rcutorture is happy to attempt to offline CPUs that cannot be offlined,
> for example, CPU 0 in both the x86 and ARM architectures.  Although this
> allows rcutorture testing to proceed normally, it confounds attempts at
> error analysis due to the resulting flood of spurious CPU-hotplug errors.
> 
> Therefore, this commit creates a cpu_is_hotpluggable() function that
> allows rcutorture to avoid attempting to offline CPUs that are not
> hotpluggable, which in turn allows rcutorture to avoid reporting spurious
> CPU-hotplug errors.  Note that this function is EXPORT_SYMBOL_GPL()
> to allow rcutorture to use it when compiled as a kernel module.
> This commit also includes modifications to rcutorture to use this
> new function.

I'd suggest writing this as a two-patch series: add the API, then use
it.

I'd also suggest making the more general case for this API, beyond just
rcutorture.

> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -23,6 +23,7 @@ struct sysdev_class cpu_sysdev_class = {
>  EXPORT_SYMBOL(cpu_sysdev_class);
>  
>  static DEFINE_PER_CPU(struct sys_device *, cpu_sys_devices);
> +static DEFINE_PER_CPU(bool, is_hotpluggable);
[...]
> @@ -224,8 +226,10 @@ int __cpuinit register_cpu(struct cpu *cpu, int num)
>  
>  	error = sysdev_register(&cpu->sysdev);
>  
> -	if (!error && cpu->hotpluggable)
> +	if (!error && cpu->hotpluggable) {
>  		register_cpu_control(cpu);
> +		per_cpu(is_hotpluggable, num) = 1;
> +	}

This information already exists in the ->hotpluggable field of "struct
cpu".  I'd suggest an alternate approach (included as a patch below),
which avoids the need to add a new per_cpu property redundant with
->hotpluggable.

Note that you can get to the patch below via "git am --scissors", or
just by copy/pasting it.

---8<---
From: Josh Triplett <josh@joshtriplett.org>
Subject: [PATCH] driver-core/cpu: Expose hotpluggability to the rest of the kernel

When architectures register CPUs, they indicate whether the CPU allows
hotplugging; notably, x86 and ARM don't allow hotplugging CPU 0.
Userspace can easily query the hotpluggability of a CPU via sysfs;
however, the kernel has no convenient way of accessing that property in
an architecture-independent way.  While the kernel can simply try it and
see, some code needs to distinguish between "hotplug failed" and
"hotplug has no hope of working on this CPU"; for example, rcutorture's
CPU hotplug tests want to avoid drowning out real hotplug failures with
expected failures.

Expose this property via a new cpu_is_hotpluggable function, so that the
rest of the kernel can access it in an architecture-independent way.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 drivers/base/cpu.c  |    7 +++++++
 include/linux/cpu.h |    1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 251acea..3991502 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -247,6 +247,13 @@ struct sys_device *get_cpu_sysdev(unsigned cpu)
 }
 EXPORT_SYMBOL_GPL(get_cpu_sysdev);
 
+bool cpu_is_hotpluggable(unsigned cpu)
+{
+	struct sys_device *dev = get_cpu_sysdev(cpu);
+	return dev && container_of(dev, struct cpu, sysdev)->hotpluggable;
+}
+EXPORT_SYMBOL_GPL(cpu_is_hotpluggable);
+
 int __init cpu_dev_init(void)
 {
 	int err;
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 6cb60fd..305c263 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -27,6 +27,7 @@ struct cpu {
 
 extern int register_cpu(struct cpu *cpu, int num);
 extern struct sys_device *get_cpu_sysdev(unsigned cpu);
+extern bool cpu_is_hotpluggable(unsigned cpu);
 
 extern int cpu_add_sysdev_attr(struct sysdev_attribute *attr);
 extern void cpu_remove_sysdev_attr(struct sysdev_attribute *attr);
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 6/7] driver-core/cpu: Add cpu_is_hotpluggable() for rcutorture error analysis
  2011-12-03 21:06   ` Josh Triplett
@ 2011-12-03 23:14     ` Paul E. McKenney
  0 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-03 23:14 UTC (permalink / raw)
  To: Josh Triplett
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches, Paul E. McKenney

On Sat, Dec 03, 2011 at 01:06:50PM -0800, Josh Triplett wrote:
> On Sat, Dec 03, 2011 at 10:34:41AM -0800, Paul E. McKenney wrote:
> > The rcutorture test now can automatically exercise CPU hotplug and
> > collect success statistics, which can be correlated with other rcutorture
> > activity.  This permits rcutorture to completely exercise RCU regardless
> > of what sort of userspace and filesystem layout is in use.  Unfortunately,
> > rcutorture is happy to attempt to offline CPUs that cannot be offlined,
> > for example, CPU 0 in both the x86 and ARM architectures.  Although this
> > allows rcutorture testing to proceed normally, it confounds attempts at
> > error analysis due to the resulting flood of spurious CPU-hotplug errors.
> > 
> > Therefore, this commit creates a cpu_is_hotpluggable() function that
> > allows rcutorture to avoid attempting to offline CPUs that are not
> > hotpluggable, which in turn allows rcutorture to avoid reporting spurious
> > CPU-hotplug errors.  Note that this function is EXPORT_SYMBOL_GPL()
> > to allow rcutorture to use it when compiled as a kernel module.
> > This commit also includes modifications to rcutorture to use this
> > new function.
> 
> I'd suggest writing this as a two-patch series: add the API, then use
> it.
> 
> I'd also suggest making the more general case for this API, beyond just
> rcutorture.
> 
> > --- a/drivers/base/cpu.c
> > +++ b/drivers/base/cpu.c
> > @@ -23,6 +23,7 @@ struct sysdev_class cpu_sysdev_class = {
> >  EXPORT_SYMBOL(cpu_sysdev_class);
> >  
> >  static DEFINE_PER_CPU(struct sys_device *, cpu_sys_devices);
> > +static DEFINE_PER_CPU(bool, is_hotpluggable);
> [...]
> > @@ -224,8 +226,10 @@ int __cpuinit register_cpu(struct cpu *cpu, int num)
> >  
> >  	error = sysdev_register(&cpu->sysdev);
> >  
> > -	if (!error && cpu->hotpluggable)
> > +	if (!error && cpu->hotpluggable) {
> >  		register_cpu_control(cpu);
> > +		per_cpu(is_hotpluggable, num) = 1;
> > +	}
> 
> This information already exists in the ->hotpluggable field of "struct
> cpu".  I'd suggest an alternate approach (included as a patch below),
> which avoids the need to add a new per_cpu property redundant with
> ->hotpluggable.

I do like your patch better, as it is smaller, less intrusive, and
makes better use of existing facilities.  I have therefore replaced
mine with it.  I also created a separate patch containing the rcutorture
modifications from my original.

> Note that you can get to the patch below via "git am --scissors", or
> just by copy/pasting it.

Very cool -- wasn't aware of --scissors before.  ;-)

							Thanx, Paul

> ---8<---
> From: Josh Triplett <josh@joshtriplett.org>
> Subject: [PATCH] driver-core/cpu: Expose hotpluggability to the rest of the kernel
> 
> When architectures register CPUs, they indicate whether the CPU allows
> hotplugging; notably, x86 and ARM don't allow hotplugging CPU 0.
> Userspace can easily query the hotpluggability of a CPU via sysfs;
> however, the kernel has no convenient way of accessing that property in
> an architecture-independent way.  While the kernel can simply try it and
> see, some code needs to distinguish between "hotplug failed" and
> "hotplug has no hope of working on this CPU"; for example, rcutorture's
> CPU hotplug tests want to avoid drowning out real hotplug failures with
> expected failures.
> 
> Expose this property via a new cpu_is_hotpluggable function, so that the
> rest of the kernel can access it in an architecture-independent way.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
>  drivers/base/cpu.c  |    7 +++++++
>  include/linux/cpu.h |    1 +
>  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 251acea..3991502 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -247,6 +247,13 @@ struct sys_device *get_cpu_sysdev(unsigned cpu)
>  }
>  EXPORT_SYMBOL_GPL(get_cpu_sysdev);
> 
> +bool cpu_is_hotpluggable(unsigned cpu)
> +{
> +	struct sys_device *dev = get_cpu_sysdev(cpu);
> +	return dev && container_of(dev, struct cpu, sysdev)->hotpluggable;
> +}
> +EXPORT_SYMBOL_GPL(cpu_is_hotpluggable);
> +
>  int __init cpu_dev_init(void)
>  {
>  	int err;
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 6cb60fd..305c263 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -27,6 +27,7 @@ struct cpu {
> 
>  extern int register_cpu(struct cpu *cpu, int num);
>  extern struct sys_device *get_cpu_sysdev(unsigned cpu);
> +extern bool cpu_is_hotpluggable(unsigned cpu);
> 
>  extern int cpu_add_sysdev_attr(struct sysdev_attribute *attr);
>  extern void cpu_remove_sysdev_attr(struct sysdev_attribute *attr);
> -- 
> 1.7.7.3
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Paul E. McKenney
@ 2011-12-05  9:19   ` Yong Zhang
  2011-12-05 16:45     ` Paul E. McKenney
  2011-12-05  9:41   ` Peter Zijlstra
  1 sibling, 1 reply; 32+ messages in thread
From: Yong Zhang @ 2011-12-05  9:19 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches

On Sat, Dec 03, 2011 at 10:34:42AM -0800, Paul E. McKenney wrote:
> From: Yong Zhang <yong.zhang0@gmail.com>
> 
> RCU-lockdep will issue warnings given the following use pattern:
> 
> 	rcu_read_lock();
> 	local_irq_disable();
> 	rcu_read_unlock();
> 	local_irq_enable();
> 
> However, this use pattern is legal except for the scheduler's runqueue
> and priority-inheritance locks (and any other locks that the scheduler
> might use during priority-inheritance operations).
> 
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  kernel/rcutree_plugin.h |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> index 8cd9efe..2020e8a 100644
> --- a/kernel/rcutree_plugin.h
> +++ b/kernel/rcutree_plugin.h
> @@ -401,8 +401,11 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
>  
>  #ifdef CONFIG_RCU_BOOST
>  		/* Unboost if we were boosted. */
> -		if (rbmp)
> +		if (rbmp) {
> +			local_irq_save(flags);
>  			rt_mutex_unlock(rbmp);
> +			local_irq_restore(flags);
> +		}
>  #endif /* #ifdef CONFIG_RCU_BOOST */
>  
>  		/*
> @@ -1233,9 +1236,10 @@ static int rcu_boost(struct rcu_node *rnp)
>  	lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class,
>  				   "rcu_boost_mutex");
>  	t->rcu_boost_mutex = &mtx;
> -	raw_spin_unlock_irqrestore(&rnp->lock, flags);
> +	raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */
>  	rt_mutex_lock(&mtx);  /* Side effect: boosts task t's priority. */
>  	rt_mutex_unlock(&mtx);  /* Keep lockdep happy. */

We permit rt_mutex_unlock() to be call with irq disabled,
but rt_mutex_lock() is still not allowed. So this usage
is not legal now.

Sounds we should hold this patch on until a more suitable
way is found.

Thanks,
Yong

> +	local_irq_restore(flags);
>  
>  	return rnp->exp_tasks != NULL || rnp->boost_tasks != NULL;
>  }
> -- 
> 1.7.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Only stand for myself

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Paul E. McKenney
  2011-12-05  9:19   ` Yong Zhang
@ 2011-12-05  9:41   ` Peter Zijlstra
  2011-12-05 10:03     ` Yong Zhang
  1 sibling, 1 reply; 32+ messages in thread
From: Peter Zijlstra @ 2011-12-05  9:41 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, niv, tglx, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches, Yong Zhang

On Sat, 2011-12-03 at 10:34 -0800, Paul E. McKenney wrote:
> From: Yong Zhang <yong.zhang0@gmail.com>
> 
> RCU-lockdep will issue warnings given the following use pattern:
> 
> 	rcu_read_lock();
> 	local_irq_disable();
> 	rcu_read_unlock();
> 	local_irq_enable();
> 
> However, this use pattern is legal except for the scheduler's runqueue
> and priority-inheritance locks (and any other locks that the scheduler
> might use during priority-inheritance operations).

So what does this patch do? Make it not complain when you do the above?
How often does this pattern actually happen? Can't be that often
otherwise we'd have had more complaints, no?



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-05  9:41   ` Peter Zijlstra
@ 2011-12-05 10:03     ` Yong Zhang
  2011-12-05 16:48       ` Paul E. McKenney
  0 siblings, 1 reply; 32+ messages in thread
From: Yong Zhang @ 2011-12-05 10:03 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Paul E. McKenney, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Mon, Dec 05, 2011 at 10:41:36AM +0100, Peter Zijlstra wrote:
> On Sat, 2011-12-03 at 10:34 -0800, Paul E. McKenney wrote:
> > From: Yong Zhang <yong.zhang0@gmail.com>
> > 
> > RCU-lockdep will issue warnings given the following use pattern:
> > 
> > 	rcu_read_lock();
> > 	local_irq_disable();
> > 	rcu_read_unlock();
> > 	local_irq_enable();
> > 
> > However, this use pattern is legal except for the scheduler's runqueue
> > and priority-inheritance locks (and any other locks that the scheduler
> > might use during priority-inheritance operations).
> 
> So what does this patch do? Make it not complain when you do the above?

It suppose to not complain but it bring other complain :(

> How often does this pattern actually happen?

IIRC, we have just one which is cured by commit [a841796: signal: align
__lock_task_sighand() irq disabling and RCU]

> Can't be that often
> otherwise we'd have had more complaints, no?

Yeah,

So that also means we don't dedicated lock_class_key for mtx.wait_lock.

Thanks,
Yong

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-05  9:19   ` Yong Zhang
@ 2011-12-05 16:45     ` Paul E. McKenney
  2011-12-06  1:26       ` Yong Zhang
  0 siblings, 1 reply; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-05 16:45 UTC (permalink / raw)
  To: Yong Zhang
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches

On Mon, Dec 05, 2011 at 05:19:24PM +0800, Yong Zhang wrote:
> On Sat, Dec 03, 2011 at 10:34:42AM -0800, Paul E. McKenney wrote:
> > From: Yong Zhang <yong.zhang0@gmail.com>
> > 
> > RCU-lockdep will issue warnings given the following use pattern:
> > 
> > 	rcu_read_lock();
> > 	local_irq_disable();
> > 	rcu_read_unlock();
> > 	local_irq_enable();
> > 
> > However, this use pattern is legal except for the scheduler's runqueue
> > and priority-inheritance locks (and any other locks that the scheduler
> > might use during priority-inheritance operations).
> > 
> > Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> >  kernel/rcutree_plugin.h |    8 ++++++--
> >  1 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> > index 8cd9efe..2020e8a 100644
> > --- a/kernel/rcutree_plugin.h
> > +++ b/kernel/rcutree_plugin.h
> > @@ -401,8 +401,11 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
> >  
> >  #ifdef CONFIG_RCU_BOOST
> >  		/* Unboost if we were boosted. */
> > -		if (rbmp)
> > +		if (rbmp) {
> > +			local_irq_save(flags);
> >  			rt_mutex_unlock(rbmp);
> > +			local_irq_restore(flags);
> > +		}
> >  #endif /* #ifdef CONFIG_RCU_BOOST */
> >  
> >  		/*
> > @@ -1233,9 +1236,10 @@ static int rcu_boost(struct rcu_node *rnp)
> >  	lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class,
> >  				   "rcu_boost_mutex");
> >  	t->rcu_boost_mutex = &mtx;
> > -	raw_spin_unlock_irqrestore(&rnp->lock, flags);
> > +	raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */
> >  	rt_mutex_lock(&mtx);  /* Side effect: boosts task t's priority. */
> >  	rt_mutex_unlock(&mtx);  /* Keep lockdep happy. */
> 
> We permit rt_mutex_unlock() to be call with irq disabled,
> but rt_mutex_lock() is still not allowed. So this usage
> is not legal now.

Even after commit #5342e269b has been applied?  The purpose of that
commit was to allow rt_mutex_lock() to be called with irqs disabled.

So, what am I missing?

						Thanx, Paul

> Sounds we should hold this patch on until a more suitable
> way is found.
> 
> Thanks,
> Yong
> 
> > +	local_irq_restore(flags);
> >  
> >  	return rnp->exp_tasks != NULL || rnp->boost_tasks != NULL;
> >  }
> > -- 
> > 1.7.3.2
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> -- 
> Only stand for myself
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-05 10:03     ` Yong Zhang
@ 2011-12-05 16:48       ` Paul E. McKenney
  0 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-05 16:48 UTC (permalink / raw)
  To: Yong Zhang
  Cc: Peter Zijlstra, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Mon, Dec 05, 2011 at 06:03:46PM +0800, Yong Zhang wrote:
> On Mon, Dec 05, 2011 at 10:41:36AM +0100, Peter Zijlstra wrote:
> > On Sat, 2011-12-03 at 10:34 -0800, Paul E. McKenney wrote:
> > > From: Yong Zhang <yong.zhang0@gmail.com>
> > > 
> > > RCU-lockdep will issue warnings given the following use pattern:
> > > 
> > > 	rcu_read_lock();
> > > 	local_irq_disable();
> > > 	rcu_read_unlock();
> > > 	local_irq_enable();
> > > 
> > > However, this use pattern is legal except for the scheduler's runqueue
> > > and priority-inheritance locks (and any other locks that the scheduler
> > > might use during priority-inheritance operations).
> > 
> > So what does this patch do? Make it not complain when you do the above?
> 
> It suppose to not complain but it bring other complain :(

Again, even with commit #5342e269b applied?

> > How often does this pattern actually happen?
> 
> IIRC, we have just one which is cured by commit [a841796: signal: align
> __lock_task_sighand() irq disabling and RCU]
> 
> > Can't be that often
> > otherwise we'd have had more complaints, no?

Maybe, maybe not.  To see the complaint, you have to have RCU_BOOST=y.
This is used heavily in -rt, but I bet that there are config options that
don't see much use in -rt.

With this one, prevention is better than after-the-fact cure.

							Thanx, Paul

> Yeah,
> 
> So that also means we don't dedicated lock_class_key for mtx.wait_lock.
> 
> Thanks,
> Yong
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-05 16:45     ` Paul E. McKenney
@ 2011-12-06  1:26       ` Yong Zhang
  2011-12-06  2:12         ` Paul E. McKenney
  2011-12-06  9:52         ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Peter Zijlstra
  0 siblings, 2 replies; 32+ messages in thread
From: Yong Zhang @ 2011-12-06  1:26 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches

On Mon, Dec 05, 2011 at 08:45:05AM -0800, Paul E. McKenney wrote:
> On Mon, Dec 05, 2011 at 05:19:24PM +0800, Yong Zhang wrote:
> > On Sat, Dec 03, 2011 at 10:34:42AM -0800, Paul E. McKenney wrote:
> > > From: Yong Zhang <yong.zhang0@gmail.com>
> > > 
> > > RCU-lockdep will issue warnings given the following use pattern:
> > > 
> > > 	rcu_read_lock();
> > > 	local_irq_disable();
> > > 	rcu_read_unlock();
> > > 	local_irq_enable();
> > > 
> > > However, this use pattern is legal except for the scheduler's runqueue
> > > and priority-inheritance locks (and any other locks that the scheduler
> > > might use during priority-inheritance operations).
> > > 
> > > Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > ---
> > >  kernel/rcutree_plugin.h |    8 ++++++--
> > >  1 files changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> > > index 8cd9efe..2020e8a 100644
> > > --- a/kernel/rcutree_plugin.h
> > > +++ b/kernel/rcutree_plugin.h
> > > @@ -401,8 +401,11 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
> > >  
> > >  #ifdef CONFIG_RCU_BOOST
> > >  		/* Unboost if we were boosted. */
> > > -		if (rbmp)
> > > +		if (rbmp) {
> > > +			local_irq_save(flags);
> > >  			rt_mutex_unlock(rbmp);
> > > +			local_irq_restore(flags);
> > > +		}
> > >  #endif /* #ifdef CONFIG_RCU_BOOST */
> > >  
> > >  		/*
> > > @@ -1233,9 +1236,10 @@ static int rcu_boost(struct rcu_node *rnp)
> > >  	lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class,
> > >  				   "rcu_boost_mutex");
> > >  	t->rcu_boost_mutex = &mtx;
> > > -	raw_spin_unlock_irqrestore(&rnp->lock, flags);
> > > +	raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */
> > >  	rt_mutex_lock(&mtx);  /* Side effect: boosts task t's priority. */
> > >  	rt_mutex_unlock(&mtx);  /* Keep lockdep happy. */
> > 
> > We permit rt_mutex_unlock() to be call with irq disabled,
> > but rt_mutex_lock() is still not allowed. So this usage
> > is not legal now.
> 
> Even after commit #5342e269b has been applied?

Yeah, because we call might_sleep() in rt_mutex_lock() unconditionally.
But in this case the 'BUG: sleeping function called from invalid context
at *' is obviously false positive.

Maybe we could teach might_sleep() about this special case?

Thanks,
Yong

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06  1:26       ` Yong Zhang
@ 2011-12-06  2:12         ` Paul E. McKenney
  2011-12-06  3:27           ` [PATCH 1/3] kernel.h: sched: introduce might_sleep_disabled() Yong Zhang
                             ` (2 more replies)
  2011-12-06  9:52         ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Peter Zijlstra
  1 sibling, 3 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-06  2:12 UTC (permalink / raw)
  To: Yong Zhang
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches

On Tue, Dec 06, 2011 at 09:26:35AM +0800, Yong Zhang wrote:
> On Mon, Dec 05, 2011 at 08:45:05AM -0800, Paul E. McKenney wrote:
> > On Mon, Dec 05, 2011 at 05:19:24PM +0800, Yong Zhang wrote:
> > > On Sat, Dec 03, 2011 at 10:34:42AM -0800, Paul E. McKenney wrote:
> > > > From: Yong Zhang <yong.zhang0@gmail.com>
> > > > 
> > > > RCU-lockdep will issue warnings given the following use pattern:
> > > > 
> > > > 	rcu_read_lock();
> > > > 	local_irq_disable();
> > > > 	rcu_read_unlock();
> > > > 	local_irq_enable();
> > > > 
> > > > However, this use pattern is legal except for the scheduler's runqueue
> > > > and priority-inheritance locks (and any other locks that the scheduler
> > > > might use during priority-inheritance operations).
> > > > 
> > > > Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > ---
> > > >  kernel/rcutree_plugin.h |    8 ++++++--
> > > >  1 files changed, 6 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> > > > index 8cd9efe..2020e8a 100644
> > > > --- a/kernel/rcutree_plugin.h
> > > > +++ b/kernel/rcutree_plugin.h
> > > > @@ -401,8 +401,11 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
> > > >  
> > > >  #ifdef CONFIG_RCU_BOOST
> > > >  		/* Unboost if we were boosted. */
> > > > -		if (rbmp)
> > > > +		if (rbmp) {
> > > > +			local_irq_save(flags);
> > > >  			rt_mutex_unlock(rbmp);
> > > > +			local_irq_restore(flags);
> > > > +		}
> > > >  #endif /* #ifdef CONFIG_RCU_BOOST */
> > > >  
> > > >  		/*
> > > > @@ -1233,9 +1236,10 @@ static int rcu_boost(struct rcu_node *rnp)
> > > >  	lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class,
> > > >  				   "rcu_boost_mutex");
> > > >  	t->rcu_boost_mutex = &mtx;
> > > > -	raw_spin_unlock_irqrestore(&rnp->lock, flags);
> > > > +	raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */
> > > >  	rt_mutex_lock(&mtx);  /* Side effect: boosts task t's priority. */
> > > >  	rt_mutex_unlock(&mtx);  /* Keep lockdep happy. */
> > > 
> > > We permit rt_mutex_unlock() to be call with irq disabled,
> > > but rt_mutex_lock() is still not allowed. So this usage
> > > is not legal now.
> > 
> > Even after commit #5342e269b has been applied?
> 
> Yeah, because we call might_sleep() in rt_mutex_lock() unconditionally.
> But in this case the 'BUG: sleeping function called from invalid context
> at *' is obviously false positive.
> 
> Maybe we could teach might_sleep() about this special case?

That sounds very good to me!

							Thanx, Paul


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/3] kernel.h: sched: introduce might_sleep_disabled()
  2011-12-06  2:12         ` Paul E. McKenney
@ 2011-12-06  3:27           ` Yong Zhang
  2011-12-06  3:28           ` [PATCH 2/3] rtmutex: introduce rt_mutex_lock_irqdisabled() Yong Zhang
  2011-12-06  3:29           ` [PATCH 3/3] rcu: use rt_mutex_lock_irqdisabled() in rcu_boost() Yong Zhang
  2 siblings, 0 replies; 32+ messages in thread
From: Yong Zhang @ 2011-12-06  3:27 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches

Since commit [5342e269: rcu: Permit rt_mutex_unlock() with
irqs disabled], rt_mutex_lock() could be called with irqs
disabled and it will call might_sleep() unconditionally.
To prevent this kind of false positive, introduce
might_sleep_disabled() which will be used in later patch.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 include/linux/kernel.h |   25 +++++++++++++++++++++----
 include/linux/sched.h  |   14 +++++++-------
 kernel/sched.c         |    6 ++++--
 3 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index e8b1597..deaba68 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -130,7 +130,8 @@ extern int _cond_resched(void);
 #endif
 
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
-  void __might_sleep(const char *file, int line, int preempt_offset);
+  void __might_sleep(const char *file, int line, int preempt_offset,
+					bool warn_irqs);
 /**
  * might_sleep - annotation for functions that can sleep
  *
@@ -141,12 +142,28 @@ extern int _cond_resched(void);
  * be bitten later when the calling function happens to sleep when it is not
  * supposed to.
  */
-# define might_sleep() \
-	do { __might_sleep(__FILE__, __LINE__, 0); might_resched(); } while (0)
+# define might_sleep()						\
+	do {							\
+		__might_sleep(__FILE__, __LINE__, 0, true);	\
+		might_resched(); }				\
+	while (0)
+
+/*
+ * Special version of might_sleep() which don't warn when called
+ * with irq disabled. Will be used in rt_mutex_lock() which
+ * could be called with irq disabled.
+ */
+# define might_sleep_irqdisabled()						\
+	do {							\
+		__might_sleep(__FILE__, __LINE__, 0, false);	\
+		might_resched(); }				\
+	while (0)
+
 #else
   static inline void __might_sleep(const char *file, int line,
-				   int preempt_offset) { }
+		int preempt_offset, bool warn_irqs) { }
 # define might_sleep() do { might_resched(); } while (0)
+# define might_sleep_irqdisabled() do { might_resched(); } while (0)
 #endif
 
 #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 4a7e4d3..d077c62 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2534,9 +2534,9 @@ static inline int need_resched(void)
  */
 extern int _cond_resched(void);
 
-#define cond_resched() ({			\
-	__might_sleep(__FILE__, __LINE__, 0);	\
-	_cond_resched();			\
+#define cond_resched() ({				\
+	__might_sleep(__FILE__, __LINE__, 0, true);	\
+	_cond_resched();				\
 })
 
 extern int __cond_resched_lock(spinlock_t *lock);
@@ -2547,15 +2547,15 @@ extern int __cond_resched_lock(spinlock_t *lock);
 #define PREEMPT_LOCK_OFFSET	0
 #endif
 
-#define cond_resched_lock(lock) ({				\
-	__might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET);	\
-	__cond_resched_lock(lock);				\
+#define cond_resched_lock(lock) ({					\
+	__might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET, true);	\
+	__cond_resched_lock(lock);					\
 })
 
 extern int __cond_resched_softirq(void);
 
 #define cond_resched_softirq() ({					\
-	__might_sleep(__FILE__, __LINE__, SOFTIRQ_DISABLE_OFFSET);	\
+	__might_sleep(__FILE__, __LINE__, SOFTIRQ_DISABLE_OFFSET, true);\
 	__cond_resched_softirq();					\
 })
 
diff --git a/kernel/sched.c b/kernel/sched.c
index 0e9344a..ce24450 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -8363,12 +8363,14 @@ static inline int preempt_count_equals(int preempt_offset)
 	return (nested == preempt_offset);
 }
 
-void __might_sleep(const char *file, int line, int preempt_offset)
+void __might_sleep(const char *file, int line, int preempt_offset,
+		   bool warn_irqs)
 {
 	static unsigned long prev_jiffy;	/* ratelimiting */
 
 	rcu_sleep_check(); /* WARN_ON_ONCE() by default, no rate limit reqd. */
-	if ((preempt_count_equals(preempt_offset) && !irqs_disabled()) ||
+	if ((preempt_count_equals(preempt_offset) &&
+	    (warn_irqs ? !irqs_disabled() : 1)) ||
 	    system_state != SYSTEM_RUNNING || oops_in_progress)
 		return;
 	if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy)
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/3] rtmutex: introduce rt_mutex_lock_irqdisabled()
  2011-12-06  2:12         ` Paul E. McKenney
  2011-12-06  3:27           ` [PATCH 1/3] kernel.h: sched: introduce might_sleep_disabled() Yong Zhang
@ 2011-12-06  3:28           ` Yong Zhang
  2011-12-06  3:29           ` [PATCH 3/3] rcu: use rt_mutex_lock_irqdisabled() in rcu_boost() Yong Zhang
  2 siblings, 0 replies; 32+ messages in thread
From: Yong Zhang @ 2011-12-06  3:28 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches

Since we permit rt_mutex_lock() to be called with irq disabled
in commit 5342e269, and we will call might_sleep() in
rt_mutex_lock() unconditionally, false positive will be
made. So introduce rt_mutex_lock_irqdisabled() for that kind
of usage.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 include/linux/rtmutex.h |    1 +
 kernel/rtmutex.c        |   13 +++++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index de17134..0732a25 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -88,6 +88,7 @@ extern void __rt_mutex_init(struct rt_mutex *lock, const char *name);
 extern void rt_mutex_destroy(struct rt_mutex *lock);
 
 extern void rt_mutex_lock(struct rt_mutex *lock);
+extern void rt_mutex_lock_irqdisabled(struct rt_mutex *lock);
 extern int rt_mutex_lock_interruptible(struct rt_mutex *lock,
 						int detect_deadlock);
 extern int rt_mutex_timed_lock(struct rt_mutex *lock,
diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
index f9d8482..961a100 100644
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -797,6 +797,19 @@ void __sched rt_mutex_lock(struct rt_mutex *lock)
 EXPORT_SYMBOL_GPL(rt_mutex_lock);
 
 /**
+ * rt_mutex_lock_irqdisabled - lock a rt_mutex with irq disabled
+ *
+ * @lock: the rt_mutex to be locked
+ */
+void __sched rt_mutex_lock_irqdisabled(struct rt_mutex *lock)
+{
+	might_sleep_irqdisabled();
+
+	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, 0, rt_mutex_slowlock);
+}
+EXPORT_SYMBOL_GPL(rt_mutex_lock_irqdisabled);
+
+/**
  * rt_mutex_lock_interruptible - lock a rt_mutex interruptible
  *
  * @lock: 		the rt_mutex to be locked
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/3] rcu: use rt_mutex_lock_irqdisabled() in rcu_boost()
  2011-12-06  2:12         ` Paul E. McKenney
  2011-12-06  3:27           ` [PATCH 1/3] kernel.h: sched: introduce might_sleep_disabled() Yong Zhang
  2011-12-06  3:28           ` [PATCH 2/3] rtmutex: introduce rt_mutex_lock_irqdisabled() Yong Zhang
@ 2011-12-06  3:29           ` Yong Zhang
  2 siblings, 0 replies; 32+ messages in thread
From: Yong Zhang @ 2011-12-06  3:29 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	eric.dumazet, darren, patches

Otherwise we will get false positive warning from might_sleep()

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 kernel/rcutree_plugin.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 2020e8a..489b1cd 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -1237,7 +1237,7 @@ static int rcu_boost(struct rcu_node *rnp)
 				   "rcu_boost_mutex");
 	t->rcu_boost_mutex = &mtx;
 	raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */
-	rt_mutex_lock(&mtx);  /* Side effect: boosts task t's priority. */
+	rt_mutex_lock_irqdisabled(&mtx);  /* Side effect: boosts task t's priority. */
 	rt_mutex_unlock(&mtx);  /* Keep lockdep happy. */
 	local_irq_restore(flags);
 
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06  1:26       ` Yong Zhang
  2011-12-06  2:12         ` Paul E. McKenney
@ 2011-12-06  9:52         ` Peter Zijlstra
  2011-12-06 10:05           ` Yong Zhang
                             ` (2 more replies)
  1 sibling, 3 replies; 32+ messages in thread
From: Peter Zijlstra @ 2011-12-06  9:52 UTC (permalink / raw)
  To: Yong Zhang
  Cc: Paul E. McKenney, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, 2011-12-06 at 09:26 +0800, Yong Zhang wrote:

> Yeah, because we call might_sleep() in rt_mutex_lock() unconditionally.
> But in this case the 'BUG: sleeping function called from invalid context
> at *' is obviously false positive.

Why can't this mutex acquisition not block?

> Maybe we could teach might_sleep() about this special case?

Sounds horrid.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06  9:52         ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Peter Zijlstra
@ 2011-12-06 10:05           ` Yong Zhang
  2011-12-06 10:32             ` Peter Zijlstra
  2011-12-06 10:27           ` Peter Zijlstra
  2011-12-06 16:01           ` Paul E. McKenney
  2 siblings, 1 reply; 32+ messages in thread
From: Yong Zhang @ 2011-12-06 10:05 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Paul E. McKenney, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, Dec 06, 2011 at 10:52:32AM +0100, Peter Zijlstra wrote:
> On Tue, 2011-12-06 at 09:26 +0800, Yong Zhang wrote:
> 
> > Yeah, because we call might_sleep() in rt_mutex_lock() unconditionally.
> > But in this case the 'BUG: sleeping function called from invalid context
> > at *' is obviously false positive.
> 
> Why can't this mutex acquisition not block?

It could block. The issue it's legal to call rt_mutex_lock() with
irqs disabled and we don't want might_sleep() bite us on this
special case. When we are going to sleep, we re-enable irq in
__rt_mutex_slowlock().

> 
> > Maybe we could teach might_sleep() about this special case?
> 
> Sounds horrid.

Maybe, any alternative?

Thanks,
Yong

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06  9:52         ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Peter Zijlstra
  2011-12-06 10:05           ` Yong Zhang
@ 2011-12-06 10:27           ` Peter Zijlstra
  2011-12-06 16:11             ` Paul E. McKenney
  2011-12-06 16:01           ` Paul E. McKenney
  2 siblings, 1 reply; 32+ messages in thread
From: Peter Zijlstra @ 2011-12-06 10:27 UTC (permalink / raw)
  To: Yong Zhang
  Cc: Paul E. McKenney, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, 2011-12-06 at 10:52 +0100, Peter Zijlstra wrote:
> On Tue, 2011-12-06 at 09:26 +0800, Yong Zhang wrote:
> 
> > Yeah, because we call might_sleep() in rt_mutex_lock() unconditionally.
> > But in this case the 'BUG: sleeping function called from invalid context
> > at *' is obviously false positive.
> 
> Why can't this mutex acquisition not block?

Gaah!! I see, this 5342e269 patch is revolting.. guys that's really vile
don't do that!

I tried reading the RCU code but I gave up.. rcu_boost() does:

  rt_mutex_init_proxy_locked();
  raw_spin_unlock_irqrestore();
  rt_mutex_lock();
  rt_mutex_unlock();

vs rcu_read_unlock_special()'s RCU_READ_UNLOCK_BLOCKED branch:

  rt_mutex_unlock();


The latter looks to be unbalanced because I can't actually find a
matching lock. Also, all of that is ran with IRQs enabled. So what's the
problem?



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06 10:05           ` Yong Zhang
@ 2011-12-06 10:32             ` Peter Zijlstra
  2011-12-06 12:26               ` Steven Rostedt
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Zijlstra @ 2011-12-06 10:32 UTC (permalink / raw)
  To: Yong Zhang
  Cc: Paul E. McKenney, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, 2011-12-06 at 18:05 +0800, Yong Zhang wrote:
> On Tue, Dec 06, 2011 at 10:52:32AM +0100, Peter Zijlstra wrote:
> > On Tue, 2011-12-06 at 09:26 +0800, Yong Zhang wrote:
> > 
> > > Yeah, because we call might_sleep() in rt_mutex_lock() unconditionally.
> > > But in this case the 'BUG: sleeping function called from invalid context
> > > at *' is obviously false positive.
> > 
> > Why can't this mutex acquisition not block?
> 
> It could block. The issue it's legal to call rt_mutex_lock() with
> irqs disabled and we don't want might_sleep() bite us on this

Of course its legal (nobody will arrest you for it), but its also bloody
horrid.

> special case. When we are going to sleep, we re-enable irq in
> __rt_mutex_slowlock().
> 
> > 
> > > Maybe we could teach might_sleep() about this special case?
> > 
> > Sounds horrid.
> 
> Maybe, any alternative?

Maybe someone explain this mess first?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06 10:32             ` Peter Zijlstra
@ 2011-12-06 12:26               ` Steven Rostedt
  2011-12-06 16:04                 ` Paul E. McKenney
  0 siblings, 1 reply; 32+ messages in thread
From: Steven Rostedt @ 2011-12-06 12:26 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Yong Zhang, Paul E. McKenney, linux-kernel, mingo, laijs,
	dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
	Valdis.Kletnieks, dhowells, eric.dumazet, darren, patches

On Tue, 2011-12-06 at 11:32 +0100, Peter Zijlstra wrote:

> > > 
> > > > Maybe we could teach might_sleep() about this special case?
> > > 
> > > Sounds horrid.
> > 
> > Maybe, any alternative?
> 
> Maybe someone explain this mess first?

Me too, because this looks like a hack that's just like a lie. The first
lie to be said gets you out of a bit of trouble, but then something else
happens based on that lie, in which you need to make another bigger lie.
This new lie affects more people and requires new more ingenious lies to
control the chaos. But eventually the lies required to keep everything
going become so overwhelming that it all blows up in your face and you
end up looking like a jackass.

Why is rcu using rt_mutex_lock() in strange ways? It's lying about its
use. And now this patch is the bigger lie to get around the issues of
the first lie. Eventually this code will continue to expand largely
based on these lies and will explode in our faces, and I feel sorry for
the poor jackass that needs to fix it.

Perhaps the real answer is that we need to create an API for priority
inheritance, that things like RCU could use. Attach a task that another
task requires to finish something and boost the priority of that task.
Maybe even completions could use such a thing?

-- Steve



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06  9:52         ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Peter Zijlstra
  2011-12-06 10:05           ` Yong Zhang
  2011-12-06 10:27           ` Peter Zijlstra
@ 2011-12-06 16:01           ` Paul E. McKenney
  2 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-06 16:01 UTC (permalink / raw)
  To: Peter Zijlstra, y
  Cc: Yong Zhang, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, Dec 06, 2011 at 10:52:32AM +0100, Peter Zijlstra wrote:
> On Tue, 2011-12-06 at 09:26 +0800, Yong Zhang wrote:
> 
> > Yeah, because we call might_sleep() in rt_mutex_lock() unconditionally.
> > But in this case the 'BUG: sleeping function called from invalid context
> > at *' is obviously false positive.
> 
> Why can't this mutex acquisition not block?

Because its purpose in life is to pass its priority on to the lock
holder.

							Thanx, Paul

> > Maybe we could teach might_sleep() about this special case?
> 
> Sounds horrid.
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06 12:26               ` Steven Rostedt
@ 2011-12-06 16:04                 ` Paul E. McKenney
  2011-12-06 16:33                   ` Paul E. McKenney
  2011-12-06 16:56                   ` Steven Rostedt
  0 siblings, 2 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-06 16:04 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Peter Zijlstra, Yong Zhang, linux-kernel, mingo, laijs, dipankar,
	akpm, mathieu.desnoyers, josh, niv, tglx, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, Dec 06, 2011 at 07:26:45AM -0500, Steven Rostedt wrote:
> On Tue, 2011-12-06 at 11:32 +0100, Peter Zijlstra wrote:
> 
> > > > 
> > > > > Maybe we could teach might_sleep() about this special case?
> > > > 
> > > > Sounds horrid.
> > > 
> > > Maybe, any alternative?
> > 
> > Maybe someone explain this mess first?
> 
> Me too, because this looks like a hack that's just like a lie. The first
> lie to be said gets you out of a bit of trouble, but then something else
> happens based on that lie, in which you need to make another bigger lie.
> This new lie affects more people and requires new more ingenious lies to
> control the chaos. But eventually the lies required to keep everything
> going become so overwhelming that it all blows up in your face and you
> end up looking like a jackass.
> 
> Why is rcu using rt_mutex_lock() in strange ways? It's lying about its
> use. And now this patch is the bigger lie to get around the issues of
> the first lie. Eventually this code will continue to expand largely
> based on these lies and will explode in our faces, and I feel sorry for
> the poor jackass that needs to fix it.
> 
> Perhaps the real answer is that we need to create an API for priority
> inheritance, that things like RCU could use. Attach a task that another
> task requires to finish something and boost the priority of that task.
> Maybe even completions could use such a thing?

I would be OK with that -- that was in fact the approach I was taking
when I was advised to use mutexes instead.  ;-)

							Thanx, Paul


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06 10:27           ` Peter Zijlstra
@ 2011-12-06 16:11             ` Paul E. McKenney
  2011-12-06 16:14               ` Peter Zijlstra
  0 siblings, 1 reply; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-06 16:11 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Yong Zhang, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, Dec 06, 2011 at 11:27:26AM +0100, Peter Zijlstra wrote:
> On Tue, 2011-12-06 at 10:52 +0100, Peter Zijlstra wrote:
> > On Tue, 2011-12-06 at 09:26 +0800, Yong Zhang wrote:
> > 
> > > Yeah, because we call might_sleep() in rt_mutex_lock() unconditionally.
> > > But in this case the 'BUG: sleeping function called from invalid context
> > > at *' is obviously false positive.
> > 
> > Why can't this mutex acquisition not block?
> 
> Gaah!! I see, this 5342e269 patch is revolting.. guys that's really vile
> don't do that!
> 
> I tried reading the RCU code but I gave up.. rcu_boost() does:
> 
>   rt_mutex_init_proxy_locked();
>   raw_spin_unlock_irqrestore();
>   rt_mutex_lock();
>   rt_mutex_unlock();
> 
> vs rcu_read_unlock_special()'s RCU_READ_UNLOCK_BLOCKED branch:
> 
>   rt_mutex_unlock();
> 
> 
> The latter looks to be unbalanced because I can't actually find a
> matching lock. Also, all of that is ran with IRQs enabled. So what's the
> problem?

The rt_mutex_init_proxy_locked() creates the lock in held state,
held by the RCU reader who is holding up the grace period.
So rcu_read_unlock_special()'s rt_mutex_unlock() is balanced by the
rt_mutex_init_proxy_locked().

The problem with the IRQs enabled is the following sequence:

	rcu_read_lock();
	/* do stuff */
	local_irq_save(flags);
	/* do more stuff */
	rcu_read_unlock();
	/* do even more stuff */
	local_irq_restore(flags);

This has been legal in the past, and might well be used in places that
-rt does not exercise, hence the desire to explicitly legalize it.

							Thanx, Paul


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06 16:11             ` Paul E. McKenney
@ 2011-12-06 16:14               ` Peter Zijlstra
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Zijlstra @ 2011-12-06 16:14 UTC (permalink / raw)
  To: paulmck
  Cc: Yong Zhang, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, niv, tglx, rostedt, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, 2011-12-06 at 08:11 -0800, Paul E. McKenney wrote:

> The problem with the IRQs enabled is the following sequence:
> 
> 	rcu_read_lock();
> 	/* do stuff */
> 	local_irq_save(flags);
> 	/* do more stuff */
> 	rcu_read_unlock();
> 	/* do even more stuff */
> 	local_irq_restore(flags);
> 
> This has been legal in the past, and might well be used in places that
> -rt does not exercise, hence the desire to explicitly legalize it.

So why not make it strictly dis-allowed, even for !-rt and see what
falls over? If there's lots of fallout we might need to reconsider, but
wouldn't it be easier to all abide by the strictest rules than to try
and frob stuff like was proposed?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06 16:04                 ` Paul E. McKenney
@ 2011-12-06 16:33                   ` Paul E. McKenney
  2011-12-06 16:56                   ` Steven Rostedt
  1 sibling, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-06 16:33 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Peter Zijlstra, Yong Zhang, linux-kernel, mingo, laijs, dipankar,
	akpm, mathieu.desnoyers, josh, niv, tglx, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, Dec 06, 2011 at 08:04:55AM -0800, Paul E. McKenney wrote:
> On Tue, Dec 06, 2011 at 07:26:45AM -0500, Steven Rostedt wrote:
> > On Tue, 2011-12-06 at 11:32 +0100, Peter Zijlstra wrote:
> > 
> > > > > 
> > > > > > Maybe we could teach might_sleep() about this special case?
> > > > > 
> > > > > Sounds horrid.
> > > > 
> > > > Maybe, any alternative?
> > > 
> > > Maybe someone explain this mess first?
> > 
> > Me too, because this looks like a hack that's just like a lie. The first
> > lie to be said gets you out of a bit of trouble, but then something else
> > happens based on that lie, in which you need to make another bigger lie.
> > This new lie affects more people and requires new more ingenious lies to
> > control the chaos. But eventually the lies required to keep everything
> > going become so overwhelming that it all blows up in your face and you
> > end up looking like a jackass.
> > 
> > Why is rcu using rt_mutex_lock() in strange ways? It's lying about its
> > use. And now this patch is the bigger lie to get around the issues of
> > the first lie. Eventually this code will continue to expand largely
> > based on these lies and will explode in our faces, and I feel sorry for
> > the poor jackass that needs to fix it.
> > 
> > Perhaps the real answer is that we need to create an API for priority
> > inheritance, that things like RCU could use. Attach a task that another
> > task requires to finish something and boost the priority of that task.
> > Maybe even completions could use such a thing?
> 
> I would be OK with that -- that was in fact the approach I was taking
> when I was advised to use mutexes instead.  ;-)

But in the spirit of full disclosure -- moving from explicit priority
boosting to mutexes simplified the code greatly.  It eliminated a bunch
of races between one task boosting and the boostee exiting its RCU
read-side critical section.  The difficulty is (or maybe was) the need
to block when boosting priority, which meant that there were races
where the booster marked the task, dropped the rcu_node ->lock,
the boostee exited its RCU read-side critical section and uselessly
deboosted itself, then the booster incorrectly did the boost.

But perhaps I was just suffering a failure of imagination.

							Thanx, Paul


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06 16:04                 ` Paul E. McKenney
  2011-12-06 16:33                   ` Paul E. McKenney
@ 2011-12-06 16:56                   ` Steven Rostedt
  2011-12-06 17:16                     ` Paul E. McKenney
  1 sibling, 1 reply; 32+ messages in thread
From: Steven Rostedt @ 2011-12-06 16:56 UTC (permalink / raw)
  To: paulmck
  Cc: Peter Zijlstra, Yong Zhang, linux-kernel, mingo, laijs, dipankar,
	akpm, mathieu.desnoyers, josh, niv, tglx, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, 2011-12-06 at 08:04 -0800, Paul E. McKenney wrote:

> > Perhaps the real answer is that we need to create an API for priority
> > inheritance, that things like RCU could use. Attach a task that another
> > task requires to finish something and boost the priority of that task.
> > Maybe even completions could use such a thing?
> 
> I would be OK with that -- that was in fact the approach I was taking
> when I was advised to use mutexes instead.  ;-)

Maybe we should rethink it. Using the makeshift mutex looks to be a
short term hack. But if we are starting to build on it, it will end up
being a horrible design, based off of a hack.

A mutex is to provide mutual exclusion. If we start bastardizing it to
do other things, it will become unmaintainable. I dare say that it's
close to unmaintainable now ;)

If we create a new API to handle inheritance, then perhaps it could be
used for other things like workqueues and completions (in -rt only).

-- Steve



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling
  2011-12-06 16:56                   ` Steven Rostedt
@ 2011-12-06 17:16                     ` Paul E. McKenney
  0 siblings, 0 replies; 32+ messages in thread
From: Paul E. McKenney @ 2011-12-06 17:16 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Peter Zijlstra, Yong Zhang, linux-kernel, mingo, laijs, dipankar,
	akpm, mathieu.desnoyers, josh, niv, tglx, Valdis.Kletnieks,
	dhowells, eric.dumazet, darren, patches

On Tue, Dec 06, 2011 at 11:56:42AM -0500, Steven Rostedt wrote:
> On Tue, 2011-12-06 at 08:04 -0800, Paul E. McKenney wrote:
> 
> > > Perhaps the real answer is that we need to create an API for priority
> > > inheritance, that things like RCU could use. Attach a task that another
> > > task requires to finish something and boost the priority of that task.
> > > Maybe even completions could use such a thing?
> > 
> > I would be OK with that -- that was in fact the approach I was taking
> > when I was advised to use mutexes instead.  ;-)
> 
> Maybe we should rethink it. Using the makeshift mutex looks to be a
> short term hack. But if we are starting to build on it, it will end up
> being a horrible design, based off of a hack.
> 
> A mutex is to provide mutual exclusion. If we start bastardizing it to
> do other things, it will become unmaintainable. I dare say that it's
> close to unmaintainable now ;)
> 
> If we create a new API to handle inheritance, then perhaps it could be
> used for other things like workqueues and completions (in -rt only).

Tough choice between yours and Peter's suggestion...

1.	Re-introduce ugly races by eliminating the mutex.

2.	Possibly have to deal with a new spate of lockdep-RCU splats.

Decisions, decisions!  ;-)

I suppose that one approach is to start with Peter's approach,
possibly adapting lockdep to explicitly check -- with an exception for
srcu_read_lock_raw(), of course.  If lockdep-RCU splats rain down too
hard, then perhaps the explicit priority inheritance would be one
potential umbrella.

Thoughts?

							Thanx, Paul


^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2011-12-06 17:18 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-03 18:34 [PATCH tip/core/rcu 0/7] Preview of fourth set of RCU changes for 3.3 Paul E. McKenney
2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 1/7] rcu: Don't check irq nesting from rcu idle entry/exit Paul E. McKenney
2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 2/7] rcu: Irq nesting is always 0 on rcu_enter_idle_common Paul E. McKenney
2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 3/7] rcu: Keep invoking callbacks if CPU otherwise idle Paul E. McKenney
2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 4/7] rcu: Adaptive dyntick-idle preparation Paul E. McKenney
2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 5/7] rcu: remove redundant rcu_cpu_stall_suppress declaration Paul E. McKenney
2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 6/7] driver-core/cpu: Add cpu_is_hotpluggable() for rcutorture error analysis Paul E. McKenney
2011-12-03 21:06   ` Josh Triplett
2011-12-03 23:14     ` Paul E. McKenney
2011-12-03 18:34 ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Paul E. McKenney
2011-12-05  9:19   ` Yong Zhang
2011-12-05 16:45     ` Paul E. McKenney
2011-12-06  1:26       ` Yong Zhang
2011-12-06  2:12         ` Paul E. McKenney
2011-12-06  3:27           ` [PATCH 1/3] kernel.h: sched: introduce might_sleep_disabled() Yong Zhang
2011-12-06  3:28           ` [PATCH 2/3] rtmutex: introduce rt_mutex_lock_irqdisabled() Yong Zhang
2011-12-06  3:29           ` [PATCH 3/3] rcu: use rt_mutex_lock_irqdisabled() in rcu_boost() Yong Zhang
2011-12-06  9:52         ` [PATCH RFC tip/core/rcu 7/7] rcu: Quiet RCU-lockdep warnings involving interrupt disabling Peter Zijlstra
2011-12-06 10:05           ` Yong Zhang
2011-12-06 10:32             ` Peter Zijlstra
2011-12-06 12:26               ` Steven Rostedt
2011-12-06 16:04                 ` Paul E. McKenney
2011-12-06 16:33                   ` Paul E. McKenney
2011-12-06 16:56                   ` Steven Rostedt
2011-12-06 17:16                     ` Paul E. McKenney
2011-12-06 10:27           ` Peter Zijlstra
2011-12-06 16:11             ` Paul E. McKenney
2011-12-06 16:14               ` Peter Zijlstra
2011-12-06 16:01           ` Paul E. McKenney
2011-12-05  9:41   ` Peter Zijlstra
2011-12-05 10:03     ` Yong Zhang
2011-12-05 16:48       ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox