From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7EBE31DAC95 for ; Wed, 6 Aug 2025 06:26:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.187 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754461600; cv=none; b=jU/JPiJ3kxjBw7Ll0mjvEOCrvHZDLm1PbGwTEFlYWJKlxycEXTLwq2ohxPbHKYMfwdpzQ7M7W1LePjy9QQIuac0hBbdFqcs4all1eAM4+BpmFuaegu9xYefBNGjk23nJ5HbEWtKeA2RFJW8gWAg4S3W4WTMk21giB0BXl9mq/vI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754461600; c=relaxed/simple; bh=UFsc8wdouk+ALN3i4/0++LuXJB4uKaU0H6wmOhl4lbY=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=YTyUXvSMAUUyXw4zAUswqSxRnPKUEDYsTZQhYgQpJj89W2ARh7S8pvXhAbZKk8T/7HPmU7w+sewOb6/yLScb5mFMhlZHQ/8F+YptNdRR1xDtrEDF0KyCBDv7oRF9aSwd8j4S6xmMqQvG55DzimQfOf5y388SJgsn+IayGWQLmwo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.187 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.163.48]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4bxgDK4p7yz14MKj; Wed, 6 Aug 2025 14:21:37 +0800 (CST) Received: from dggpemf500011.china.huawei.com (unknown [7.185.36.131]) by mail.maildlp.com (Postfix) with ESMTPS id D56AE1800B1; Wed, 6 Aug 2025 14:26:33 +0800 (CST) Received: from [10.67.109.254] (10.67.109.254) by dggpemf500011.china.huawei.com (7.185.36.131) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Wed, 6 Aug 2025 14:26:32 +0800 Message-ID: <5d81d30f-8dd4-d74c-7c8a-0714dafc09fe@huawei.com> Date: Wed, 6 Aug 2025 14:26:31 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.2.0 Subject: Re: [PATCH -next v7 5/7] arm64: entry: Refactor preempt_schedule_irq() check code Content-Language: en-US To: Ada Couprie Diaz CC: , , , , , , , , , , , , , , , , , , References: <20250729015456.3411143-1-ruanjinjie@huawei.com> <20250729015456.3411143-6-ruanjinjie@huawei.com> <44fd646c-4e31-4ca6-9e22-f715ad19e0d7@arm.com> From: Jinjie Ruan In-Reply-To: <44fd646c-4e31-4ca6-9e22-f715ad19e0d7@arm.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-ClientProxiedBy: kwepems100002.china.huawei.com (7.221.188.206) To dggpemf500011.china.huawei.com (7.185.36.131) On 2025/8/5 23:06, Ada Couprie Diaz wrote: > Hi Jinjie, > > On 29/07/2025 02:54, Jinjie Ruan wrote: >> ARM64 requires an additional check whether to reschedule on return >> from interrupt. So add arch_irqentry_exit_need_resched() as the default >> NOP implementation and hook it up into the need_resched() condition in >> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement >> the architecture specific version for switching over to >> the generic entry code. > I was a bit confused by this, as I didn't see the link with the generic > entry > given you implement `raw_irqentry_exit_cond_resched()` in arch/arm64 > as well in this patch : I expected the arm64 implementation to be added. > I share more thoughts below. > > What do you think about something along those lines ? > >     Compared to the generic entry code, arm64 does additional checks >     when deciding to reschedule on return from an interrupt. >     Introduce arch_irqentry_exit_need_resched() in the need_resched() > condition >     of the generic raw_irqentry_exit_cond_resched(), with a NOP default. >     This will allow arm64 to implement its architecture specific checks > when >     switching over to the generic entry code. This revision makes it easier for people to understand. > >> [...] >> diff --git a/kernel/entry/common.c b/kernel/entry/common.c >> index b82032777310..4aa9656fa1b4 100644 >> --- a/kernel/entry/common.c >> +++ b/kernel/entry/common.c >> @@ -142,6 +142,20 @@ noinstr irqentry_state_t irqentry_enter(struct >> pt_regs *regs) >>       return ret; >>   } >>   +/** >> + * arch_irqentry_exit_need_resched - Architecture specific need >> resched function >> + * >> + * Invoked from raw_irqentry_exit_cond_resched() to check if need >> resched. > Very nit : "to check if resched is needed." ? This is good. >> + * Defaults return true. >> + * >> + * The main purpose is to permit arch to skip preempt a task from an >> IRQ. > If feel that "to avoid preemption of a task" instead of "to skip preempt > a task" > would make this much clearer, what do you think ? Yes, this is more clearer. >> + */ >> +static inline bool arch_irqentry_exit_need_resched(void); >> + >> +#ifndef arch_irqentry_exit_need_resched >> +static inline bool arch_irqentry_exit_need_resched(void) { return >> true; } >> +#endif >> + > > I've had some trouble reviewing this patch : on the one hand because > I didn't notice `arch_irqentry_exit_need_resched()` was added in > the common entry code, which is on me ! > On the other hand, I felt that the patch itself was a bit disconnected : > we add `arch_irqentry_exit_need_resched()` in the common entry code, > with a default NOP, but in the same function we add to arm64, > while mentioning that this is for arm64's additional checks, > which we only implement in patch 7. > > Would it make sense to move the `arch_irqentry_exit_need_resched()` > part of the patch to patch 7, so that the introduction and > arch-specific implementation appear together ? > To me it seems easier to wrap my head around, as it would look like > "Move arm64 to generic entry, but it does additional checks : add a new > arch-specific function controlling re-scheduling, defaulting to true, > and implement it for arm64". I feel it could help making patch 7's > commit message clearer as well. > > From what I gathered on the archive `arch_irqentry_exit_need_resched()` > being added here was suggested previously, so others might not have the > same opinion. > > Maybe improving the commit message and comment for this would be enough > as well, as per my suggestions above. > > > Otherwise the changes make sense and I don't see any functional issues ! > > Thanks, > Ada > >