Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Thu, 14 Apr 2022 11:35:51 +0100
Message-ID: <877d7sar5k.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Marek Szyprowski <m.szyprowski@samsung.com>
Cc:     linux-kernel <linux-kernel@vger.kernel.org>,
        'Linux Samsung SOC' <linux-samsung-soc@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        John Garry <john.garry@huawei.com>,
        Xiongfeng Wang <wangxiongfeng2@huawei.com>,
        David Decotigny <ddecotig@google.com>,
        Krzysztof Kozlowski <krzk@kernel.org>
Subject: Re: [PATCH v3 2/3] genirq: Always limit the affinity to online CPUs
In-Reply-To: <5dcf8d22-e9b3-f306-4c5f-256707e08fbf@samsung.com>
References: <20220405185040.206297-1-maz@kernel.org>
        <20220405185040.206297-3-maz@kernel.org>
        <CGME20220413145922eucas1p2dc46908354f4d2b48db79978d086a838@eucas1p2.samsung.com>
        <4b7fc13c-887b-a664-26e8-45aed13f048a@samsung.com>
        <878rs8c2t2.wl-maz@kernel.org>
        <5dcf8d22-e9b3-f306-4c5f-256707e08fbf@samsung.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

Hi Marek,

On Thu, 14 Apr 2022 10:09:31 +0100,
Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> Hi Marc,
> 
> On 13.04.2022 19:26, Marc Zyngier wrote:
> > Hi Marek,
> >
> > On Wed, 13 Apr 2022 15:59:21 +0100,
> > Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >> Hi Marc,
> >>
> >> On 05.04.2022 20:50, Marc Zyngier wrote:
> >>> When booting with maxcpus=<small number> (or even loading a driver
> >>> while most CPUs are offline), it is pretty easy to observe managed
> >>> affinities containing a mix of online and offline CPUs being passed
> >>> to the irqchip driver.
> >>>
> >>> This means that the irqchip cannot trust the affinity passed down
> >>> from the core code, which is a bit annoying and requires (at least
> >>> in theory) all drivers to implement some sort of affinity narrowing.
> >>>
> >>> In order to address this, always limit the cpumask to the set of
> >>> online CPUs.
> >>>
> >>> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >> This patch landed in linux next-20220413 as commit 33de0aa4bae9
> >> ("genirq: Always limit the affinity to online CPUs"). Unfortunately it
> >> breaks booting of most ARM 32bit Samsung Exynos based boards.
> >>
> >> I don't see anything specific in the log, though. Booting just hangs at
> >> some point. The only Samsung Exynos boards that boot properly are those
> >> Exynos4412 based.
> >>
> >> I assume that this is related to the Multi Core Timer IRQ configuration
> >> specific for that SoCs. Exynos4412 uses PPI interrupts, while all other
> >> Exynos SoCs have separate IRQ lines for each CPU.
> >>
> >> Let me know how I can help debugging this issue.
> > Thanks for the heads up. Can you pick the last working kernel, enable
> > CONFIG_GENERIC_IRQ_DEBUGFS, and dump the /sys/kernel/debug/irq/irqs/
> > entries for the timer IRQs?
> 
> Exynos4210, Trats board, next-20220411:

Thanks for all of the debug, super helpful. The issue is that we don't
handle the 'force' case, which a handful of drivers are using when
bringing up CPUs (and doing so before the CPUs are marked online).

Can you please give the below hack a go?

Thanks,

	M.

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index f71ecc100545..f1d5a94c6c9f 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -266,10 +266,16 @@ int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
 		prog_mask = mask;
 	}
 
-	/* Make sure we only provide online CPUs to the irqchip */
+	/*
+	 * Make sure we only provide online CPUs to the irqchip,
+	 * unless we are being asked to force the affinity (in which
+	 * case we do as we are told).
+	 */
 	cpumask_and(&tmp_mask, prog_mask, cpu_online_mask);
-	if (!cpumask_empty(&tmp_mask))
+	if (!force && !cpumask_empty(&tmp_mask))
 		ret = chip->irq_set_affinity(data, &tmp_mask, force);
+	else if (force)
+		ret = chip->irq_set_affinity(data, mask, force);
 	else
 		ret = -EINVAL;
 

-- 
Without deviation from the norm, progress is not possible.