All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Marc Zyngier <Marc.Zyngier@arm.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	Frank Rowand <frank.rowand@am.sony.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Oleg Nesterov <oleg@redhat.com>
Subject: Re: [BUG]  "sched: Remove rq->lock from the first half of ttwu()"  locks up on ARM
Date: Wed, 25 May 2011 23:15:28 +0200	[thread overview]
Message-ID: <1306358128.21578.107.camel@twins> (raw)
In-Reply-To: <1306343335.21578.65.camel@twins>

On Wed, 2011-05-25 at 19:08 +0200, Peter Zijlstra wrote:
> Ooh, shiny, whilst typing this I got an NMI-watchdog error reporting me
> that CPU1 got stuck in try_to_wake_up(), so it looks like I can indeed
> reproduce some funnies.
> 
> /me goes dig in. 

Does the below make your ARM box happy again?

It restores the old ttwu behaviour for this case and seems to not mess
up my x86 with __ARCH_WANT_INTERRUPTS_ON_CTXSW.

Figuring out why the existing condition failed and writing a proper
changelog requires a mind that is slightly less deprived of sleep and I
shall attempt that tomorrow -- provided this does indeed work for you.

---
diff --git a/kernel/sched.c b/kernel/sched.c
index 2d12893..6976eac 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2573,7 +2573,19 @@ static void ttwu_queue_remote(struct task_struct *p, int cpu)
 	if (!next)
 		smp_send_reschedule(cpu);
 }
-#endif
+
+#ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW
+static void ttwu_activate_remote(struct task_struct *p, int wake_flags)
+{
+	struct rq *rq = __task_rq_lock(p);
+
+	ttwu_activate(rq, p, ENQUEUE_WAKEUP | ENQUEUE_WAKING);
+	ttwu_do_wakeup(rq, p, wake_flags);
+
+	__task_rq_unlock(rq);
+}
+#endif /* __ARCH_WANT_INTERRUPTS_ON_CTXSW */
+#endif /* CONFIG_SMP */
 
 static void ttwu_queue(struct task_struct *p, int cpu)
 {
@@ -2630,18 +2642,11 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 	 */
 	while (p->on_cpu) {
 #ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW
-		/*
-		 * If called from interrupt context we could have landed in the
-		 * middle of schedule(), in this case we should take care not
-		 * to spin on ->on_cpu if p is current, since that would
-		 * deadlock.
-		 */
-		if (p == current) {
-			ttwu_queue(p, cpu);
-			goto stat;
-		}
-#endif
+		ttwu_activate_remote(p, wake_flags);
+		goto stat;
+#else
 		cpu_relax();
+#endif
 	}
 	/*
 	 * Pairs with the smp_wmb() in finish_lock_switch().



WARNING: multiple messages have this Message-ID (diff)
From: peterz@infradead.org (Peter Zijlstra)
To: linux-arm-kernel@lists.infradead.org
Subject: [BUG]  "sched: Remove rq->lock from the first half of ttwu()" locks up on ARM
Date: Wed, 25 May 2011 23:15:28 +0200	[thread overview]
Message-ID: <1306358128.21578.107.camel@twins> (raw)
In-Reply-To: <1306343335.21578.65.camel@twins>

On Wed, 2011-05-25 at 19:08 +0200, Peter Zijlstra wrote:
> Ooh, shiny, whilst typing this I got an NMI-watchdog error reporting me
> that CPU1 got stuck in try_to_wake_up(), so it looks like I can indeed
> reproduce some funnies.
> 
> /me goes dig in. 

Does the below make your ARM box happy again?

It restores the old ttwu behaviour for this case and seems to not mess
up my x86 with __ARCH_WANT_INTERRUPTS_ON_CTXSW.

Figuring out why the existing condition failed and writing a proper
changelog requires a mind that is slightly less deprived of sleep and I
shall attempt that tomorrow -- provided this does indeed work for you.

---
diff --git a/kernel/sched.c b/kernel/sched.c
index 2d12893..6976eac 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2573,7 +2573,19 @@ static void ttwu_queue_remote(struct task_struct *p, int cpu)
 	if (!next)
 		smp_send_reschedule(cpu);
 }
-#endif
+
+#ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW
+static void ttwu_activate_remote(struct task_struct *p, int wake_flags)
+{
+	struct rq *rq = __task_rq_lock(p);
+
+	ttwu_activate(rq, p, ENQUEUE_WAKEUP | ENQUEUE_WAKING);
+	ttwu_do_wakeup(rq, p, wake_flags);
+
+	__task_rq_unlock(rq);
+}
+#endif /* __ARCH_WANT_INTERRUPTS_ON_CTXSW */
+#endif /* CONFIG_SMP */
 
 static void ttwu_queue(struct task_struct *p, int cpu)
 {
@@ -2630,18 +2642,11 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 	 */
 	while (p->on_cpu) {
 #ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW
-		/*
-		 * If called from interrupt context we could have landed in the
-		 * middle of schedule(), in this case we should take care not
-		 * to spin on ->on_cpu if p is current, since that would
-		 * deadlock.
-		 */
-		if (p == current) {
-			ttwu_queue(p, cpu);
-			goto stat;
-		}
-#endif
+		ttwu_activate_remote(p, wake_flags);
+		goto stat;
+#else
 		cpu_relax();
+#endif
 	}
 	/*
 	 * Pairs with the smp_wmb() in finish_lock_switch().

  reply	other threads:[~2011-05-25 21:16 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-24 18:13 [BUG] "sched: Remove rq->lock from the first half of ttwu()" locks up on ARM Marc Zyngier
2011-05-24 18:13 ` Marc Zyngier
2011-05-24 21:32 ` Peter Zijlstra
2011-05-24 21:32   ` Peter Zijlstra
2011-05-24 21:39   ` Ingo Molnar
2011-05-24 21:39     ` Ingo Molnar
2011-05-25 12:23     ` Marc Zyngier
2011-05-25 12:23       ` Marc Zyngier
2011-05-25 17:08   ` Peter Zijlstra
2011-05-25 17:08     ` Peter Zijlstra
2011-05-25 21:15     ` Peter Zijlstra [this message]
2011-05-25 21:15       ` Peter Zijlstra
2011-05-26  7:29       ` Yong Zhang
2011-05-26  7:29         ` Yong Zhang
2011-05-26 10:32         ` Peter Zijlstra
2011-05-26 10:32           ` Peter Zijlstra
2011-05-26 11:02           ` Marc Zyngier
2011-05-26 11:02             ` Marc Zyngier
2011-05-26 11:32             ` Peter Zijlstra
2011-05-26 11:32               ` Peter Zijlstra
2011-05-26 12:21               ` Peter Zijlstra
2011-05-26 12:21                 ` Peter Zijlstra
2011-05-26 12:26                 ` Ingo Molnar
2011-05-26 12:26                   ` Ingo Molnar
2011-05-26 12:31                   ` Russell King - ARM Linux
2011-05-26 12:31                     ` Russell King - ARM Linux
2011-05-26 12:37                     ` Peter Zijlstra
2011-05-26 12:37                       ` Peter Zijlstra
2011-05-26 12:50                     ` Ingo Molnar
2011-05-26 12:50                       ` Ingo Molnar
2011-05-26 13:36                       ` Russell King - ARM Linux
2011-05-26 13:36                         ` Russell King - ARM Linux
2011-05-26 14:45                       ` Catalin Marinas
2011-05-26 14:45                         ` Catalin Marinas
2011-05-27 12:06                         ` Ingo Molnar
2011-05-27 12:06                           ` Ingo Molnar
2011-05-27 17:55                           ` Russell King - ARM Linux
2011-05-27 17:55                             ` Russell King - ARM Linux
2011-05-27 19:41                           ` Nicolas Pitre
2011-05-27 19:41                             ` Nicolas Pitre
2011-05-27 20:52                           ` Russell King - ARM Linux
2011-05-27 20:52                             ` Russell King - ARM Linux
2011-05-28 13:13                             ` Peter Zijlstra
2011-05-28 13:13                               ` Peter Zijlstra
2011-05-31 11:08                               ` Michal Simek
2011-05-31 11:08                                 ` Michal Simek
2011-05-31 13:22                                 ` Peter Zijlstra
2011-05-31 13:22                                   ` Peter Zijlstra
2011-05-31 13:37                                   ` Michal Simek
2011-05-31 13:37                                     ` Michal Simek
2011-05-31 13:52                                     ` Peter Zijlstra
2011-05-31 13:52                                       ` Peter Zijlstra
2011-05-31 14:08                                       ` Michal Simek
2011-05-31 14:08                                         ` Michal Simek
2011-05-31 14:29                                         ` Peter Zijlstra
2011-05-31 14:29                                           ` Peter Zijlstra
2011-05-29 10:21                             ` Catalin Marinas
2011-05-29 10:21                               ` Catalin Marinas
2011-05-29 10:26                               ` Russell King - ARM Linux
2011-05-29 10:26                                 ` Russell King - ARM Linux
2011-05-29 12:01                                 ` Catalin Marinas
2011-05-29 12:01                                   ` Catalin Marinas
2011-05-29 13:19                                   ` Russell King - ARM Linux
2011-05-29 13:19                                     ` Russell King - ARM Linux
2011-05-29 21:21                                     ` Catalin Marinas
2011-05-29 21:21                                       ` Catalin Marinas
2011-05-29  9:51                           ` Catalin Marinas
2011-05-29  9:51                             ` Catalin Marinas
2011-06-06 10:29                           ` Pavel Machek
2011-06-06 10:29                             ` Pavel Machek
2011-05-26 14:56                 ` Marc Zyngier
2011-05-26 14:56                   ` Marc Zyngier
2011-05-26 15:45                 ` Oleg Nesterov
2011-05-26 15:45                   ` Oleg Nesterov
2011-05-26 15:59                   ` Peter Zijlstra
2011-05-26 15:59                     ` Peter Zijlstra
2011-05-26 16:09                     ` Peter Zijlstra
2011-05-26 16:09                       ` Peter Zijlstra
2011-05-26 16:20                       ` Marc Zyngier
2011-05-26 16:20                         ` Marc Zyngier
2011-05-26 16:32                         ` Peter Zijlstra
2011-05-26 16:32                           ` Peter Zijlstra
2011-05-27  8:01                           ` Marc Zyngier
2011-05-27  8:01                             ` Marc Zyngier
2011-05-26 16:22                       ` Marc Zyngier
2011-05-26 16:22                         ` Marc Zyngier
2011-05-26 17:04                       ` Oleg Nesterov
2011-05-26 17:04                         ` Oleg Nesterov
2011-05-26 17:17                         ` Peter Zijlstra
2011-05-26 17:17                           ` Peter Zijlstra
2011-05-26 17:23                           ` Peter Zijlstra
2011-05-26 17:23                             ` Peter Zijlstra
2011-05-26 17:49                             ` Oleg Nesterov
2011-05-26 17:49                               ` Oleg Nesterov
2011-05-27  7:01                             ` Yong Zhang
2011-05-27  7:01                               ` Yong Zhang
2011-05-27 15:23                             ` Santosh Shilimkar
2011-05-27 15:23                               ` Santosh Shilimkar
2011-05-27 15:29                               ` Marc Zyngier
2011-05-27 15:29                                 ` Marc Zyngier
2011-05-27 15:30                                 ` Santosh Shilimkar
2011-05-27 15:30                                   ` Santosh Shilimkar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1306358128.21578.107.camel@twins \
    --to=peterz@infradead.org \
    --cc=Marc.Zyngier@arm.com \
    --cc=frank.rowand@am.sony.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oleg@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.