All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RT 0/4] Linux 3.10.70-rt75-rc2
@ 2015-03-17 16:35 Steven Rostedt
  2015-03-17 16:35 ` [PATCH RT 1/4] fs,btrfs: fix rt deadlock on extent_buffer->lock Steven Rostedt
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Steven Rostedt @ 2015-03-17 16:35 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Thomas Gleixner, Carsten Emde, Sebastian Andrzej Siewior,
	John Kacur, Paul Gortmaker


Dear RT Folks,

This is the RT stable review cycle of patch 3.10.70-rt75-rc2.

Please scream at me if I messed something up. Please test the patches too.

The -rc release will be uploaded to kernel.org and will be deleted when
the final release is out. This is just a review release (or release candidate).

The pre-releases will not be pushed to the git repository, only the
final release is.

If all goes well, this patch will be converted to the next main release
on 3/20/2015.

I'm only posting the patches that were added since rc1.

Enjoy,

-- Steve


To build 3.10.70-rt75-rc2 directly, the following patches should be applied:

  http://www.kernel.org/pub/linux/kernel/v3.x/linux-3.10.tar.xz

  http://www.kernel.org/pub/linux/kernel/v3.x/patch-3.10.70.xz

  http://www.kernel.org/pub/linux/kernel/projects/rt/3.10/patch-3.10.70-rt75-rc2.patch.xz

You can also build from 3.10.70-rt74 by applying the incremental patch:

http://www.kernel.org/pub/linux/kernel/projects/rt/3.10/incr/patch-3.10.70-rt74-rt75-rc2.patch.xz


Changes from 3.10.70-rt74:

---


Daniel Wagner (1):
      work-simple: Simple work queue implemenation

Josh Cartwright (1):
      lockdep: selftest: fix warnings due to missing PREEMPT_RT conditionals

Kevin Hao (1):
      netpoll: guard the access to dev->npinfo with rcu_read_lock/unlock_bh() for CONFIG_PREEMPT_RT_FULL=y

Mike Galbraith (4):
      x86: UV: raw_spinlock conversion
      scheduling while atomic in cgroup code
      sunrpc: make svc_xprt_do_enqueue() use get_cpu_light()
      fs,btrfs: fix rt deadlock on extent_buffer->lock

Paul E. McKenney (4):
      timers: Track total number of timers in list
      timers: Reduce __run_timers() latency for empty list
      timers: Reduce future __run_timers() latency for newly emptied list
      timers: Reduce future __run_timers() latency for first add to empty list

Paul Gortmaker (1):
      sas-ata/isci: dont't disable interrupts in qc_issue handler

Sebastian Andrzej Siewior (3):
      gpio: omap: use raw locks for locking
      rt-mutex: avoid a NULL pointer dereference on deadlock
      arm/futex: disable preemption during futex_atomic_cmpxchg_inatomic()

Steven Rostedt (1):
      create-rt-enqueue

Steven Rostedt (Red Hat) (2):
      Revert "timers: do not raise softirq unconditionally"
      Linux 3.10.70-rt75-rc2

Thomas Gleixner (14):
      rtmutex: Simplify rtmutex_slowtrylock()
      rtmutex: Simplify and document try_to_take_rtmutex()
      rtmutex: No need to keep task ref for lock owner check
      rtmutex: Clarify the boost/deboost part
      rtmutex: Document pi chain walk
      rtmutex: Simplify remove_waiter()
      rtmutex: Confine deadlock logic to futex
      rtmutex: Cleanup deadlock detector debug logic
      rtmutex: Avoid pointless requeueing in the deadlock detection chain walk
      futex: Make unlock_pi more robust
      futex: Use futex_top_waiter() in lookup_pi_state()
      futex: Split out the waiter check from lookup_pi_state()
      futex: Split out the first waiter attachment from lookup_pi_state()
      futex: Simplify futex_lock_pi_atomic() and make it more robust

Yadi.hu (1):
      ARM: enable irq in translation/section permission fault handlers

Yong Zhang (1):
      ARM: cmpxchg: define __HAVE_ARCH_CMPXCHG for armv6 and later

----
 arch/arm/include/asm/cmpxchg.h     |   2 +
 arch/arm/include/asm/futex.h       |   4 +
 arch/arm/mm/fault.c                |   6 +
 arch/x86/include/asm/uv/uv_bau.h   |  14 +-
 arch/x86/include/asm/uv/uv_hub.h   |   2 +-
 arch/x86/kernel/apic/x2apic_uv_x.c |   2 +-
 arch/x86/platform/uv/tlb_uv.c      |  26 +-
 arch/x86/platform/uv/uv_time.c     |  21 +-
 drivers/gpio/gpio-omap.c           |  70 ++---
 drivers/scsi/libsas/sas_ata.c      |   4 +-
 fs/btrfs/ctree.c                   |   4 +-
 fs/btrfs/extent-tree.c             |   8 -
 include/linux/hrtimer.h            |   3 +-
 include/linux/netpoll.h            |  16 +-
 include/linux/rtmutex.h            |   8 +-
 include/linux/work-simple.h        |  24 ++
 kernel/futex.c                     | 407 +++++++++++--------------
 kernel/hrtimer.c                   |  31 +-
 kernel/rt.c                        |   8 +-
 kernel/rtmutex-debug.c             |   5 +-
 kernel/rtmutex-debug.h             |   7 +-
 kernel/rtmutex-tester.c            |   4 +-
 kernel/rtmutex.c                   | 604 +++++++++++++++++++++++++++----------
 kernel/rtmutex.h                   |   7 +-
 kernel/rtmutex_common.h            |  22 +-
 kernel/sched/Makefile              |   1 +
 kernel/sched/work-simple.c         | 172 +++++++++++
 kernel/timer.c                     |  72 ++---
 lib/locking-selftest.c             |  27 ++
 localversion-rt                    |   2 +-
 mm/memcontrol.c                    |   7 +-
 net/sunrpc/svc_xprt.c              |   4 +-
 32 files changed, 1057 insertions(+), 537 deletions(-)

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

* [PATCH RT 1/4] fs,btrfs: fix rt deadlock on extent_buffer->lock
  2015-03-17 16:35 [PATCH RT 0/4] Linux 3.10.70-rt75-rc2 Steven Rostedt
@ 2015-03-17 16:35 ` Steven Rostedt
  2015-03-17 16:35   ` Steven Rostedt
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2015-03-17 16:35 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Thomas Gleixner, Carsten Emde, Sebastian Andrzej Siewior,
	John Kacur, Paul Gortmaker, Clark Williams, Chris Mason,
	Mike Galbraith

[-- Attachment #1: 0031-fs-btrfs-fix-rt-deadlock-on-extent_buffer-lock.patch --]
[-- Type: text/plain, Size: 2731 bytes --]

3.10.70-rt75-rc2 stable review patch.
If anyone has any objections, please let me know.

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

From: Mike Galbraith <mgalbraith@suse.de>

Sat Jul 14 12:30:41 CEST 2012

Trivially repeatable deadlock is cured by enabling lockdep code in
btrfs_clear_path_blocking() as suggested by Chris Mason.  He also
suggested restricting blocking reader count to one, and not allowing
a spinning reader while blocking reader exists.  This has proven to
be unnecessary, the strict lock order enforcement is enough.. or
rather that's my box's opinion after long hours of hard pounding.

Note: extent-tree.c bit is additional recommendation from Chris
      Mason, split into a separate patch after discussion.

Link: http://lkml.kernel.org/r/1414913478.5380.114.camel@marge.simpson.net

Cc: linux-rt-users <linux-rt-users@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Carsten Emde <C.Emde@osadl.org>
Cc: John Kacur <jkacur@redhat.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Clark Williams <williams@redhat.com>
Cc: Chris Mason <chris.mason@fusionio.com>
Signed-off-by: Mike Galbraith <efault@gmx.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Conflicts:
	fs/btrfs/extent-tree.c
---
 fs/btrfs/ctree.c       | 4 ++--
 fs/btrfs/extent-tree.c | 8 --------
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 7fb054ba1b60..07753ee246f1 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -81,7 +81,7 @@ noinline void btrfs_clear_path_blocking(struct btrfs_path *p,
 {
 	int i;
 
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
+#if (defined(CONFIG_DEBUG_LOCK_ALLOC) || defined(CONFIG_PREEMPT_RT_BASE))
 	/* lockdep really cares that we take all of these spinlocks
 	 * in the right order.  If any of the locks in the path are not
 	 * currently blocking, it is going to complain.  So, make really
@@ -108,7 +108,7 @@ noinline void btrfs_clear_path_blocking(struct btrfs_path *p,
 		}
 	}
 
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
+#if (defined(CONFIG_DEBUG_LOCK_ALLOC) || defined(CONFIG_PREEMPT_RT_BASE))
 	if (held)
 		btrfs_clear_lock_blocking_rw(held, held_rw);
 #endif
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f99c71e40f8b..920a85e82beb 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6678,14 +6678,6 @@ again:
 		goto again;
 	}
 
-	if (btrfs_test_opt(root, ENOSPC_DEBUG)) {
-		static DEFINE_RATELIMIT_STATE(_rs,
-				DEFAULT_RATELIMIT_INTERVAL * 10,
-				/*DEFAULT_RATELIMIT_BURST*/ 1);
-		if (__ratelimit(&_rs))
-			WARN(1, KERN_DEBUG
-				"btrfs: block rsv returned %d\n", ret);
-	}
 try_reserve:
 	ret = reserve_metadata_bytes(root, block_rsv, blocksize,
 				     BTRFS_RESERVE_NO_FLUSH);
-- 
2.1.4



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

* [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-17 16:35 [PATCH RT 0/4] Linux 3.10.70-rt75-rc2 Steven Rostedt
@ 2015-03-17 16:35   ` Steven Rostedt
  2015-03-17 16:35   ` Steven Rostedt
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2015-03-17 16:35 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Thomas Gleixner, Carsten Emde, Sebastian Andrzej Siewior,
	John Kacur, Paul Gortmaker

[-- Attachment #1: 0032-Revert-timers-do-not-raise-softirq-unconditionally.patch --]
[-- Type: text/plain, Size: 4459 bytes --]

3.10.70-rt75-rc2 stable review patch.
If anyone has any objections, please let me know.

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

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

This reverts commit f406cca1f3ecb370bc2915d176557b1cbafb8d4c

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Conflicts:
	kernel/timer.c
---
 include/linux/hrtimer.h |  3 ++-
 kernel/hrtimer.c        | 31 ++++++++++++++++++++++++-------
 kernel/timer.c          | 46 ++--------------------------------------------
 3 files changed, 28 insertions(+), 52 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index bdbf77db0f4d..79a7a35e1a6e 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -461,8 +461,9 @@ extern int schedule_hrtimeout_range_clock(ktime_t *expires,
 		unsigned long delta, const enum hrtimer_mode mode, int clock);
 extern int schedule_hrtimeout(ktime_t *expires, const enum hrtimer_mode mode);
 
-/* Called from the periodic timer tick */
+/* Soft interrupt function to run the hrtimer queues: */
 extern void hrtimer_run_queues(void);
+extern void hrtimer_run_pending(void);
 
 /* Bootup initialization: */
 extern void __init hrtimers_init(void);
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 1defbda1def8..1a28cafbac91 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1713,6 +1713,30 @@ static void run_hrtimer_softirq(struct softirq_action *h)
 }
 
 /*
+ * Called from timer softirq every jiffy, expire hrtimers:
+ *
+ * For HRT its the fall back code to run the softirq in the timer
+ * softirq context in case the hrtimer initialization failed or has
+ * not been done yet.
+ */
+void hrtimer_run_pending(void)
+{
+	if (hrtimer_hres_active())
+		return;
+
+	/*
+	 * This _is_ ugly: We have to check in the softirq context,
+	 * whether we can switch to highres and / or nohz mode. The
+	 * clocksource switch happens in the timer interrupt with
+	 * xtime_lock held. Notification from there only sets the
+	 * check bit in the tick_oneshot code, otherwise we might
+	 * deadlock vs. xtime_lock.
+	 */
+	if (tick_check_oneshot_change(!hrtimer_is_hres_enabled()))
+		hrtimer_switch_to_hres();
+}
+
+/*
  * Called from hardirq context every jiffy
  */
 void hrtimer_run_queues(void)
@@ -1725,13 +1749,6 @@ void hrtimer_run_queues(void)
 	if (hrtimer_hres_active())
 		return;
 
-	/*
-	 * Check whether we can switch to highres mode.
-	 */
-	if (tick_check_oneshot_change(!hrtimer_is_hres_enabled())
-	    && hrtimer_switch_to_hres())
-		return;
-
 	for (index = 0; index < HRTIMER_MAX_CLOCK_BASES; index++) {
 		base = &cpu_base->clock_base[index];
 		if (!timerqueue_getnext(&base->active))
diff --git a/kernel/timer.c b/kernel/timer.c
index 06050920aee5..a2bfef4a8f23 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1464,6 +1464,8 @@ static void run_timer_softirq(struct softirq_action *h)
 {
 	struct tvec_base *base = __this_cpu_read(tvec_bases);
 
+	hrtimer_run_pending();
+
 #if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)
 	irq_work_run();
 #endif
@@ -1477,52 +1479,8 @@ static void run_timer_softirq(struct softirq_action *h)
  */
 void run_local_timers(void)
 {
-	struct tvec_base *base = __this_cpu_read(tvec_bases);
-
 	hrtimer_run_queues();
-	/*
-	 * We can access this lockless as we are in the timer
-	 * interrupt. If there are no timers queued, nothing to do in
-	 * the timer softirq.
-	 */
-#ifdef CONFIG_PREEMPT_RT_FULL
-
-#ifndef CONFIG_SMP
-	/*
-	 * The spin_do_trylock() later may fail as the lock may be hold before
-	 * the interrupt arrived. The spin-lock debugging code will raise a
-	 * warning if the try_lock fails on UP. Since this is only an
-	 * optimization for the FULL_NO_HZ case (not to run the timer softirq on
-	 * an nohz_full CPU) we don't really care and shedule the softirq.
-	 */
 	raise_softirq(TIMER_SOFTIRQ);
-	return;
-#endif
-
-	/* On RT, irq work runs from softirq */
-	if (irq_work_needs_cpu()) {
-		raise_softirq(TIMER_SOFTIRQ);
-		return;
-	}
-
-	if (!spin_do_trylock(&base->lock)) {
-		raise_softirq(TIMER_SOFTIRQ);
-		return;
-	}
-#endif
-
-	if (!base->active_timers)
-		goto out;
-
-	/* Check whether the next pending timer has expired */
-	if (time_before_eq(base->next_timer, jiffies))
-		raise_softirq(TIMER_SOFTIRQ);
-out:
-#ifdef CONFIG_PREEMPT_RT_FULL
-	rt_spin_unlock_after_trylock_in_irq(&base->lock);
-#endif
-	/* The ; ensures that gcc won't complain in the !RT case */
-	;
 }
 
 #ifdef __ARCH_WANT_SYS_ALARM
-- 
2.1.4



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

* [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
@ 2015-03-17 16:35   ` Steven Rostedt
  0 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2015-03-17 16:35 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Thomas Gleixner, Carsten Emde, Sebastian Andrzej Siewior,
	John Kacur, Paul Gortmaker

[-- Attachment #1: 0032-Revert-timers-do-not-raise-softirq-unconditionally.patch --]
[-- Type: text/plain, Size: 4108 bytes --]

3.10.70-rt75-rc2 stable review patch.
If anyone has any objections, please let me know.

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

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

This reverts commit f406cca1f3ecb370bc2915d176557b1cbafb8d4c

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Conflicts:
	kernel/timer.c
---
 include/linux/hrtimer.h |  3 ++-
 kernel/hrtimer.c        | 31 ++++++++++++++++++++++++-------
 kernel/timer.c          | 46 ++--------------------------------------------
 3 files changed, 28 insertions(+), 52 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index bdbf77db0f4d..79a7a35e1a6e 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -461,8 +461,9 @@ extern int schedule_hrtimeout_range_clock(ktime_t *expires,
 		unsigned long delta, const enum hrtimer_mode mode, int clock);
 extern int schedule_hrtimeout(ktime_t *expires, const enum hrtimer_mode mode);
 
-/* Called from the periodic timer tick */
+/* Soft interrupt function to run the hrtimer queues: */
 extern void hrtimer_run_queues(void);
+extern void hrtimer_run_pending(void);
 
 /* Bootup initialization: */
 extern void __init hrtimers_init(void);
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 1defbda1def8..1a28cafbac91 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1713,6 +1713,30 @@ static void run_hrtimer_softirq(struct softirq_action *h)
 }
 
 /*
+ * Called from timer softirq every jiffy, expire hrtimers:
+ *
+ * For HRT its the fall back code to run the softirq in the timer
+ * softirq context in case the hrtimer initialization failed or has
+ * not been done yet.
+ */
+void hrtimer_run_pending(void)
+{
+	if (hrtimer_hres_active())
+		return;
+
+	/*
+	 * This _is_ ugly: We have to check in the softirq context,
+	 * whether we can switch to highres and / or nohz mode. The
+	 * clocksource switch happens in the timer interrupt with
+	 * xtime_lock held. Notification from there only sets the
+	 * check bit in the tick_oneshot code, otherwise we might
+	 * deadlock vs. xtime_lock.
+	 */
+	if (tick_check_oneshot_change(!hrtimer_is_hres_enabled()))
+		hrtimer_switch_to_hres();
+}
+
+/*
  * Called from hardirq context every jiffy
  */
 void hrtimer_run_queues(void)
@@ -1725,13 +1749,6 @@ void hrtimer_run_queues(void)
 	if (hrtimer_hres_active())
 		return;
 
-	/*
-	 * Check whether we can switch to highres mode.
-	 */
-	if (tick_check_oneshot_change(!hrtimer_is_hres_enabled())
-	    && hrtimer_switch_to_hres())
-		return;
-
 	for (index = 0; index < HRTIMER_MAX_CLOCK_BASES; index++) {
 		base = &cpu_base->clock_base[index];
 		if (!timerqueue_getnext(&base->active))
diff --git a/kernel/timer.c b/kernel/timer.c
index 06050920aee5..a2bfef4a8f23 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1464,6 +1464,8 @@ static void run_timer_softirq(struct softirq_action *h)
 {
 	struct tvec_base *base = __this_cpu_read(tvec_bases);
 
+	hrtimer_run_pending();
+
 #if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)
 	irq_work_run();
 #endif
@@ -1477,52 +1479,8 @@ static void run_timer_softirq(struct softirq_action *h)
  */
 void run_local_timers(void)
 {
-	struct tvec_base *base = __this_cpu_read(tvec_bases);
-
 	hrtimer_run_queues();
-	/*
-	 * We can access this lockless as we are in the timer
-	 * interrupt. If there are no timers queued, nothing to do in
-	 * the timer softirq.
-	 */
-#ifdef CONFIG_PREEMPT_RT_FULL
-
-#ifndef CONFIG_SMP
-	/*
-	 * The spin_do_trylock() later may fail as the lock may be hold before
-	 * the interrupt arrived. The spin-lock debugging code will raise a
-	 * warning if the try_lock fails on UP. Since this is only an
-	 * optimization for the FULL_NO_HZ case (not to run the timer softirq on
-	 * an nohz_full CPU) we don't really care and shedule the softirq.
-	 */
 	raise_softirq(TIMER_SOFTIRQ);
-	return;
-#endif
-
-	/* On RT, irq work runs from softirq */
-	if (irq_work_needs_cpu()) {
-		raise_softirq(TIMER_SOFTIRQ);
-		return;
-	}
-
-	if (!spin_do_trylock(&base->lock)) {
-		raise_softirq(TIMER_SOFTIRQ);
-		return;
-	}
-#endif
-
-	if (!base->active_timers)
-		goto out;

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

* [PATCH RT 3/4] netpoll: guard the access to dev->npinfo with rcu_read_lock/unlock_bh() for CONFIG_PREEMPT_RT_FULL=y
  2015-03-17 16:35 [PATCH RT 0/4] Linux 3.10.70-rt75-rc2 Steven Rostedt
  2015-03-17 16:35 ` [PATCH RT 1/4] fs,btrfs: fix rt deadlock on extent_buffer->lock Steven Rostedt
  2015-03-17 16:35   ` Steven Rostedt
@ 2015-03-17 16:35 ` Steven Rostedt
  2015-03-17 16:35 ` [PATCH RT 4/4] Linux 3.10.70-rt75-rc2 Steven Rostedt
  3 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2015-03-17 16:35 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Thomas Gleixner, Carsten Emde, Sebastian Andrzej Siewior,
	John Kacur, Paul Gortmaker, Kevin Hao

[-- Attachment #1: 0033-netpoll-guard-the-access-to-dev-npinfo-with-rcu_read.patch --]
[-- Type: text/plain, Size: 3243 bytes --]

3.10.70-rt75-rc2 stable review patch.
If anyone has any objections, please let me know.

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

From: Kevin Hao <haokexin@gmail.com>

For vanilla kernel we don't need to invoke rcu_read_lock/unlock_bh
explicitly to mark an RCU-bh critical section in the softirq context
because bh is already disabled in this case. But for a rt kernel,
the commit ("rcu: Merge RCU-bh into RCU-preempt") implements the
RCU-bh in term of RCU-preempt. So we have to use
rcu_read_lock/unlock_bh() to mark an RCU-bh critical section even in
a softirq context. Otherwise we will get a call trace like this:
  include/linux/netpoll.h:90 suspicious rcu_dereference_check() usage!
  other info that might help us debug this:
  rcu_scheduler_active = 1, debug_locks = 0
  1 lock held by irq/177-eth0_g0/129:
   #0:  (&per_cpu(local_softirq_locks[i], __cpu).lock){+.+...}, at: [<8002f544>] do_current_softirqs+0x12c/0x5ec

  stack backtrace:
  CPU: 0 PID: 129 Comm: irq/177-eth0_g0 Not tainted 3.14.23 #11
  [<80018c0c>] (unwind_backtrace) from [<800138b0>] (show_stack+0x20/0x24)
  [<800138b0>] (show_stack) from [<8075c3bc>] (dump_stack+0x84/0xd0)
  [<8075c3bc>] (dump_stack) from [<8008111c>] (lockdep_rcu_suspicious+0xe8/0x11c)
  [<8008111c>] (lockdep_rcu_suspicious) from [<805e94e8>] (dev_gro_receive+0x240/0x724)
  [<805e94e8>] (dev_gro_receive) from [<805e9c34>] (napi_gro_receive+0x3c/0x1e8)
  [<805e9c34>] (napi_gro_receive) from [<804b01ac>] (gfar_clean_rx_ring+0x2d4/0x624)
  [<804b01ac>] (gfar_clean_rx_ring) from [<804b078c>] (gfar_poll_rx_sq+0x58/0xe8)
  [<804b078c>] (gfar_poll_rx_sq) from [<805eada8>] (net_rx_action+0x1c8/0x418)
  [<805eada8>] (net_rx_action) from [<8002f62c>] (do_current_softirqs+0x214/0x5ec)
  [<8002f62c>] (do_current_softirqs) from [<8002fa88>] (__local_bh_enable+0x84/0x9c)
  [<8002fa88>] (__local_bh_enable) from [<8002fab8>] (local_bh_enable+0x18/0x1c)
  [<8002fab8>] (local_bh_enable) from [<80093924>] (irq_forced_thread_fn+0x50/0x74)
  [<80093924>] (irq_forced_thread_fn) from [<80093c30>] (irq_thread+0x158/0x1c4)
  [<80093c30>] (irq_thread) from [<800555b8>] (kthread+0xd4/0xe8)
  [<800555b8>] (kthread) from [<8000ee88>] (ret_from_fork+0x14/0x20)

Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/netpoll.h | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index fa2cb76a7029..c1e80c6272aa 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -86,9 +86,21 @@ static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
 #ifdef CONFIG_NETPOLL
 static inline bool netpoll_rx_on(struct sk_buff *skb)
 {
-	struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
+	struct netpoll_info *npinfo;
+	bool ret;
+
+#ifdef CONFIG_PREEMPT_RT_FULL
+	rcu_read_lock_bh();
+#endif
+
+	npinfo = rcu_dereference_bh(skb->dev->npinfo);
+	ret = npinfo && (!list_empty(&npinfo->rx_np) || npinfo->rx_flags);
+
+#ifdef CONFIG_PREEMPT_RT_FULL
+	rcu_read_unlock_bh();
+#endif
 
-	return npinfo && (!list_empty(&npinfo->rx_np) || npinfo->rx_flags);
+	return ret;
 }
 
 static inline bool netpoll_rx(struct sk_buff *skb)
-- 
2.1.4



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

* [PATCH RT 4/4] Linux 3.10.70-rt75-rc2
  2015-03-17 16:35 [PATCH RT 0/4] Linux 3.10.70-rt75-rc2 Steven Rostedt
                   ` (2 preceding siblings ...)
  2015-03-17 16:35 ` [PATCH RT 3/4] netpoll: guard the access to dev->npinfo with rcu_read_lock/unlock_bh() for CONFIG_PREEMPT_RT_FULL=y Steven Rostedt
@ 2015-03-17 16:35 ` Steven Rostedt
  3 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2015-03-17 16:35 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Thomas Gleixner, Carsten Emde, Sebastian Andrzej Siewior,
	John Kacur, Paul Gortmaker

[-- Attachment #1: 0034-Linux-3.10.70-rt75-rc2.patch --]
[-- Type: text/plain, Size: 414 bytes --]

3.10.70-rt75-rc2 stable review patch.
If anyone has any objections, please let me know.

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

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

---
 localversion-rt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/localversion-rt b/localversion-rt
index 7d028f4a9e56..2d2ca36685e5 100644
--- a/localversion-rt
+++ b/localversion-rt
@@ -1 +1 @@
--rt74
+-rt75-rc2
-- 
2.1.4



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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-17 16:35   ` Steven Rostedt
  (?)
@ 2015-03-17 20:35   ` Steven Rostedt
  2015-03-19  8:17     ` Mike Galbraith
  -1 siblings, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2015-03-17 20:35 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Thomas Gleixner, Carsten Emde, Sebastian Andrzej Siewior,
	John Kacur, Paul Gortmaker

On Tue, 17 Mar 2015 12:35:43 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> 3.10.70-rt75-rc2 stable review patch.
> If anyone has any objections, please let me know.
> 
> ------------------
> 

Here's the missing change log for this revert. I'll go back and add it
in:


An issue arisen that if a rt_mutex (spin_lock converted to a mutex
in PREEMPT_RT) is taken in hard interrupt context, it could cause
a false deadlock detection and trigger a BUG_ON() from the return
value of task_blocks_on_rt_mutex() in rt_spin_lock_slowlock().

The problem is this:

    CPU0			CPU1
    ----			----
  spin_lock(A)
				spin_lock(A)
			[ blocks, but spins as owner on
			  CPU 0 is running ]

				<interrupt>
					spin_trylock(B)
					[ succeeds ]

  spin_lock(B)
  <blocks>

Now the deadlock detection triggers and follows the locking:

  Task X (on CPU0) blocked on spinlock B owned by task Y on
  CPU1 (via the interrupt taking it with a try lock)

  The owner of B (Y) is blocked on spin_lock A (still spinning)
  A is owned by task X (self). DEADLOCK detected! BUG_ON triggered.

This was caused by the code to try to not raise softirq unconditionally
to allow NO_HZ_FULL to work. Unfortunately, reverting that patch causes
NO_HZ_FULL to break again, but that's still better than triggering
a BUG_ON().



-- Steve

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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-17 20:35   ` Steven Rostedt
@ 2015-03-19  8:17     ` Mike Galbraith
  2015-03-19 16:26       ` Steven Rostedt
  0 siblings, 1 reply; 25+ messages in thread
From: Mike Galbraith @ 2015-03-19  8:17 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-rt-users, Thomas Gleixner, Carsten Emde,
	Sebastian Andrzej Siewior, John Kacur, Paul Gortmaker

On Tue, 2015-03-17 at 16:35 -0400, Steven Rostedt wrote:
> On Tue, 17 Mar 2015 12:35:43 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > 3.10.70-rt75-rc2 stable review patch.
> > If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> 
> Here's the missing change log for this revert. I'll go back and add it
> in:
> 
> 
> An issue arisen that if a rt_mutex (spin_lock converted to a mutex
> in PREEMPT_RT) is taken in hard interrupt context, it could cause
> a false deadlock detection and trigger a BUG_ON() from the return
> value of task_blocks_on_rt_mutex() in rt_spin_lock_slowlock().
> 
> The problem is this:
> 
>     CPU0			CPU1
>     ----			----
>   spin_lock(A)
> 				spin_lock(A)
> 			[ blocks, but spins as owner on
> 			  CPU 0 is running ]
> 
> 				<interrupt>
> 					spin_trylock(B)
> 					[ succeeds ]
> 
>   spin_lock(B)
>   <blocks>
> 
> Now the deadlock detection triggers and follows the locking:
> 
>   Task X (on CPU0) blocked on spinlock B owned by task Y on
>   CPU1 (via the interrupt taking it with a try lock)
> 
>   The owner of B (Y) is blocked on spin_lock A (still spinning)
>   A is owned by task X (self). DEADLOCK detected! BUG_ON triggered.
> 
> This was caused by the code to try to not raise softirq unconditionally
> to allow NO_HZ_FULL to work. Unfortunately, reverting that patch causes
> NO_HZ_FULL to break again, but that's still better than triggering
> a BUG_ON().


(aw crap, let's go shopping)... so why is the one in timer.c ok?

	-Mike


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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-19  8:17     ` Mike Galbraith
@ 2015-03-19 16:26       ` Steven Rostedt
  2015-03-19 16:42         ` Thavatchai Makphaibulchoke
  2015-03-24 18:10         ` Sebastian Andrzej Siewior
  0 siblings, 2 replies; 25+ messages in thread
From: Steven Rostedt @ 2015-03-19 16:26 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: linux-kernel, linux-rt-users, Thomas Gleixner, Carsten Emde,
	Sebastian Andrzej Siewior, John Kacur, Paul Gortmaker

On Thu, 19 Mar 2015 09:17:09 +0100
Mike Galbraith <umgwanakikbuti@gmail.com> wrote:


> (aw crap, let's go shopping)... so why is the one in timer.c ok?

It's not. Sebastian, you said there were no other cases of rt_mutexes
being taken in hard irq context. Looks like timer.c has one.

So perhaps the real fix is to get that special case of ownership in
hard interrupt context?

-- Steve


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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-19 16:26       ` Steven Rostedt
@ 2015-03-19 16:42         ` Thavatchai Makphaibulchoke
  2015-03-21 18:02           ` Mike Galbraith
  2015-03-24 18:10         ` Sebastian Andrzej Siewior
  1 sibling, 1 reply; 25+ messages in thread
From: Thavatchai Makphaibulchoke @ 2015-03-19 16:42 UTC (permalink / raw)
  To: Steven Rostedt, Mike Galbraith
  Cc: linux-kernel, linux-rt-users, Thomas Gleixner, Carsten Emde,
	Sebastian Andrzej Siewior, John Kacur, Paul Gortmaker


On 03/19/2015 10:26 AM, Steven Rostedt wrote:
> On Thu, 19 Mar 2015 09:17:09 +0100
> Mike Galbraith <umgwanakikbuti@gmail.com> wrote:
> 
> 
>> (aw crap, let's go shopping)... so why is the one in timer.c ok?
> 
> It's not. Sebastian, you said there were no other cases of rt_mutexes
> being taken in hard irq context. Looks like timer.c has one.
> 
> So perhaps the real fix is to get that special case of ownership in
> hard interrupt context?
> 
> -- Steve
> 

Steve, I'm still working on the fix we discussed using dummy irq_task.
I should be able to submit some time next week, if still interested.

Either that, or I think we should remove the function
spin_do_trylock_in_interrupt() to prevent any possibility of running
into similar problems in the future.

Thanks,
Mak.



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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-19 16:42         ` Thavatchai Makphaibulchoke
@ 2015-03-21 18:02           ` Mike Galbraith
  2015-03-23  4:42             ` Mike Galbraith
  2015-03-24 18:15               ` Sebastian Andrzej Siewior
  0 siblings, 2 replies; 25+ messages in thread
From: Mike Galbraith @ 2015-03-21 18:02 UTC (permalink / raw)
  To: Thavatchai Makphaibulchoke
  Cc: Steven Rostedt, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, Sebastian Andrzej Siewior, John Kacur,
	Paul Gortmaker

On Thu, 2015-03-19 at 10:42 -0600, Thavatchai Makphaibulchoke wrote:
> On 03/19/2015 10:26 AM, Steven Rostedt wrote:
> > On Thu, 19 Mar 2015 09:17:09 +0100
> > Mike Galbraith <umgwanakikbuti@gmail.com> wrote:
> > 
> > 
> >> (aw crap, let's go shopping)... so why is the one in timer.c ok?
> > 
> > It's not. Sebastian, you said there were no other cases of rt_mutexes
> > being taken in hard irq context. Looks like timer.c has one.
> > 
> > So perhaps the real fix is to get that special case of ownership in
> > hard interrupt context?
> > 
> > -- Steve
> > 
> 
> Steve, I'm still working on the fix we discussed using dummy irq_task.
> I should be able to submit some time next week, if still interested.
> 
> Either that, or I think we should remove the function
> spin_do_trylock_in_interrupt() to prevent any possibility of running
> into similar problems in the future.

Why can't we just Let swapper be the owner when in irq with no dummy?

I have "don't raise timer unconditionally" re-applied, the check for a
running callback bits of my nohz_full fixlet, and the below on top of
that, and all _seems_ well.

---
 include/linux/spinlock_rt.h |    1 
 kernel/locking/rtmutex.c    |   58 ++++++++++++++++++--------------------------
 kernel/time/timer.c         |    4 +--
 3 files changed, 27 insertions(+), 36 deletions(-)

--- a/include/linux/spinlock_rt.h
+++ b/include/linux/spinlock_rt.h
@@ -22,7 +22,6 @@ extern void __lockfunc rt_spin_lock(spin
 extern unsigned long __lockfunc rt_spin_lock_trace_flags(spinlock_t *lock);
 extern void __lockfunc rt_spin_lock_nested(spinlock_t *lock, int subclass);
 extern void __lockfunc rt_spin_unlock(spinlock_t *lock);
-extern void __lockfunc rt_spin_unlock_after_trylock_in_irq(spinlock_t *lock);
 extern void __lockfunc rt_spin_unlock_wait(spinlock_t *lock);
 extern int __lockfunc rt_spin_trylock_irqsave(spinlock_t *lock, unsigned long *flags);
 extern int __lockfunc rt_spin_trylock_bh(spinlock_t *lock);
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -934,8 +934,10 @@ static inline void rt_spin_lock_fastlock
 static inline void rt_spin_lock_fastunlock(struct rt_mutex *lock,
 					   void  (*slowfn)(struct rt_mutex *lock))
 {
-	if (likely(rt_mutex_cmpxchg(lock, current, NULL)))
-		rt_mutex_deadlock_account_unlock(current);
+	struct task_struct *owner = rt_mutex_owner(lock);
+
+	if (likely(rt_mutex_cmpxchg(lock, owner, NULL)))
+		rt_mutex_deadlock_account_unlock(owner);
 	else
 		slowfn(lock);
 }
@@ -1072,11 +1074,16 @@ static void wakeup_next_waiter(struct rt
 /*
  * Slow path to release a rt_mutex spin_lock style
  */
-static void __sched __rt_spin_lock_slowunlock(struct rt_mutex *lock)
+static void noinline __sched rt_spin_lock_slowunlock(struct rt_mutex *lock)
 {
+	struct task_struct *owner;
+
+	raw_spin_lock(&lock->wait_lock);
+
 	debug_rt_mutex_unlock(lock);
 
-	rt_mutex_deadlock_account_unlock(current);
+	owner = rt_mutex_owner(lock);
+	rt_mutex_deadlock_account_unlock(owner);
 
 	if (!rt_mutex_has_waiters(lock)) {
 		lock->owner = NULL;
@@ -1089,24 +1096,8 @@ static void __sched __rt_spin_lock_slowu
 	raw_spin_unlock(&lock->wait_lock);
 
 	/* Undo pi boosting.when necessary */
-	rt_mutex_adjust_prio(current);
-}
-
-static void  noinline __sched rt_spin_lock_slowunlock(struct rt_mutex *lock)
-{
-	raw_spin_lock(&lock->wait_lock);
-	__rt_spin_lock_slowunlock(lock);
-}
-
-static void  noinline __sched rt_spin_lock_slowunlock_hirq(struct rt_mutex *lock)
-{
-	int ret;
-
-	do {
-		ret = raw_spin_trylock(&lock->wait_lock);
-	} while (!ret);
-
-	__rt_spin_lock_slowunlock(lock);
+	if (likely(!is_idle_task(owner)))
+		rt_mutex_adjust_prio(owner);
 }
 
 void __lockfunc rt_spin_lock(spinlock_t *lock)
@@ -1139,13 +1130,6 @@ void __lockfunc rt_spin_unlock(spinlock_
 }
 EXPORT_SYMBOL(rt_spin_unlock);
 
-void __lockfunc rt_spin_unlock_after_trylock_in_irq(spinlock_t *lock)
-{
-	/* NOTE: we always pass in '1' for nested, for simplicity */
-	spin_release(&lock->dep_map, 1, _RET_IP_);
-	rt_spin_lock_fastunlock(&lock->lock, rt_spin_lock_slowunlock_hirq);
-}
-
 void __lockfunc __rt_spin_unlock(struct rt_mutex *lock)
 {
 	rt_spin_lock_fastunlock(lock, rt_spin_lock_slowunlock);
@@ -1341,7 +1325,7 @@ static int task_blocks_on_rt_mutex(struc
 
 	raw_spin_unlock_irqrestore(&task->pi_lock, flags);
 
-	if (!owner)
+	if (!owner || is_idle_task(owner))
 		return 0;
 
 	raw_spin_lock_irqsave(&owner->pi_lock, flags);
@@ -1746,6 +1730,7 @@ rt_mutex_slowlock(struct rt_mutex *lock,
  */
 static inline int rt_mutex_slowtrylock(struct rt_mutex *lock)
 {
+	struct task_struct *owner;
 	int ret;
 
 	/*
@@ -1763,7 +1748,9 @@ static inline int rt_mutex_slowtrylock(s
 	if (!raw_spin_trylock(&lock->wait_lock))
 		return 0;
 
-	ret = try_to_take_rt_mutex(lock, current, NULL);
+	owner = in_irq() ? idle_task(raw_smp_processor_id()) : current;
+
+	ret = try_to_take_rt_mutex(lock, owner, NULL);
 
 	/*
 	 * try_to_take_rt_mutex() sets the lock waiters bit
@@ -1886,8 +1873,13 @@ static inline int
 rt_mutex_fasttrylock(struct rt_mutex *lock,
 		     int (*slowfn)(struct rt_mutex *lock))
 {
-	if (likely(rt_mutex_cmpxchg(lock, NULL, current))) {
-		rt_mutex_deadlock_account_lock(lock, current);
+	struct task_struct *owner = current;
+
+	if (unlikely(in_irq()))
+		owner = idle_task(raw_smp_processor_id());
+
+	if (likely(rt_mutex_cmpxchg(lock, NULL, owner))) {
+		rt_mutex_deadlock_account_lock(lock, owner);
 		return 1;
 	}
 	return slowfn(lock);
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1425,7 +1425,7 @@ unsigned long get_next_timer_interrupt(u
 		expires = base->next_timer;
 	}
 #ifdef CONFIG_PREEMPT_RT_FULL
-	rt_spin_unlock_after_trylock_in_irq(&base->lock);
+	rt_spin_unlock(&base->lock);
 #else
 	spin_unlock(&base->lock);
 #endif
@@ -1518,7 +1518,7 @@ void run_local_timers(void)
 		raise_softirq(TIMER_SOFTIRQ);
 out:
 #ifdef CONFIG_PREEMPT_RT_FULL
-	rt_spin_unlock_after_trylock_in_irq(&base->lock);
+	rt_spin_unlock(&base->lock);
 #endif
 	/* The ; ensures that gcc won't complain in the !RT case */
 	;



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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-21 18:02           ` Mike Galbraith
@ 2015-03-23  4:42             ` Mike Galbraith
  2015-03-26  2:17               ` Thavatchai Makphaibulchoke
  2015-03-24 18:15               ` Sebastian Andrzej Siewior
  1 sibling, 1 reply; 25+ messages in thread
From: Mike Galbraith @ 2015-03-23  4:42 UTC (permalink / raw)
  To: Thavatchai Makphaibulchoke
  Cc: Steven Rostedt, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, Sebastian Andrzej Siewior, John Kacur,
	Paul Gortmaker

On Sat, 2015-03-21 at 19:02 +0100, Mike Galbraith wrote:
> On Thu, 2015-03-19 at 10:42 -0600, Thavatchai Makphaibulchoke wrote:
> > On 03/19/2015 10:26 AM, Steven Rostedt wrote:
> > > On Thu, 19 Mar 2015 09:17:09 +0100
> > > Mike Galbraith <umgwanakikbuti@gmail.com> wrote:
> > > 
> > > 
> > >> (aw crap, let's go shopping)... so why is the one in timer.c ok?
> > > 
> > > It's not. Sebastian, you said there were no other cases of rt_mutexes
> > > being taken in hard irq context. Looks like timer.c has one.
> > > 
> > > So perhaps the real fix is to get that special case of ownership in
> > > hard interrupt context?
> > > 
> > > -- Steve
> > > 
> > 
> > Steve, I'm still working on the fix we discussed using dummy irq_task.
> > I should be able to submit some time next week, if still interested.
> > 
> > Either that, or I think we should remove the function
> > spin_do_trylock_in_interrupt() to prevent any possibility of running
> > into similar problems in the future.
> 
> Why can't we just Let swapper be the owner when in irq with no dummy?
> 
> I have "don't raise timer unconditionally" re-applied, the check for a
> running callback bits of my nohz_full fixlet, and the below on top of
> that, and all _seems_ well.

But not so well on 64 core box.  That has nothing to do with hacklet
though, re-applying timers-do-not-raise-softirq-unconditionally.patch
without thta hangs the 64 core box during boot with no help from me
other than to patchlet to let nohz work at all, seems there's another
issue lurking there.  Hohum.  Without 'don't raise..", big box is fine.

	-Mike


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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-19 16:26       ` Steven Rostedt
  2015-03-19 16:42         ` Thavatchai Makphaibulchoke
@ 2015-03-24 18:10         ` Sebastian Andrzej Siewior
  2015-03-25  2:33           ` Mike Galbraith
  1 sibling, 1 reply; 25+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-03-24 18:10 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Mike Galbraith, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, John Kacur, Paul Gortmaker

* Steven Rostedt | 2015-03-19 12:26:11 [-0400]:

>On Thu, 19 Mar 2015 09:17:09 +0100
>Mike Galbraith <umgwanakikbuti@gmail.com> wrote:
>
>
>> (aw crap, let's go shopping)... so why is the one in timer.c ok?
>
>It's not. Sebastian, you said there were no other cases of rt_mutexes
>being taken in hard irq context. Looks like timer.c has one.

If you refer to switch_timer_base() then this one is not taken in
hard-irq context. The callchain is:

lock_timer_base() (with spin_lock_irqsave(&base->lock, *flags) which
                   makes it a sleeping lock or lockdep would scream)
  -> switch_timer_base()
     -> spin_trylock() (not in hardirq conteyt)

>So perhaps the real fix is to get that special case of ownership in
>hard interrupt context?

I'm really not sure we want to keep doing this.

>
>-- Steve

Sebastian

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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-21 18:02           ` Mike Galbraith
@ 2015-03-24 18:15               ` Sebastian Andrzej Siewior
  2015-03-24 18:15               ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 25+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-03-24 18:15 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Thavatchai Makphaibulchoke, Steven Rostedt, linux-kernel,
	linux-rt-users, Thomas Gleixner, Carsten Emde, John Kacur,
	Paul Gortmaker

* Mike Galbraith | 2015-03-21 19:02:23 [+0100]:

>> Steve, I'm still working on the fix we discussed using dummy irq_task.
>> I should be able to submit some time next week, if still interested.
>> 
>> Either that, or I think we should remove the function
>> spin_do_trylock_in_interrupt() to prevent any possibility of running
>> into similar problems in the future.
>
>Why can't we just Let swapper be the owner when in irq with no dummy?

so you abuse the owner to be swapper and mask it out everywhere. It does
not look like a final solution. I'm more inclined to take you other
patch. In the end I hope we get a timer re-work and do not need any
hackary around it…

>I have "don't raise timer unconditionally" re-applied, the check for a
>running callback bits of my nohz_full fixlet, and the below on top of
>that, and all _seems_ well.

Sebastian

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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
@ 2015-03-24 18:15               ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 25+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-03-24 18:15 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Thavatchai Makphaibulchoke, Steven Rostedt, linux-kernel,
	linux-rt-users, Thomas Gleixner, Carsten Emde, John Kacur,
	Paul Gortmaker

* Mike Galbraith | 2015-03-21 19:02:23 [+0100]:

>> Steve, I'm still working on the fix we discussed using dummy irq_task.
>> I should be able to submit some time next week, if still interested.
>> 
>> Either that, or I think we should remove the function
>> spin_do_trylock_in_interrupt() to prevent any possibility of running
>> into similar problems in the future.
>
>Why can't we just Let swapper be the owner when in irq with no dummy?

so you abuse the owner to be swapper and mask it out everywhere. It does
not look like a final solution. I'm more inclined to take you other
patch. In the end I hope we get a timer re-work and do not need any
hackary around it…

>I have "don't raise timer unconditionally" re-applied, the check for a
>running callback bits of my nohz_full fixlet, and the below on top of
>that, and all _seems_ well.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-24 18:10         ` Sebastian Andrzej Siewior
@ 2015-03-25  2:33           ` Mike Galbraith
  2015-04-09 13:39             ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 25+ messages in thread
From: Mike Galbraith @ 2015-03-25  2:33 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Steven Rostedt, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, John Kacur, Paul Gortmaker

On Tue, 2015-03-24 at 19:10 +0100, Sebastian Andrzej Siewior wrote:
> * Steven Rostedt | 2015-03-19 12:26:11 [-0400]:
> 
> >On Thu, 19 Mar 2015 09:17:09 +0100
> >Mike Galbraith <umgwanakikbuti@gmail.com> wrote:
> >
> >
> >> (aw crap, let's go shopping)... so why is the one in timer.c ok?
> >
> >It's not. Sebastian, you said there were no other cases of rt_mutexes
> >being taken in hard irq context. Looks like timer.c has one.
> 
> If you refer to switch_timer_base() then this one is not taken in
> hard-irq context. The callchain is:
> 
> lock_timer_base() (with spin_lock_irqsave(&base->lock, *flags) which
>                    makes it a sleeping lock or lockdep would scream)
>   -> switch_timer_base()
>      -> spin_trylock() (not in hardirq conteyt)

Nah, I was referring to get_next_timer_interrupt() because I saw that
rt_spin_unlock_after_trylock_in_irq(&base->lock) sitting there.

	-Mike


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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-24 18:15               ` Sebastian Andrzej Siewior
@ 2015-03-25  2:38                 ` Mike Galbraith
  -1 siblings, 0 replies; 25+ messages in thread
From: Mike Galbraith @ 2015-03-25  2:38 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Thavatchai Makphaibulchoke, Steven Rostedt, linux-kernel,
	linux-rt-users, Thomas Gleixner, Carsten Emde, John Kacur,
	Paul Gortmaker

On Tue, 2015-03-24 at 19:15 +0100, Sebastian Andrzej Siewior wrote:
> * Mike Galbraith | 2015-03-21 19:02:23 [+0100]:
> 
> >> Steve, I'm still working on the fix we discussed using dummy irq_task.
> >> I should be able to submit some time next week, if still interested.
> >> 
> >> Either that, or I think we should remove the function
> >> spin_do_trylock_in_interrupt() to prevent any possibility of running
> >> into similar problems in the future.
> >
> >Why can't we just Let swapper be the owner when in irq with no dummy?
> 
> so you abuse the owner to be swapper and mask it out everywhere. It does
> not look like a final solution. I'm more inclined to take you other
> patch. In the end I hope we get a timer re-work and do not need any
> hackary around it…

Yeah, it was just _a_ way to dodge the bullet.

	-Mike


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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
@ 2015-03-25  2:38                 ` Mike Galbraith
  0 siblings, 0 replies; 25+ messages in thread
From: Mike Galbraith @ 2015-03-25  2:38 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Thavatchai Makphaibulchoke, Steven Rostedt, linux-kernel,
	linux-rt-users, Thomas Gleixner, Carsten Emde, John Kacur,
	Paul Gortmaker

On Tue, 2015-03-24 at 19:15 +0100, Sebastian Andrzej Siewior wrote:
> * Mike Galbraith | 2015-03-21 19:02:23 [+0100]:
> 
> >> Steve, I'm still working on the fix we discussed using dummy irq_task.
> >> I should be able to submit some time next week, if still interested.
> >> 
> >> Either that, or I think we should remove the function
> >> spin_do_trylock_in_interrupt() to prevent any possibility of running
> >> into similar problems in the future.
> >
> >Why can't we just Let swapper be the owner when in irq with no dummy?
> 
> so you abuse the owner to be swapper and mask it out everywhere. It does
> not look like a final solution. I'm more inclined to take you other
> patch. In the end I hope we get a timer re-work and do not need any
> hackary around it…

Yeah, it was just _a_ way to dodge the bullet.

	-Mike

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-23  4:42             ` Mike Galbraith
@ 2015-03-26  2:17               ` Thavatchai Makphaibulchoke
  2015-03-26  5:23                 ` Mike Galbraith
  0 siblings, 1 reply; 25+ messages in thread
From: Thavatchai Makphaibulchoke @ 2015-03-26  2:17 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Steven Rostedt, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, Sebastian Andrzej Siewior, John Kacur,
	Paul Gortmaker



On 03/22/2015 10:42 PM, Mike Galbraith wrote:
>> Why can't we just Let swapper be the owner when in irq with no dummy?
>>

Thanks Mike for the suggestion.  That may also work. Unfortunately
somehow I'm still having a hung problem, which may be related to the
priority of the interrupt handler task.

>> I have "don't raise timer unconditionally" re-applied, the check for a
>> running callback bits of my nohz_full fixlet, and the below on top of
>> that, and all _seems_ well.
> 
> But not so well on 64 core box.  That has nothing to do with hacklet
> though, re-applying timers-do-not-raise-softirq-unconditionally.patch
> without thta hangs the 64 core box during boot with no help from me
> other than to patchlet to let nohz work at all, seems there's another
> issue lurking there.  Hohum.  Without 'don't raise..", big box is fine.
> 

If you get your patch to work, I could try my test that was able to
reproduce the problem consistently.

Thanks,
Mak.

> 	-Mike
> 

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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-26  2:17               ` Thavatchai Makphaibulchoke
@ 2015-03-26  5:23                 ` Mike Galbraith
  2015-03-26  6:01                   ` Mike Galbraith
  2015-03-26  6:27                   ` Mike Galbraith
  0 siblings, 2 replies; 25+ messages in thread
From: Mike Galbraith @ 2015-03-26  5:23 UTC (permalink / raw)
  To: Thavatchai Makphaibulchoke
  Cc: Steven Rostedt, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, Sebastian Andrzej Siewior, John Kacur,
	Paul Gortmaker

On Wed, 2015-03-25 at 20:17 -0600, Thavatchai Makphaibulchoke wrote:
> 
> On 03/22/2015 10:42 PM, Mike Galbraith wrote:
> >> Why can't we just Let swapper be the owner when in irq with no dummy?
> >>
> 
> Thanks Mike for the suggestion.  That may also work. Unfortunately
> somehow I'm still having a hung problem, which may be related to the
> priority of the interrupt handler task.
> 
> >> I have "don't raise timer unconditionally" re-applied, the check for a
> >> running callback bits of my nohz_full fixlet, and the below on top of
> >> that, and all _seems_ well.
> > 
> > But not so well on 64 core box.  That has nothing to do with hacklet
> > though, re-applying timers-do-not-raise-softirq-unconditionally.patch
> > without thta hangs the 64 core box during boot with no help from me
> > other than to patchlet to let nohz work at all, seems there's another
> > issue lurking there.  Hohum.  Without 'don't raise..", big box is fine.
> > 
> 
> If you get your patch to work, I could try my test that was able to
> reproduce the problem consistently.

If you had "don't raise timer unconditionally" applied, no surprise, my
big box hangs too with or without hacklet.  If didn't have it applied,
you don't need the hack.  If you had both and rtmutex debugging turned
on, posted version _should_ explode, as it didn't bother to side-step
that bit (et al).

I plan on taking a poke at getting "don't raise timer unconditionally"
working again when I get myself unburied, and see if I can come up with
a somewhat less icky way to work around take rtmutex in irq naughtiness.

	-Mike


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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-26  5:23                 ` Mike Galbraith
@ 2015-03-26  6:01                   ` Mike Galbraith
  2015-03-26  6:27                   ` Mike Galbraith
  1 sibling, 0 replies; 25+ messages in thread
From: Mike Galbraith @ 2015-03-26  6:01 UTC (permalink / raw)
  To: Thavatchai Makphaibulchoke
  Cc: Steven Rostedt, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, Sebastian Andrzej Siewior, John Kacur,
	Paul Gortmaker

On Thu, 2015-03-26 at 06:23 +0100, Mike Galbraith wrote:
> On Wed, 2015-03-25 at 20:17 -0600, Thavatchai Makphaibulchoke wrote:
> > 
> > On 03/22/2015 10:42 PM, Mike Galbraith wrote:
> > >> Why can't we just Let swapper be the owner when in irq with no dummy?
> > >>
> > 
> > Thanks Mike for the suggestion.  That may also work. Unfortunately
> > somehow I'm still having a hung problem, which may be related to the
> > priority of the interrupt handler task.
> > 
> > >> I have "don't raise timer unconditionally" re-applied, the check for a
> > >> running callback bits of my nohz_full fixlet, and the below on top of
> > >> that, and all _seems_ well.
> > > 
> > > But not so well on 64 core box.  That has nothing to do with hacklet
> > > though, re-applying timers-do-not-raise-softirq-unconditionally.patch
> > > without thta hangs the 64 core box during boot with no help from me
> > > other than to patchlet to let nohz work at all, seems there's another
> > > issue lurking there.  Hohum.  Without 'don't raise..", big box is fine.
> > > 
> > 
> > If you get your patch to work, I could try my test that was able to
> > reproduce the problem consistently.
> 
> If you had "don't raise timer unconditionally" applied, no surprise, my
> big box hangs too with or without hacklet.  If didn't have it applied,
> you don't need the hack.  If you had both and rtmutex debugging turned
> on, posted version _should_ explode, as it didn't bother to side-step
> that bit (et al).

Oh, and if you didn't have the below enabled, you'll certainly hang.

rt-nohz_full-fix-nohz_full-for-PREEMPT_RT_FULL.patch

	-Mike


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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-26  5:23                 ` Mike Galbraith
  2015-03-26  6:01                   ` Mike Galbraith
@ 2015-03-26  6:27                   ` Mike Galbraith
  2015-03-26 13:53                     ` Steven Rostedt
  1 sibling, 1 reply; 25+ messages in thread
From: Mike Galbraith @ 2015-03-26  6:27 UTC (permalink / raw)
  To: Thavatchai Makphaibulchoke
  Cc: Steven Rostedt, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, Sebastian Andrzej Siewior, John Kacur,
	Paul Gortmaker

On Thu, 2015-03-26 at 06:23 +0100, Mike Galbraith wrote:

> I plan on taking a poke at getting "don't raise timer unconditionally"
> working again when I get myself unburied, and see if I can come up with
> a somewhat less icky way to work around take rtmutex in irq naughtiness.

Hm.. like maybe only do a fasttrylock with the wait lock already held
via trylock, and don't bother turning it loose until we're done, to keep
the sane people away.  That might work.. but may not be considered less
icky by people equipped with that mysterious "taste" thingy ;-)

	-Mike


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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-26  6:27                   ` Mike Galbraith
@ 2015-03-26 13:53                     ` Steven Rostedt
  0 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2015-03-26 13:53 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Thavatchai Makphaibulchoke, linux-kernel, linux-rt-users,
	Thomas Gleixner, Carsten Emde, Sebastian Andrzej Siewior,
	John Kacur, Paul Gortmaker

On Thu, 26 Mar 2015 07:27:30 +0100
Mike Galbraith <umgwanakikbuti@gmail.com> wrote:

> On Thu, 2015-03-26 at 06:23 +0100, Mike Galbraith wrote:
> 
> > I plan on taking a poke at getting "don't raise timer unconditionally"
> > working again when I get myself unburied, and see if I can come up with
> > a somewhat less icky way to work around take rtmutex in irq naughtiness.
> 
> Hm.. like maybe only do a fasttrylock with the wait lock already held
> via trylock, and don't bother turning it loose until we're done, to keep
> the sane people away.  That might work.. but may not be considered less
> icky by people equipped with that mysterious "taste" thingy ;-)

You would still need to add some ownership so that all will fail the
fast path.

You mean create a spin_trylock_in_hirq() which would just lock
the waitlock and not even do the fast path with the rt_mutex.

	if (!raw_spin_trylock(waitlock))
		goto failed_lock;

	if (!try_to_take_rt_mutex()) {
		raw_spin_unlock(waitlock);
		goto failed_lock;
	}

	return success;


With the waitlock held, no slow path will get to the pi code. Then you
have a spin_unlock_in_hirq() that would go right into the slow path
assuming the waitlock is already held.

Sounds reasonable to me.

-- Steve


	

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

* Re: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-03-25  2:33           ` Mike Galbraith
@ 2015-04-09 13:39             ` Sebastian Andrzej Siewior
  2015-08-11 23:21               ` Jiang, Yunhong
  0 siblings, 1 reply; 25+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-04-09 13:39 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Steven Rostedt, linux-kernel, linux-rt-users, Thomas Gleixner,
	Carsten Emde, John Kacur, Paul Gortmaker

* Mike Galbraith | 2015-03-25 03:33:53 [+0100]:

>Nah, I was referring to get_next_timer_interrupt() because I saw that
>rt_spin_unlock_after_trylock_in_irq(&base->lock) sitting there.

Hmm. Good question. But it was Ingo who introduced the lock, so it might
have special Ingo magic included.

>	-Mike

Sebastian

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

* RE: [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally"
  2015-04-09 13:39             ` Sebastian Andrzej Siewior
@ 2015-08-11 23:21               ` Jiang, Yunhong
  0 siblings, 0 replies; 25+ messages in thread
From: Jiang, Yunhong @ 2015-08-11 23:21 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, Mike Galbraith; +Cc: Steven Rostedt, linux-rt-users

Hi, all
	Just want to check if anyone is working to bring the "timers: do not raise softirq unconditionally" back? Seems it's not in the 4.1.3-rt yet.

Thanks
--jyh

> -----Original Message-----
> From: linux-rt-users-owner@vger.kernel.org [mailto:linux-rt-users-
> owner@vger.kernel.org] On Behalf Of Sebastian Andrzej Siewior
> Sent: Thursday, April 9, 2015 6:39 AM
> To: Mike Galbraith
> Cc: Steven Rostedt; linux-kernel@vger.kernel.org; linux-rt-users; Thomas
> Gleixner; Carsten Emde; John Kacur; Gortmaker, Paul (Wind River)
> Subject: Re: [PATCH RT 2/4] Revert "timers: do not raise softirq
> unconditionally"
> 
> * Mike Galbraith | 2015-03-25 03:33:53 [+0100]:
> 
> >Nah, I was referring to get_next_timer_interrupt() because I saw that
> >rt_spin_unlock_after_trylock_in_irq(&base->lock) sitting there.
> 
> Hmm. Good question. But it was Ingo who introduced the lock, so it might
> have special Ingo magic included.
> 
> >	-Mike
> 
> Sebastian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2015-08-11 23:21 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-17 16:35 [PATCH RT 0/4] Linux 3.10.70-rt75-rc2 Steven Rostedt
2015-03-17 16:35 ` [PATCH RT 1/4] fs,btrfs: fix rt deadlock on extent_buffer->lock Steven Rostedt
2015-03-17 16:35 ` [PATCH RT 2/4] Revert "timers: do not raise softirq unconditionally" Steven Rostedt
2015-03-17 16:35   ` Steven Rostedt
2015-03-17 20:35   ` Steven Rostedt
2015-03-19  8:17     ` Mike Galbraith
2015-03-19 16:26       ` Steven Rostedt
2015-03-19 16:42         ` Thavatchai Makphaibulchoke
2015-03-21 18:02           ` Mike Galbraith
2015-03-23  4:42             ` Mike Galbraith
2015-03-26  2:17               ` Thavatchai Makphaibulchoke
2015-03-26  5:23                 ` Mike Galbraith
2015-03-26  6:01                   ` Mike Galbraith
2015-03-26  6:27                   ` Mike Galbraith
2015-03-26 13:53                     ` Steven Rostedt
2015-03-24 18:15             ` Sebastian Andrzej Siewior
2015-03-24 18:15               ` Sebastian Andrzej Siewior
2015-03-25  2:38               ` Mike Galbraith
2015-03-25  2:38                 ` Mike Galbraith
2015-03-24 18:10         ` Sebastian Andrzej Siewior
2015-03-25  2:33           ` Mike Galbraith
2015-04-09 13:39             ` Sebastian Andrzej Siewior
2015-08-11 23:21               ` Jiang, Yunhong
2015-03-17 16:35 ` [PATCH RT 3/4] netpoll: guard the access to dev->npinfo with rcu_read_lock/unlock_bh() for CONFIG_PREEMPT_RT_FULL=y Steven Rostedt
2015-03-17 16:35 ` [PATCH RT 4/4] Linux 3.10.70-rt75-rc2 Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.