From patchwork Tue Dec 18 23:54:12 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 1894001 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 468D6DF2F6 for ; Tue, 18 Dec 2012 23:57:15 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Tl6zL-0005me-RJ; Tue, 18 Dec 2012 23:54:19 +0000 Received: from wolverine02.qualcomm.com ([199.106.114.251]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1Tl6zI-0005mL-4b for linux-arm-kernel@lists.infradead.org; Tue, 18 Dec 2012 23:54:16 +0000 X-IronPort-AV: E=Sophos;i="4.84,311,1355126400"; d="scan'208";a="15118136" Received: from pdmz-ns-snip_114_130.qualcomm.com (HELO mostmsg01.qualcomm.com) ([199.106.114.130]) by wolverine02.qualcomm.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 18 Dec 2012 15:54:12 -0800 Received: from [10.46.166.8] (pdmz-ns-snip_218_1.qualcomm.com [192.168.218.1]) by mostmsg01.qualcomm.com (Postfix) with ESMTPA id 9C9F710004B4; Tue, 18 Dec 2012 15:54:12 -0800 (PST) Message-ID: <50D10224.30600@codeaurora.org> Date: Tue, 18 Dec 2012 15:54:12 -0800 From: Stephen Boyd User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: "Patrik, Kluba" Subject: Re: Fw: [PATCH 01/01] arm: fix a preempt_count() corruption for CONFIG_PREEMPT=n References: <20121215085129.71d203b2.pkluba@dension.com> In-Reply-To: <20121215085129.71d203b2.pkluba@dension.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20121218_185416_397452_98FD02B4 X-CRM114-Status: GOOD ( 20.07 ) X-Spam-Score: -4.2 (----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-4.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [199.106.114.251 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On 12/14/12 23:51, Patrik, Kluba wrote: > Date: Fri, 14 Dec 2012 15:13:57 +0100 > From: "Patrik, Kluba" > To: linux-kernel@vger.kernel.org > Subject: [PATCH 01/01] arm: fix a preempt_count() corruption for > CONFIG_PREEMPT=n > > > Hi All, > > After a hard days' work, I've managed to track down a ghost causing > sporadic kernel warnings and system crashes. The patch applies for the > HEAD of linux-stable. I don't know how to create and submit patches > correctly, and hope that somebody will pick it up... > > Regards, > Patrik > --- > From 4dd31e88a2715389e5ac198dcf00b48b4f148ea6 Mon Sep 17 00:00:00 2001 > From: Patrik Kluba > Date: Fri, 14 Dec 2012 14:36:27 +0100 > Subject: [PATCH] arm: fix a preempt_count() corruption for > CONFIG_PREEMPT=n > > All the preempt-disabling assembly code is under CONFIG_PREEMPT, > while VFP_bounce calls preempt_enable() uncoditionally, leading to > a preempt_count() corruption (gets set to 0xffffffff). Fix the > imbalance by adding #ifdef CONFIG_PREEMPT before preempt_enable(). > --- > arch/arm/vfp/vfpmodule.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > index 3b44e0d..2184f7e 100644 > --- a/arch/arm/vfp/vfpmodule.c > +++ b/arch/arm/vfp/vfpmodule.c > @@ -428,7 +428,10 @@ void VFP_bounce(u32 trigger, u32 fpexc, struct > pt_regs *regs) if (exceptions) > vfp_raise_exceptions(exceptions, trigger, orig_fpscr, > regs); exit: > +#ifdef CONFIG_PREEMPT > preempt_enable(); > +#endif > + return; From include/linux/preempt.h #ifdef CONFIG_PREEMPT_COUNT ... #define preempt_enable() \ do { \ preempt_enable_no_resched(); \ barrier(); \ preempt_check_resched(); \ } while (0) ... #else /* !CONFIG_PREEMPT_COUNT */ ... #define preempt_enable() do { } while (0) ... #endif And CONFIG_PREEMPT_COUNT=y only if CONFIG_PREEMPT=y but your subject says PREEMPT_COUNT=n. Do you by chance have DEBUG_ATOMIC_SLEEP=y selected in your config? That seems to select PREEMPT_COUNT as well and so I believe we need to move ARM assembly to use PREEMPT_COUNT instead of plain PREEMPT in its preempt_count changing assembly code. Can you try this patch? I'll probably send a follow up patch that consolidates the duplicated preempt_decrement code into some assembly macro. We may also want to fix it up so that assembly stubs call into the preempt tracer. I'm not sure how that works right now with VFP. ---->8------ Tested-by: Patrik Kluba diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S index cc926c9..323ce1a 100644 --- a/arch/arm/vfp/entry.S +++ b/arch/arm/vfp/entry.S @@ -22,7 +22,7 @@ @ IRQs disabled. @ ENTRY(do_vfp) -#ifdef CONFIG_PREEMPT +#ifdef CONFIG_PREEMPT_COUNT ldr r4, [r10, #TI_PREEMPT] @ get preempt count add r11, r4, #1 @ increment it str r11, [r10, #TI_PREEMPT] @@ -35,7 +35,7 @@ ENTRY(do_vfp) ENDPROC(do_vfp) ENTRY(vfp_null_entry) -#ifdef CONFIG_PREEMPT +#ifdef CONFIG_PREEMPT_COUNT get_thread_info r10 ldr r4, [r10, #TI_PREEMPT] @ get preempt count sub r11, r4, #1 @ decrement it @@ -53,7 +53,7 @@ ENDPROC(vfp_null_entry) __INIT ENTRY(vfp_testing_entry) -#ifdef CONFIG_PREEMPT +#ifdef CONFIG_PREEMPT_COUNT get_thread_info r10 ldr r4, [r10, #TI_PREEMPT] @ get preempt count sub r11, r4, #1 @ decrement it diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S index ea0349f..dd5e56f 100644 --- a/arch/arm/vfp/vfphw.S +++ b/arch/arm/vfp/vfphw.S @@ -168,7 +168,7 @@ vfp_hw_state_valid: @ else it's one 32-bit instruction, so @ always subtract 4 from the following @ instruction address. -#ifdef CONFIG_PREEMPT +#ifdef CONFIG_PREEMPT_COUNT get_thread_info r10 ldr r4, [r10, #TI_PREEMPT] @ get preempt count sub r11, r4, #1 @ decrement it @@ -192,7 +192,7 @@ look_for_VFP_exceptions: @ not recognised by VFP DBGSTR "not VFP" -#ifdef CONFIG_PREEMPT +#ifdef CONFIG_PREEMPT_COUNT get_thread_info r10 ldr r4, [r10, #TI_PREEMPT] @ get preempt count sub r11, r4, #1 @ decrement it