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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 6EAB6C43334 for ; Tue, 19 Jul 2022 12:12:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=5qfWcocN+0UqarRf/azP90DOYqTK0Lq2nKBWxAQjQU4=; b=1IIjlnvLogbvIz kF6bRvVW+fRquutsC4ORCBlndO/CdbzCGjjgT59DJ7kwZEISOgIXqZc8iMjpHkO4l1dKvkmk8Pdkc 1ZC7XcBc3b/1BvwZ5PFBtHc0MTtgjmb1V0PMa7Hb1l19/68MZ4JHedSIRnIvCbfwe/U+3bbDlacIq w4wB2Det4alG1TwTy5uPbM5zEXsNglDNHEi+5JUUwzb+j9udRKHcf/q3jaNd1xzpPgRRceVJ8S50g 9+EY3zpGOdGyCJrOcTiUqmsfSlKqiUMo+lQh19XbrMQifqAWyccdZcNR2vnDn8Ae8oVgdq2uyeurm 8jT+dRHz27z6wB8m2F3Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oDm4O-008ZIK-CG; Tue, 19 Jul 2022 12:11:20 +0000 Received: from mail-pf1-x431.google.com ([2607:f8b0:4864:20::431]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oDm4K-008ZF1-DC for linux-arm-kernel@lists.infradead.org; Tue, 19 Jul 2022 12:11:18 +0000 Received: by mail-pf1-x431.google.com with SMTP id d10so13328659pfd.9 for ; Tue, 19 Jul 2022 05:11:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=iNDtX2qWi9m1xYihwpiwhJVw0BBHC84OS3fYxxiGs34=; b=r6vlUemm7Ry7I9jShfww8oYAQy7YDVVHqAKcZ6xOOb7Ktn1reZr/elTEx+h17sf1Rl sdpUKDhyq5zPvRCDI3nTFx6KBEixdwhNkLhj5fjn8Unv5kWb+fkP8d2oY8MzeBgjh6nW daUqKmKl6OSHPT1gRCA5s+y+EzzCBBxP5WgVe1yJYxDQuXy7p923PalHeIYu61RjbUK0 9eQMM7YP4FZiK7QhyrBghSPKfvyjFSVMG3TE8VS/tRas7kRuwIUNQJHCD9N6reaka7xv MEgHf+HDHfITTcVURNLtt34/R1CKP9l5DVca9LjRNLv0xkTpiYQECQqMfvrGtLpWlNx/ QIew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=iNDtX2qWi9m1xYihwpiwhJVw0BBHC84OS3fYxxiGs34=; b=ynuARq0I10u1MQpQXcs2sDFkvyS2jNBTPVqRnjkSql/UttS5aPBCiQ1rklPKaGT8WS 0FNdKFOEICko8ujFI99KlN7sj1IGq463DhFHrBHbyduR7yJkrthpeWi26WeivDtaqsu/ xbIIizCNva0Fhn20T8v0BmPq7g0vASciqtVBe1qaBkQx4XfOzPiX2myPqLhyO9V8Y0e2 D+/NtOaw/yzOVrkrV1qk1uToj2JgyvbIqC5BHp3ghDMCQUbAYhNjX9lw2BjM6aMQ+Ftl muSCc6CPb/Iqw9OmOUVJ1a+V52CZuWT3xnJS35OFXD0Pe9N7Y4FtjF13J81Vb90KR0Xl 6LBA== X-Gm-Message-State: AJIora8iOwkT62e1/wukBKwvqtqsywrUj7aMHeOae0NHQoVkVEq5qoKS InR5T8BjeE5tL5T4dLnaxHov/Q== X-Google-Smtp-Source: AGRyM1uppWywTWTnYB8naVjAi5O3rVZkJMFwJURmY1UQz1UM2e7wsqLcITZSZrHlkJgkFvIdp7qODg== X-Received: by 2002:a63:6902:0:b0:41a:3743:c768 with SMTP id e2-20020a636902000000b0041a3743c768mr6082694pgc.512.1658232672681; Tue, 19 Jul 2022 05:11:12 -0700 (PDT) Received: from [10.4.227.241] ([139.177.225.228]) by smtp.gmail.com with ESMTPSA id 13-20020a170902c24d00b0016a109c7606sm1761259plg.259.2022.07.19.05.11.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 19 Jul 2022 05:11:12 -0700 (PDT) Message-ID: <8518583c-d712-032f-32a1-f3f03e7c0e27@bytedance.com> Date: Tue, 19 Jul 2022 20:11:07 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.0 Subject: Re: [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK Content-Language: en-US To: Mark Rutland Cc: Arnd Bergmann , Catalin Marinas , Will Deacon , Linux ARM , Linux Kernel Mailing List References: <20220708094950.41944-1-zhengqi.arch@bytedance.com> <20220708094950.41944-3-zhengqi.arch@bytedance.com> From: Qi Zheng In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220719_051116_485976_47AD686F X-CRM114-Status: GOOD ( 30.61 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2022/7/19 19:24, Mark Rutland wrote: > On Thu, Jul 14, 2022 at 01:37:31PM +0200, Arnd Bergmann wrote: >> On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng wrote: >>> >>> Since softirqs are handled on the per-CPU IRQ stack, >>> let's support HAVE_IRQ_EXIT_ON_IRQ_STACK which causes >>> the core code to invoke __do_softirq() directly without >>> going through do_softirq_own_stack(). >>> >>> Signed-off-by: Qi Zheng >> >> Adding Mark Rutland to Cc, he's the one that worked on this area the most in the >> past and should probably review your patch. I still feel like there >> should be a way >> to improve readability of the IRQ entry path rather than just adding another >> level of indirection, but the ideas I had so far have not led to >> anything useful. >> >> Overall I suppose your version is an improvement over the extra double stack >> switch when entering softirq. Hi Mark, > > The cost of the switch is fairly trivial, so performance-wise I would expect > that to fall within the noise. So if this is just about avoiding the extra > switch, I'd prefer to leave this as-is. In this patch, I just want to avoid the extra switch. > > If we want to move more work after the stack switch, I think we'd want a more > in depth rethink of the structure of this code (since e.g. we could make the > common case have almost everything on the IRQ stack, at the cost of making For now, I don't think it's necessary to put anything other than irq_{enter|exit}_rcu() on the IRQ stack. For things that we want to run on the IRQ stack in the future, we can also easily stuff them into do_handler(). So I think the method of nesting pointers is good for extensibility except that it is not easy to read. And this method does not change much. :) > preemption require more work). Overall, I'm ok to drop this optimized patch if we don't care about the performance overhead of extra switching. Thanks, Qi > > Thanks, > Mark. > >> >> Arnd >> >>> --- >>> arch/arm64/Kconfig | 1 + >>> arch/arm64/include/asm/exception.h | 4 +++- >>> arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++---------- >>> arch/arm64/kernel/entry.S | 6 ++++-- >>> arch/arm64/kernel/irq.c | 5 +++-- >>> 5 files changed, 31 insertions(+), 15 deletions(-) >>> >>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >>> index be0a9f0052ee..d2cc7daecce3 100644 >>> --- a/arch/arm64/Kconfig >>> +++ b/arch/arm64/Kconfig >>> @@ -231,6 +231,7 @@ config ARM64 >>> select TRACE_IRQFLAGS_SUPPORT >>> select TRACE_IRQFLAGS_NMI_SUPPORT >>> select HAVE_SOFTIRQ_ON_OWN_STACK >>> + select HAVE_IRQ_EXIT_ON_IRQ_STACK >>> help >>> ARM 64-bit (AArch64) Linux support. >>> >>> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h >>> index d94aecff9690..8bff0aa7ab50 100644 >>> --- a/arch/arm64/include/asm/exception.h >>> +++ b/arch/arm64/include/asm/exception.h >>> @@ -54,7 +54,9 @@ asmlinkage void el0t_32_fiq_handler(struct pt_regs *regs); >>> asmlinkage void el0t_32_error_handler(struct pt_regs *regs); >>> >>> asmlinkage void call_on_irq_stack(struct pt_regs *regs, >>> - void (*func)(struct pt_regs *)); >>> + void (*func)(struct pt_regs *), >>> + void (*do_func)(struct pt_regs *, >>> + void (*)(struct pt_regs *))); >>> asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs); >>> >>> void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs); >>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c >>> index c75ca36b4a49..935d1ab150b5 100644 >>> --- a/arch/arm64/kernel/entry-common.c >>> +++ b/arch/arm64/kernel/entry-common.c >>> @@ -266,14 +266,16 @@ static void __sched arm64_preempt_schedule_irq(void) >>> } >>> >>> static void do_interrupt_handler(struct pt_regs *regs, >>> - void (*handler)(struct pt_regs *)) >>> + void (*handler)(struct pt_regs *), >>> + void (*do_handler)(struct pt_regs *, >>> + void (*)(struct pt_regs *))) >>> { >>> struct pt_regs *old_regs = set_irq_regs(regs); >>> >>> if (on_thread_stack()) >>> - call_on_irq_stack(regs, handler); >>> + call_on_irq_stack(regs, handler, do_handler); >>> else >>> - handler(regs); >>> + do_handler(regs, handler); >>> >>> set_irq_regs(old_regs); >>> } >>> @@ -441,22 +443,32 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs) >>> } >>> } >>> >>> +static void nmi_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *)) >>> +{ >>> + handler(regs); >>> +} >>> + >>> static __always_inline void __el1_pnmi(struct pt_regs *regs, >>> void (*handler)(struct pt_regs *)) >>> { >>> arm64_enter_nmi(regs); >>> - do_interrupt_handler(regs, handler); >>> + do_interrupt_handler(regs, handler, nmi_handler); >>> arm64_exit_nmi(regs); >>> } >>> >>> +static void irq_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *)) >>> +{ >>> + irq_enter_rcu(); >>> + handler(regs); >>> + irq_exit_rcu(); >>> +} >>> + >>> static __always_inline void __el1_irq(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(); >>> + do_interrupt_handler(regs, handler, irq_handler); >>> >>> arm64_preempt_schedule_irq(); >>> >>> @@ -699,9 +711,7 @@ 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(); >>> + do_interrupt_handler(regs, handler, irq_handler); >>> >>> exit_to_user_mode(regs); >>> } >>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S >>> index 254fe31c03a0..1c351391f6bd 100644 >>> --- a/arch/arm64/kernel/entry.S >>> +++ b/arch/arm64/kernel/entry.S >>> @@ -867,7 +867,9 @@ NOKPROBE(ret_from_fork) >>> >>> /* >>> * void call_on_irq_stack(struct pt_regs *regs, >>> - * void (*func)(struct pt_regs *)); >>> + * void (*func)(struct pt_regs *) >>> + * void (*do_func)(struct pt_regs *, >>> + * void (*)(struct pt_regs *))); >>> * >>> * Calls func(regs) using this CPU's irq stack and shadow irq stack. >>> */ >>> @@ -886,7 +888,7 @@ SYM_FUNC_START(call_on_irq_stack) >>> >>> /* Move to the new stack and call the function there */ >>> mov sp, x16 >>> - blr x1 >>> + blr x2 >>> >>> /* >>> * Restore the SP from the FP, and restore the FP and LR from the frame >>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c >>> index c36ad20a52f3..003db605bc4f 100644 >>> --- a/arch/arm64/kernel/irq.c >>> +++ b/arch/arm64/kernel/irq.c >>> @@ -73,14 +73,15 @@ static void init_irq_stacks(void) >>> #endif >>> >>> #ifndef CONFIG_PREEMPT_RT >>> -static void ____do_softirq(struct pt_regs *regs) >>> +static void ____do_softirq(struct pt_regs *regs, >>> + void (*handler)(struct pt_regs *)) >>> { >>> __do_softirq(); >>> } >>> >>> void do_softirq_own_stack(void) >>> { >>> - call_on_irq_stack(NULL, ____do_softirq); >>> + call_on_irq_stack(NULL, NULL, ____do_softirq); >>> } >>> #endif >>> >>> -- >>> 2.20.1 >>> -- Thanks, Qi _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel