From: Yong Zhang <yong.zhang0@gmail.com> To: Peter Zijlstra <peterz@infradead.org> Cc: Marc Zyngier <Marc.Zyngier@arm.com>, 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: Thu, 26 May 2011 15:29:23 +0800 [thread overview] Message-ID: <BANLkTi=XbTXQsu3jUEvQyCfBy6-aRnqSpw@mail.gmail.com> (raw) In-Reply-To: <1306358128.21578.107.camel@twins> On Thu, May 26, 2011 at 5:15 AM, Peter Zijlstra <peterz@infradead.org> wrote: > 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 Seems 'current' will change before/after switch_to since it's derived from sp register. So that means if interrupt come before we switch sp, 'p == current' will catch it, but if interrupt comes after we switch sp, we will lose a wake up. Thanks, Yong > 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(). > > > -- > 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
WARNING: multiple messages have this Message-ID (diff)
From: yong.zhang0@gmail.com (Yong Zhang) 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 15:29:23 +0800 [thread overview] Message-ID: <BANLkTi=XbTXQsu3jUEvQyCfBy6-aRnqSpw@mail.gmail.com> (raw) In-Reply-To: <1306358128.21578.107.camel@twins> On Thu, May 26, 2011 at 5:15 AM, Peter Zijlstra <peterz@infradead.org> wrote: > 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 Seems 'current' will change before/after switch_to since it's derived from sp register. So that means if interrupt come before we switch sp, 'p == current' will catch it, but if interrupt comes after we switch sp, we will lose a wake up. Thanks, Yong > 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(). > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at 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
next prev parent reply other threads:[~2011-05-26 7:29 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 [this message] 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='BANLkTi=XbTXQsu3jUEvQyCfBy6-aRnqSpw@mail.gmail.com' \ --to=yong.zhang0@gmail.com \ --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=peterz@infradead.org \ /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.