From patchwork Thu Jun 20 10:08:58 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Kleine-Budde X-Patchwork-Id: 2754391 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 7ACDA9F39E for ; Thu, 20 Jun 2013 10:10:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 71AF320455 for ; Thu, 20 Jun 2013 10:09:56 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6E83820451 for ; Thu, 20 Jun 2013 10:09:51 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UpboH-0005i1-Tn; Thu, 20 Jun 2013 10:09:46 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UpboF-0001PP-Da; Thu, 20 Jun 2013 10:09:43 +0000 Received: from metis.ext.pengutronix.de ([2001:6f8:1178:4:290:27ff:fe1d:cc33]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Upbo8-0001Od-Iv for linux-arm-kernel@lists.infradead.org; Thu, 20 Jun 2013 10:09:41 +0000 Received: from gallifrey.ext.pengutronix.de ([2001:6f8:1178:4:5054:ff:fe8d:eefb] helo=bjornoya.do.blackshift.org) by metis.ext.pengutronix.de with esmtp (Exim 4.72) (envelope-from ) id 1Upbni-0007Dc-49; Thu, 20 Jun 2013 12:09:10 +0200 Received: from [172.17.34.65] (hardanger.do.blackshift.org [172.17.34.65]) (using TLSv1 with cipher ECDHE-ECDSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) (Authenticated sender: frogger) by bjornoya.do.blackshift.org (Postfix) with ESMTPSA id A252C5E828; Thu, 20 Jun 2013 12:09:09 +0200 (CEST) Message-ID: <51C2D4BA.8020406@pengutronix.de> Date: Thu, 20 Jun 2013 12:08:58 +0200 From: Marc Kleine-Budde User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130518 Icedove/17.0.5 MIME-Version: 1.0 To: Catalin Marinas Subject: Re: BUG: commit "ARM: Remove __ARCH_WANT_INTERRUPTS_ON_CTXSW on pre-ARMv6 CPUs" breaks armv5 with CONFIG_PREEMPT References: <51C2C0B5.8020802@pengutronix.de> <20130620095705.GA18536@arm.com> In-Reply-To: <20130620095705.GA18536@arm.com> X-Enigmail-Version: 1.5.1 X-SA-Exim-Connect-IP: 2001:6f8:1178:4:5054:ff:fe8d:eefb X-SA-Exim-Mail-From: mkl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130620_060940_807757_54F9DC8D X-CRM114-Status: GOOD ( 34.11 ) X-Spam-Score: -3.2 (---) Cc: linux-arm-kernel , "kernel@pengutronix.de" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 06/20/2013 11:57 AM, Catalin Marinas wrote: > Hi Marc, > > On Thu, Jun 20, 2013 at 09:43:33AM +0100, Marc Kleine-Budde wrote: >> on current linus/master on armv5 we observed stack trashing[1] on high >> load. I bisected this problem down to commit: >> >>> b9d4d42ad901cc848ac87f1cb8923fded3645568 is the first bad commit >>> commit b9d4d42ad901cc848ac87f1cb8923fded3645568 >>> Author: Catalin Marinas >>> Date: Mon Nov 28 21:57:24 2011 +0000 >>> >>> ARM: Remove __ARCH_WANT_INTERRUPTS_ON_CTXSW on pre-ARMv6 CPUs >>> >>> This patch removes the __ARCH_WANT_INTERRUPTS_ON_CTXSW definition for >>> ARMv5 and earlier processors. On such processors, the context switch >>> requires a full cache flush. To avoid high interrupt latencies, this >>> patch defers the mm switching to the post-lock switch hook if the >>> interrupts are disabled. >>> >>> Reviewed-by: Will Deacon >>> Tested-by: Will Deacon >>> Reviewed-by: Frank Rowand >>> Tested-by: Marc Zyngier >>> Signed-off-by: Catalin Marinas >>> >>> :040000 040000 034899bdcbc9aa59b5455a85a9d78b646b4cf784 ecc23e33a4ca807d4153f87fbea85a9437ff2928 M arch >> >> The problem can be reproduced on several mx28 and an at91sam9263 and >> only occurs of CONFIG_PREEMPT (Preemptible Kernel (Low-Latency Desktop)) >> is enabled. >> >> I have the gut feeling that the "if (irqs_disabled())" check in the >> above patch is not correct for CONFIG_PREEMPT. > > The check is there to avoid long interrupt latencies (flushing the whole > cache with interrupts disabled during context switch). You can drop the > check and always call cpu_switch_mm() to confirm that it fixes the > faults. I have disabled the check: ...and the test is running without problems, while I'm writing this email. No more segfaults so far. > finish_task_switch() calls finish_arch_post_lock_switch() after the > interrupts have been enabled so that the CPU can actually switch the mm. > I wonder whether we could actually be preempted after > finish_lock_switch() but before we actually switched the MMU. > > Here's an untested patch (trying to keep it in the arch/arm code): > > > diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h > index a7b85e0..ded85e9 100644 > --- a/arch/arm/include/asm/mmu_context.h > +++ b/arch/arm/include/asm/mmu_context.h > @@ -39,17 +39,20 @@ static inline void check_and_switch_context(struct mm_struct *mm, > if (unlikely(mm->context.vmalloc_seq != init_mm.context.vmalloc_seq)) > __check_vmalloc_seq(mm); > > - if (irqs_disabled()) > + if (irqs_disabled()) { > /* > * cpu_switch_mm() needs to flush the VIVT caches. To avoid > * high interrupt latencies, defer the call and continue > * running with the old mm. Since we only support UP systems > * on non-ASID CPUs, the old mm will remain valid until the > - * finish_arch_post_lock_switch() call. > + * finish_arch_post_lock_switch() call. Preemption needs to be > + * disabled until the MMU is switched. > */ > set_ti_thread_flag(task_thread_info(tsk), TIF_SWITCH_MM); > - else > + preempt_disable(); > + } else { > cpu_switch_mm(mm->pgd, mm); > + } > } > > #define finish_arch_post_lock_switch \ > @@ -59,6 +62,10 @@ static inline void finish_arch_post_lock_switch(void) > if (test_and_clear_thread_flag(TIF_SWITCH_MM)) { > struct mm_struct *mm = current->mm; > cpu_switch_mm(mm->pgd, mm); > + /* > + * Preemption disabled in check_and_switch_context(). > + */ > + preempt_enable(); > } > } Will now reboot to test your patch. thanks, Marc diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h index a7b85e0..7b3f67f 100644 --- a/arch/arm/include/asm/mmu_context.h +++ b/arch/arm/include/asm/mmu_context.h @@ -39,7 +39,7 @@ static inline void check_and_switch_context(struct mm_struct *mm, if (unlikely(mm->context.vmalloc_seq != init_mm.context.vmalloc_seq)) __check_vmalloc_seq(mm); - if (irqs_disabled()) + if (0 && irqs_disabled()) /* * cpu_switch_mm() needs to flush the VIVT caches. To avoid * high interrupt latencies, defer the call and continue