From patchwork Mon Sep 5 17:18:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: maddy X-Patchwork-Id: 665967 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sScPP52lPz9s4n for ; Tue, 6 Sep 2016 03:37:29 +1000 (AEST) Received: from ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3sScPP2rspzDrqF for ; Tue, 6 Sep 2016 03:37:29 +1000 (AEST) X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3sSbzB5gbhzDrqP for ; Tue, 6 Sep 2016 03:18:14 +1000 (AEST) Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u85HD3j1135885 for ; Mon, 5 Sep 2016 13:18:12 -0400 Received: from e23smtp07.au.ibm.com (e23smtp07.au.ibm.com [202.81.31.140]) by mx0b-001b2d01.pphosted.com with ESMTP id 258bv059cf-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 05 Sep 2016 13:18:12 -0400 Received: from localhost by e23smtp07.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 6 Sep 2016 03:18:09 +1000 Received: from d23dlp02.au.ibm.com (202.81.31.213) by e23smtp07.au.ibm.com (202.81.31.204) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 6 Sep 2016 03:18:06 +1000 X-IBM-Helo: d23dlp02.au.ibm.com X-IBM-MailFrom: maddy@linux.vnet.ibm.com X-IBM-RcptTo: linuxppc-dev@lists.ozlabs.org Received: from d23relay10.au.ibm.com (d23relay10.au.ibm.com [9.190.26.77]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 73E272BB0059 for ; Tue, 6 Sep 2016 03:18:06 +1000 (EST) Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay10.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u85HI6v861276412 for ; Tue, 6 Sep 2016 03:18:06 +1000 Received: from d23av04.au.ibm.com (localhost [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u85HI5Fh007286 for ; Tue, 6 Sep 2016 03:18:06 +1000 Received: from [9.126.238.53] ([9.126.238.53]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u85HI0Lv007251; Tue, 6 Sep 2016 03:18:02 +1000 Subject: Re: [RFC PATCH v4 11/12] powerpc: Add a Kconfig and a function to set new soft_enabled mask To: Nicholas Piggin References: <1472409448-18172-1-git-send-email-maddy@linux.vnet.ibm.com> <1472409448-18172-12-git-send-email-maddy@linux.vnet.ibm.com> <20160829114129.7e68c805@roar.ozlabs.ibm.com> From: Madhavan Srinivasan Date: Mon, 5 Sep 2016 22:48:00 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160829114129.7e68c805@roar.ozlabs.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16090517-0044-0000-0000-000001D74A88 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16090517-0045-0000-0000-000005784E63 Message-Id: <9fbb7a10-13a2-8542-2050-d73a958167d7@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-09-05_07:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1609050254 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linuxppc-dev@lists.ozlabs.org, paulus@samba.org, anton@samba.org Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Monday 29 August 2016 07:11 AM, Nicholas Piggin wrote: > On Mon, 29 Aug 2016 00:07:27 +0530 > Madhavan Srinivasan wrote: > >> New Kconfig is added "CONFIG_IRQ_DEBUG_SUPPORT" to add a warn_on >> to alert the usage of soft_irq_set_mask() for disabling lower >> bitmask interrupts. >> >> Have also moved the code under the CONFIG_TRACE_IRQFLAGS in >> arch_local_irq_restore() to new Kconfig as suggested. >> >> Patch also adds a new soft_irq_set_mask() to update paca->soft_enabled. >> >> Signed-off-by: Madhavan Srinivasan >> --- >> arch/powerpc/Kconfig | 4 ++++ >> arch/powerpc/include/asm/hw_irq.h | 17 +++++++++++++++++ >> arch/powerpc/kernel/irq.c | 4 ++-- >> 3 files changed, 23 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 927d2ab2ce08..878f05925340 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -51,6 +51,10 @@ config TRACE_IRQFLAGS_SUPPORT >> bool >> default y >> >> +config IRQ_DEBUG_SUPPORT >> + bool >> + default n >> + >> config LOCKDEP_SUPPORT >> bool >> default y >> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h >> index 415734c07cfa..9f71559ce868 100644 >> --- a/arch/powerpc/include/asm/hw_irq.h >> +++ b/arch/powerpc/include/asm/hw_irq.h >> @@ -81,6 +81,23 @@ static inline unsigned long arch_local_irq_disable(void) >> return flags; >> } >> >> +static inline unsigned long soft_irq_set_mask(int value) >> +{ >> + unsigned long flags, zero; >> + >> +#ifdef CONFIG_IRQ_DEBUG_SUPPORT >> + WARN_ON(value <= IRQ_DISABLE_MASK_LINUX); >> +#endif >> + asm volatile( >> + "li %1,%3; lbz %0,%2(13); stb %1,%2(13)" >> + : "=r" (flags), "=&r" (zero) >> + : "i" (offsetof(struct paca_struct, soft_enabled)),\ >> + "i" (value) >> + : "memory"); >> + >> + return flags; >> +} > One other thing, if we have: > > local_irq_save(flags); // disable LINUX mask -> LINUX > local_irq_and_pmu_save(flags); // disable PMU mask -> LINUX|PMU > local_irq_and_pmu_restore(flags); // enable PMU mask -> LINUX > local_irq_restore(flags); // enable LINUX mask -> NONE > > Then the nested code that re-enables PMUs will not replay PMU interrupt > until the outer irq is re-enabled. I don't *think* this is a problem, > but it probably should be commented. > > Which brings us to arch_local_irq_restore() (from another patch): > > > @@ -208,7 +209,7 @@ notrace void arch_local_irq_restore(unsigned long en) > > /* Write the new soft-enabled value */ > set_soft_enabled(en); > - if (!en) > + if (en == IRQ_DISABLE_MASK_LINUX) > return; > /* > * From this point onward, we can take interrupts, preempt, > > I think this is a bit buggy for some cases of nested disables. For the > above it is okay, but if we have: > > local_irq_and_pmu_save(flags); > local_irq_and_pmu_save(flags); > local_irq_and_pmu_restore(flags); > local_irq_and_pmu_restore(flags); > > > The first restore will restore LINUX|PMU mask, which will not be caught > by this check. > > Testing instead for non-zero (any IRQ bits masked) should work. We should > probably also add an IRQ_DEBUG_SUPPORT check to ensure LINUX bit is always > one of the outer-most bits to be cleared (theoretically we could support Just sent out the newer version of the patchset with most of the comments addressed. But I am not sure about this comment. IIUC, will something like this will do? Maddy > masking other bits without masking LINUX, but let's disallow that until a > compelling use case shows up). > > Thanks, > Nick > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c index a02c6a3bc6fa..af0c08aefbcf 100644 --- a/arch/powerpc/kernel/irq.c +++ b/arch/powerpc/kernel/irq.c @@ -209,6 +209,11 @@ notrace void arch_local_irq_restore(unsigned long en) unsigned char irq_happened; unsigned int replay; +#ifdef CONFIG_IRQ_DEBUG_SUPPORT + WARN_ONCE (en & (en & local_paca->soft_enabled), + "soft_enabled transition to Unsupported state\n"); +#endif + /* Write the new soft-enabled value */ set_soft_enabled(en);