From: Peter Zijlstra <peterz@infradead.org> To: Russell King - ARM Linux <linux@arm.linux.org.uk> Cc: Ingo Molnar <mingo@elte.hu>, Catalin Marinas <catalin.marinas@arm.com>, Marc Zyngier <Marc.Zyngier@arm.com>, Frank Rowand <frank.rowand@am.sony.com>, Oleg Nesterov <oleg@redhat.com>, linux-kernel@vger.kernel.org, Yong Zhang <yong.zhang0@gmail.com>, linux-arm-kernel@lists.infradead.org, Michal Simek <monstr@monstr.eu> Subject: Re: [BUG] "sched: Remove rq->lock from the first half of ttwu()" locks up on ARM Date: Sat, 28 May 2011 15:13:01 +0200 [thread overview] Message-ID: <1306588381.2497.481.camel@laptop> (raw) In-Reply-To: <20110527205240.GT24876@n2100.arm.linux.org.uk> On Fri, 2011-05-27 at 21:52 +0100, Russell King - ARM Linux wrote: > On Fri, May 27, 2011 at 02:06:29PM +0200, Ingo Molnar wrote: > > The expectations are to have irqs off (we are holding the runqueue > > lock if !__ARCH_WANT_INTERRUPTS_ON_CTXSW), so that's not workable i > > suspect. > > Just a thought, but we _might_ be able to avoid a lot of this hastle if > we had a new arch hook in finish_task_switch(), after finish_lock_switch() > returns but before the old MM is dropped. I'd be more than willing to provide this. > For the new ASID-based switch_mm(), we currently do this: > > 1. check ASID validity > 2. flush branch predictor > 3. set reserved ASID value > 4. set new page tables > 5. set new ASID value > > This will be shortly changed to: > > 1. check ASID validity > 2. flush branch predictor > 3. set swapper_pg_dir tables > 4. set new ASID value > 5. set new page tables > > We could change switch_mm() to only do: > > 1. flush branch predictor > 2. set swapper_pg_dir tables > 3. check ASID validity > 4. set new ASID value > > At this point, we have no user mappings, and so nothing will be using the > ASID at this point. Then in a new post-finish_lock_switch() arch hook: > > 5. check whether we need to do flushing as a result of ASID change > 6. set new page tables > > I think this may simplify the ASID code. It needs prototyping out, > reviewing and testing, but I think it may work. > > And I think it may also be workable with the CPUs which need to flush > the caches on context switches - we can postpone their page table > switch to this new arch hook too, which will mean we wouldn't require > __ARCH_WANT_INTERRUPTS_ON_CTXSW on ARM at all. > > Any thoughts (if you've followed what I'm going on about) ? Yeah, definitely worth a try, you mentioned on IRC the problem of detecting if switch_mm() happened in the new arch hook. Since switch_mm() gets a @next pointer we can set a TIF flag there and have the new arch hook test for that and conditionally perform the required work. Now, supposing we can get ARM to not rely on __ARCH_WANT_INTERRUPTS_ON_CTXSW anymore, there's only microblaze left, Michal, would a similar scheme work for you? If so we can fully deprecate and remove this exception from the scheduler (yay!).
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: Sat, 28 May 2011 15:13:01 +0200 [thread overview] Message-ID: <1306588381.2497.481.camel@laptop> (raw) In-Reply-To: <20110527205240.GT24876@n2100.arm.linux.org.uk> On Fri, 2011-05-27 at 21:52 +0100, Russell King - ARM Linux wrote: > On Fri, May 27, 2011 at 02:06:29PM +0200, Ingo Molnar wrote: > > The expectations are to have irqs off (we are holding the runqueue > > lock if !__ARCH_WANT_INTERRUPTS_ON_CTXSW), so that's not workable i > > suspect. > > Just a thought, but we _might_ be able to avoid a lot of this hastle if > we had a new arch hook in finish_task_switch(), after finish_lock_switch() > returns but before the old MM is dropped. I'd be more than willing to provide this. > For the new ASID-based switch_mm(), we currently do this: > > 1. check ASID validity > 2. flush branch predictor > 3. set reserved ASID value > 4. set new page tables > 5. set new ASID value > > This will be shortly changed to: > > 1. check ASID validity > 2. flush branch predictor > 3. set swapper_pg_dir tables > 4. set new ASID value > 5. set new page tables > > We could change switch_mm() to only do: > > 1. flush branch predictor > 2. set swapper_pg_dir tables > 3. check ASID validity > 4. set new ASID value > > At this point, we have no user mappings, and so nothing will be using the > ASID at this point. Then in a new post-finish_lock_switch() arch hook: > > 5. check whether we need to do flushing as a result of ASID change > 6. set new page tables > > I think this may simplify the ASID code. It needs prototyping out, > reviewing and testing, but I think it may work. > > And I think it may also be workable with the CPUs which need to flush > the caches on context switches - we can postpone their page table > switch to this new arch hook too, which will mean we wouldn't require > __ARCH_WANT_INTERRUPTS_ON_CTXSW on ARM at all. > > Any thoughts (if you've followed what I'm going on about) ? Yeah, definitely worth a try, you mentioned on IRC the problem of detecting if switch_mm() happened in the new arch hook. Since switch_mm() gets a @next pointer we can set a TIF flag there and have the new arch hook test for that and conditionally perform the required work. Now, supposing we can get ARM to not rely on __ARCH_WANT_INTERRUPTS_ON_CTXSW anymore, there's only microblaze left, Michal, would a similar scheme work for you? If so we can fully deprecate and remove this exception from the scheduler (yay!).
next prev parent reply other threads:[~2011-05-28 13:09 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 [this message] 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=1306588381.2497.481.camel@laptop \ --to=peterz@infradead.org \ --cc=Marc.Zyngier@arm.com \ --cc=catalin.marinas@arm.com \ --cc=frank.rowand@am.sony.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=mingo@elte.hu \ --cc=monstr@monstr.eu \ --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.