From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EA61CC433F5 for ; Thu, 30 Sep 2021 13:53:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D458861440 for ; Thu, 30 Sep 2021 13:53:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1351508AbhI3NzC (ORCPT ); Thu, 30 Sep 2021 09:55:02 -0400 Received: from foss.arm.com ([217.140.110.172]:54606 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235171AbhI3NzA (ORCPT ); Thu, 30 Sep 2021 09:55:00 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 03BB1101E; Thu, 30 Sep 2021 06:53:18 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7327D3F70D; Thu, 30 Sep 2021 06:53:16 -0700 (PDT) Date: Thu, 30 Sep 2021 14:53:14 +0100 From: Mark Rutland To: Pingfan Liu Cc: linux-arm-kernel@lists.infradead.org, "Paul E. McKenney" , Catalin Marinas , Will Deacon , Marc Zyngier , Joey Gouly , Sami Tolvanen , Julien Thierry , Thomas Gleixner , Yuichi Ito , linux-kernel@vger.kernel.org Subject: Re: [PATCHv3 3/3] arm64/entry-common: supplement irq accounting Message-ID: <20210930135314.GC18258@lakrids.cambridge.arm.com> References: <20210930131708.35328-1-kernelfans@gmail.com> <20210930131708.35328-4-kernelfans@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210930131708.35328-4-kernelfans@gmail.com> User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 30, 2021 at 09:17:08PM +0800, Pingfan Liu wrote: > At present, the irq entry/exit accounting, which is performed by > handle_domain_irq(), overlaps with arm64 exception entry code somehow. > > By supplementing irq accounting on arm64 exception entry code, the > accounting in handle_domain_irq() can be dropped totally by selecting > the macro HAVE_ARCH_IRQENTRY. I think we need to be more thorough and explain the specific problem and solution. How about we crib some wording from patch 1, and say: arm64: entry: avoid double-accounting IRQ RCU entry When an IRQ is taken, some accounting needs to be performed to enter and exit IRQ context around the IRQ handler. On arm64 some of this accounting is performed by both the architecture code and the IRQ domain code, resulting in calling rcu_irq_enter() twice per exception entry, violating the expectations of the core RCU code, and resulting in failing to identify quiescent periods correctly (e.g. in rcu_is_cpu_rrupt_from_idle()). To fix this, we must perform all the accounting from the architecture code. We prevent the IRQ domain code from performing any accounting by selecting HAVE_ARCH_IRQENTRY, and must call irq_enter_rcu() and irq_exit_rcu() around invoking the root IRQ handler. When we take a pNMI from a context with IRQs disabled, we'll perform the necessary accounting as part of arm64_enter_nmi() and arm64_exit_nmi(), and should only call irq_enter_rcu() and irq_exit_rcu() when we may have taken a regular interrupt. That way it's clear what specifically the overlap is and the problem(s) it results in. The bit at the end explains why we don't call irq_{enter,exit}_rcu() when we're certain we've taken a pNMI. > Signed-off-by: Pingfan Liu > Cc: "Paul E. McKenney" > Cc: Catalin Marinas > Cc: Will Deacon > Cc: Mark Rutland > Cc: Marc Zyngier > Cc: Joey Gouly > Cc: Sami Tolvanen > Cc: Julien Thierry > Cc: Thomas Gleixner > Cc: Yuichi Ito > Cc: linux-kernel@vger.kernel.org > To: linux-arm-kernel@lists.infradead.org > --- > arch/arm64/Kconfig | 1 + > arch/arm64/kernel/entry-common.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 5c7ae4c3954b..d29bae38a951 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -98,6 +98,7 @@ config ARM64 > select ARCH_HAS_UBSAN_SANITIZE_ALL > select ARM_AMBA > select ARM_ARCH_TIMER > + select HAVE_ARCH_IRQENTRY Please put this with the other HAVE_ARCH_* entries in arch/arm64/Kconfig -- it should be between HAVE_ARCH_HUGE_VMAP and HAVE_ARCH_JUMP_LABEL to keep that in alphabetical order. With that and the title and commit message above: Reviewed-by: Mark Rutland Thanks, Mark. > select ARM_GIC > select AUDIT_ARCH_COMPAT_GENERIC > select ARM_GIC_V2M if PCI > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > index 5f1473319fb0..6d4dc3b3799f 100644 > --- a/arch/arm64/kernel/entry-common.c > +++ b/arch/arm64/kernel/entry-common.c > @@ -428,7 +428,9 @@ static __always_inline void > __el1_interrupt(struct pt_regs *regs, void (*handler)(struct pt_regs *)) > { > enter_from_kernel_mode(regs); > + irq_enter_rcu(); > do_interrupt_handler(regs, handler); > + irq_exit_rcu(); > /* > * Note: thread_info::preempt_count includes both thread_info::count > * and thread_info::need_resched, and is not equivalent to > @@ -667,7 +669,9 @@ static void noinstr el0_interrupt(struct pt_regs *regs, > if (regs->pc & BIT(55)) > arm64_apply_bp_hardening(); > > + irq_enter_rcu(); > do_interrupt_handler(regs, handler); > + irq_exit_rcu(); > > exit_to_user_mode(regs); > } > -- > 2.31.1 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B8C6AC433FE for ; Thu, 30 Sep 2021 13:56:15 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 890DB60E75 for ; Thu, 30 Sep 2021 13:56:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 890DB60E75 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=QfbMoyRMipD3EeQ/U8g+yU4uOkKtHA/dfLWtahwWKVk=; b=Aabv8ZAdthYPs3 P7/sMjEaERgQr1U7UcavSH0gMgwnrZlOCfcDiITWybQqGIi88il+yiGESw4KRekXTOcQzqmgULM8W wvylCuW3KqbmwT6l753UvqjNNUfx9CjEHJoa0s6IcH5qbg3NiDB6+OUnAbiRQ74Rq4nEcW47gl9/I T3o2QghIiwve7j7nrQND4GWCoq3Q0AIs5DwdzCN1ndXVHkQUhCNECouy+W9NV6QicMfqBs3gflblf sa3trkOSKpE7DSp6rcFJZ/gTddzx8Dq1U1y/3Slmb8S0QSWfCLjQXq1gXBPNB45EKzW4VmJ6c6inG 2moENUR4x7cgEm5XDqig==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mVwV3-00Ea6Z-F2; Thu, 30 Sep 2021 13:53:25 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mVwV0-00Ea53-5X for linux-arm-kernel@lists.infradead.org; Thu, 30 Sep 2021 13:53:23 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 03BB1101E; Thu, 30 Sep 2021 06:53:18 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7327D3F70D; Thu, 30 Sep 2021 06:53:16 -0700 (PDT) Date: Thu, 30 Sep 2021 14:53:14 +0100 From: Mark Rutland To: Pingfan Liu Cc: linux-arm-kernel@lists.infradead.org, "Paul E. McKenney" , Catalin Marinas , Will Deacon , Marc Zyngier , Joey Gouly , Sami Tolvanen , Julien Thierry , Thomas Gleixner , Yuichi Ito , linux-kernel@vger.kernel.org Subject: Re: [PATCHv3 3/3] arm64/entry-common: supplement irq accounting Message-ID: <20210930135314.GC18258@lakrids.cambridge.arm.com> References: <20210930131708.35328-1-kernelfans@gmail.com> <20210930131708.35328-4-kernelfans@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210930131708.35328-4-kernelfans@gmail.com> User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210930_065322_330095_A33A2FB3 X-CRM114-Status: GOOD ( 27.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Sep 30, 2021 at 09:17:08PM +0800, Pingfan Liu wrote: > At present, the irq entry/exit accounting, which is performed by > handle_domain_irq(), overlaps with arm64 exception entry code somehow. > > By supplementing irq accounting on arm64 exception entry code, the > accounting in handle_domain_irq() can be dropped totally by selecting > the macro HAVE_ARCH_IRQENTRY. I think we need to be more thorough and explain the specific problem and solution. How about we crib some wording from patch 1, and say: arm64: entry: avoid double-accounting IRQ RCU entry When an IRQ is taken, some accounting needs to be performed to enter and exit IRQ context around the IRQ handler. On arm64 some of this accounting is performed by both the architecture code and the IRQ domain code, resulting in calling rcu_irq_enter() twice per exception entry, violating the expectations of the core RCU code, and resulting in failing to identify quiescent periods correctly (e.g. in rcu_is_cpu_rrupt_from_idle()). To fix this, we must perform all the accounting from the architecture code. We prevent the IRQ domain code from performing any accounting by selecting HAVE_ARCH_IRQENTRY, and must call irq_enter_rcu() and irq_exit_rcu() around invoking the root IRQ handler. When we take a pNMI from a context with IRQs disabled, we'll perform the necessary accounting as part of arm64_enter_nmi() and arm64_exit_nmi(), and should only call irq_enter_rcu() and irq_exit_rcu() when we may have taken a regular interrupt. That way it's clear what specifically the overlap is and the problem(s) it results in. The bit at the end explains why we don't call irq_{enter,exit}_rcu() when we're certain we've taken a pNMI. > Signed-off-by: Pingfan Liu > Cc: "Paul E. McKenney" > Cc: Catalin Marinas > Cc: Will Deacon > Cc: Mark Rutland > Cc: Marc Zyngier > Cc: Joey Gouly > Cc: Sami Tolvanen > Cc: Julien Thierry > Cc: Thomas Gleixner > Cc: Yuichi Ito > Cc: linux-kernel@vger.kernel.org > To: linux-arm-kernel@lists.infradead.org > --- > arch/arm64/Kconfig | 1 + > arch/arm64/kernel/entry-common.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 5c7ae4c3954b..d29bae38a951 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -98,6 +98,7 @@ config ARM64 > select ARCH_HAS_UBSAN_SANITIZE_ALL > select ARM_AMBA > select ARM_ARCH_TIMER > + select HAVE_ARCH_IRQENTRY Please put this with the other HAVE_ARCH_* entries in arch/arm64/Kconfig -- it should be between HAVE_ARCH_HUGE_VMAP and HAVE_ARCH_JUMP_LABEL to keep that in alphabetical order. With that and the title and commit message above: Reviewed-by: Mark Rutland Thanks, Mark. > select ARM_GIC > select AUDIT_ARCH_COMPAT_GENERIC > select ARM_GIC_V2M if PCI > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > index 5f1473319fb0..6d4dc3b3799f 100644 > --- a/arch/arm64/kernel/entry-common.c > +++ b/arch/arm64/kernel/entry-common.c > @@ -428,7 +428,9 @@ static __always_inline void > __el1_interrupt(struct pt_regs *regs, void (*handler)(struct pt_regs *)) > { > enter_from_kernel_mode(regs); > + irq_enter_rcu(); > do_interrupt_handler(regs, handler); > + irq_exit_rcu(); > /* > * Note: thread_info::preempt_count includes both thread_info::count > * and thread_info::need_resched, and is not equivalent to > @@ -667,7 +669,9 @@ static void noinstr el0_interrupt(struct pt_regs *regs, > if (regs->pc & BIT(55)) > arm64_apply_bp_hardening(); > > + irq_enter_rcu(); > do_interrupt_handler(regs, handler); > + irq_exit_rcu(); > > exit_to_user_mode(regs); > } > -- > 2.31.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel