From: Peter Zijlstra <peterz@infradead.org> To: Oleg Nesterov <oleg@redhat.com> Cc: Marc Zyngier <Marc.Zyngier@arm.com>, Yong Zhang <yong.zhang0@gmail.com>, Ingo Molnar <mingo@elte.hu>, Frank Rowand <frank.rowand@am.sony.com>, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [BUG] "sched: Remove rq->lock from the first half of ttwu()" locks up on ARM Date: Thu, 26 May 2011 19:23:53 +0200 [thread overview] Message-ID: <1306430633.2497.91.camel@laptop> (raw) In-Reply-To: <1306430264.2497.88.camel@laptop> On Thu, 2011-05-26 at 19:17 +0200, Peter Zijlstra wrote: > On Thu, 2011-05-26 at 19:04 +0200, Oleg Nesterov wrote: > > On 05/26, Peter Zijlstra wrote: > > > > > > @@ -2636,7 +2636,8 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags) > > > * to spin on ->on_cpu if p is current, since that would > > > * deadlock. > > > */ > > > - if (p == current) { > > > + if (cpu == smp_processor_id()) { > > > + p->sched_contributes_to_load = 0; > > > ttwu_queue(p, cpu); > > > > Btw. I do not pretend I really understand se->vruntime, but in this > > case we are doing enqueue_task() without ->task_waking(), however we > > pass ENQUEUE_WAKING. Is it correct? > > No its not, that's the thing that I got wrong the first time and caused > these pauses. We'd end up with something like the below, which isn't too different from what I've now got queued. It has the extra cpu == smp_processor_id() check, but I'm not sure this whole case is worth the trouble. I could go stick some counters in to verify how often all this happens I guess. --- arch/x86/include/asm/system.h | 2 ++ kernel/sched.c | 14 +++++++++++--- kernel/sched_debug.c | 7 +++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h index c2ff2a1..2c597e8 100644 --- a/arch/x86/include/asm/system.h +++ b/arch/x86/include/asm/system.h @@ -10,6 +10,8 @@ #include <linux/kernel.h> #include <linux/irqflags.h> +#define __ARCH_WANT_INTERRUPTS_ON_CTXSW + /* entries in ARCH_DLINFO: */ #if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64) # define AT_VECTOR_SIZE_ARCH 2 diff --git a/kernel/sched.c b/kernel/sched.c index 2d12893..e4f7a9f 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -2636,9 +2636,17 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags) * to spin on ->on_cpu if p is current, since that would * deadlock. */ - if (p == current) { - ttwu_queue(p, cpu); - goto stat; + if (cpu == smp_processor_id()) { + struct rq *rq; + + rq = __task_rq_lock(p); + if (p->on_cpu) { + ttwu_activate(rq, p, ENQUEUE_WAKEUP); + ttwu_do_wakeup(rq, p, wake_flags); + __task_rq_unlock(rq); + goto stat; + } + __task_rq_unlock(rq); } #endif cpu_relax(); diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c index a6710a1..f0ff1de 100644 --- a/kernel/sched_debug.c +++ b/kernel/sched_debug.c @@ -332,6 +332,13 @@ static int sched_debug_show(struct seq_file *m, void *v) (int)strcspn(init_utsname()->version, " "), init_utsname()->version); +#ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW + SEQ_printf(m, "__ARCH_WANT_INTERRUPTS_ON_CTXSW\n"); +#endif +#ifdef __ARCH_WANT_UNLOCKED_CTXSW + SEQ_printf(m, "__ARCH_WANT_UNLOCKED_CTXSW\n"); +#endif + #define P(x) \ SEQ_printf(m, "%-40s: %Ld\n", #x, (long long)(x)) #define PN(x) \
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: Thu, 26 May 2011 19:23:53 +0200 [thread overview] Message-ID: <1306430633.2497.91.camel@laptop> (raw) In-Reply-To: <1306430264.2497.88.camel@laptop> On Thu, 2011-05-26 at 19:17 +0200, Peter Zijlstra wrote: > On Thu, 2011-05-26 at 19:04 +0200, Oleg Nesterov wrote: > > On 05/26, Peter Zijlstra wrote: > > > > > > @@ -2636,7 +2636,8 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags) > > > * to spin on ->on_cpu if p is current, since that would > > > * deadlock. > > > */ > > > - if (p == current) { > > > + if (cpu == smp_processor_id()) { > > > + p->sched_contributes_to_load = 0; > > > ttwu_queue(p, cpu); > > > > Btw. I do not pretend I really understand se->vruntime, but in this > > case we are doing enqueue_task() without ->task_waking(), however we > > pass ENQUEUE_WAKING. Is it correct? > > No its not, that's the thing that I got wrong the first time and caused > these pauses. We'd end up with something like the below, which isn't too different from what I've now got queued. It has the extra cpu == smp_processor_id() check, but I'm not sure this whole case is worth the trouble. I could go stick some counters in to verify how often all this happens I guess. --- arch/x86/include/asm/system.h | 2 ++ kernel/sched.c | 14 +++++++++++--- kernel/sched_debug.c | 7 +++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h index c2ff2a1..2c597e8 100644 --- a/arch/x86/include/asm/system.h +++ b/arch/x86/include/asm/system.h @@ -10,6 +10,8 @@ #include <linux/kernel.h> #include <linux/irqflags.h> +#define __ARCH_WANT_INTERRUPTS_ON_CTXSW + /* entries in ARCH_DLINFO: */ #if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64) # define AT_VECTOR_SIZE_ARCH 2 diff --git a/kernel/sched.c b/kernel/sched.c index 2d12893..e4f7a9f 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -2636,9 +2636,17 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags) * to spin on ->on_cpu if p is current, since that would * deadlock. */ - if (p == current) { - ttwu_queue(p, cpu); - goto stat; + if (cpu == smp_processor_id()) { + struct rq *rq; + + rq = __task_rq_lock(p); + if (p->on_cpu) { + ttwu_activate(rq, p, ENQUEUE_WAKEUP); + ttwu_do_wakeup(rq, p, wake_flags); + __task_rq_unlock(rq); + goto stat; + } + __task_rq_unlock(rq); } #endif cpu_relax(); diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c index a6710a1..f0ff1de 100644 --- a/kernel/sched_debug.c +++ b/kernel/sched_debug.c @@ -332,6 +332,13 @@ static int sched_debug_show(struct seq_file *m, void *v) (int)strcspn(init_utsname()->version, " "), init_utsname()->version); +#ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW + SEQ_printf(m, "__ARCH_WANT_INTERRUPTS_ON_CTXSW\n"); +#endif +#ifdef __ARCH_WANT_UNLOCKED_CTXSW + SEQ_printf(m, "__ARCH_WANT_UNLOCKED_CTXSW\n"); +#endif + #define P(x) \ SEQ_printf(m, "%-40s: %Ld\n", #x, (long long)(x)) #define PN(x) \
next prev parent reply other threads:[~2011-05-26 17:21 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 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 [this message] 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=1306430633.2497.91.camel@laptop \ --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 \ --cc=yong.zhang0@gmail.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: linkBe 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.