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 324C2D2C54B for ; Tue, 22 Oct 2024 13:10:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=panBbuREB9+tE5mV/pZ4jez0jwMVM5pj9FNMQY2lPZk=; b=mDyrhNol/O7Yf3PCAM+l+lCUIf XCEQVznR4CMkUVf0jfZsHDGdBjEXHaPsEyFjuTu/+jZ2v4FAfjHMiQmikvKBYnX38g0iOKlsI7kft YINP3/rtBjpsNcJMSDXkj5X7r/B/iZrsva6OGl7+LCsGhRo7EFp6Lor5QQgKDvWuxDoYcV9geIRpQ KzzjMev496NhmyQIofSMpWHvNMRs4RHaoJ4J/o5VNzZVGpAvrowWzmoU3x1dlyDn5MvDEhXo146+y 5wmEB/fMmx61BFP34tnPWjAsOlqh0xtQv16OtJ8ThMPlRLYNh0pJ1HHmFuLoF78W7ofqble2Www31 m+YRbrbg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t3EeC-0000000Aubb-19Rt; Tue, 22 Oct 2024 13:10:04 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t3Ecg-0000000AuRn-08d5 for linux-arm-kernel@lists.infradead.org; Tue, 22 Oct 2024 13:08:31 +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 9C506497; Tue, 22 Oct 2024 06:08:56 -0700 (PDT) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3104B3F73B; Tue, 22 Oct 2024 06:08:24 -0700 (PDT) Date: Tue, 22 Oct 2024 14:08:12 +0100 From: Mark Rutland To: Jinjie Ruan Cc: catalin.marinas@arm.com, will@kernel.org, oleg@redhat.com, tglx@linutronix.de, peterz@infradead.org, luto@kernel.org, kees@kernel.org, wad@chromium.org, rostedt@goodmis.org, arnd@arndb.de, ardb@kernel.org, broonie@kernel.org, rick.p.edgecombe@intel.com, leobras@redhat.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 0/3] arm64: entry: Convert to generic entry Message-ID: References: <20240629085601.470241-1-ruanjinjie@huawei.com> <2cc049fe-8f1a-0829-d879-d89278027be6@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2cc049fe-8f1a-0829-d879-d89278027be6@huawei.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241022_060830_183052_08D2CBC3 X-CRM114-Status: GOOD ( 29.13 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Oct 22, 2024 at 08:07:54PM +0800, Jinjie Ruan wrote: > On 2024/10/17 23:25, Mark Rutland wrote: > > There's also some indirection that I don't think is necessary *and* > > hides important ordering concerns and results in mistakes. In > > particular, note that before this series, enter_from_kernel_mode() calls > > the (instrumentable) MTE checks *after* all the necessary lockdep+RCU > > management is performed by __enter_from_kernel_mode(): > > > > static void noinstr enter_from_kernel_mode(struct pt_regs *regs) > > { > > __enter_from_kernel_mode(regs); > > mte_check_tfsr_entry(); > > mte_disable_tco_entry(current); > > } > > > > ... whereas after this series is applied, those MTE checks are placed in > > arch_enter_from_kernel_mode(), which irqentry_enter() calls *before* the > > necessary lockdep+RCU management. That is broken. > > > > It would be better to keep that explicit in the arm64 entry code with > > arm64-specific wrappers, e.g. > > > > static noinstr irqentry_state_t enter_from_kernel_mode(struct pt_regs *regs) > > { > > irqentry_state_t state = irqentry_enter(regs); > > mte_check_tfsr_entry(); > > mte_disable_tco_entry(current); > > > > return state; > > } > > Hi, Mark, It seems that there is a problem for > arm64_preempt_schedule_irq() when wrap irqentry_exit() with > exit_to_kernel_mode(). > > The arm64_preempt_schedule_irq() is about PREEMPT_DYNAMIC and preempt > irq which is the raw_irqentry_exit_cond_resched() in generic code called > by irqentry_exit(). > > Only __el1_irq() call arm64_preempt_schedule_irq(), but when we switch > all exit_to_kernel_mode() to arm64-specific one that wrap > irqentry_exit(), not only __el1_irq() but also el1_abort(), el1_pc(), > el1_undef() etc. will try to reschedule by calling > arm64_preempt_schedule_irq() similar logic. Yes, the generic entry code will preempt any context where an interrupt *could* have been taken from. I'm not sure it actually matters either way; I believe that the generic entry code was written this way because that's what x86 did, and checking for whether interrupts are enabled in the interrupted context is cheap. I's suggest you first write a patch to align arm64's entry code with the generic code, by removing the call to arm64_preempt_schedule_irq() from __el1_irq(), and adding a call to arm64_preempt_schedule_irq() in __exit_to_kernel_mode(), e.g. | static __always_inline void __exit_to_kernel_mode(struct pt_regs *regs) | { | ... | if (interrupts_enabled(regs)) { | ... | if (regs->exit_rcu) { | ... | } | ... | arm64_preempt_schedule_irq(); | ... | } else { | ... | } | } That way the behaviour and structure will be more aligned with the generic code, and with that as an independent patch it will be easier to review/test/bisect/etc. This change will have at least two key impacts: (1) We'll preempt even without taking a "real" interrupt. That shouldn't result in preemption that wasn't possible before, but it does change the probability of preempting at certain points, and might have a performance impact, so probably warrants a benchmark. (2) We will not preempt when taking interrupts from a region of kernel code where IRQs are enabled but RCU is not watching, matching the behaviour of the generic entry code. This has the potential to introduce livelock if we can ever have a screaming interrupt in such a region, so we'll need to go figure out whether that's actually a problem. Having this as a separate patch will make it easier to test/bisect for that specifically. Mark.