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 744ADC47258 for ; Wed, 31 Jan 2024 14:05:58 +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-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=sSdhxhq7Eow8igNZibLUq6suUJRS5ZKlz5LFssuXtRU=; b=l57SKU0xjJXWq/ /UDW6fyHlER18eLbV/DsHIcDRx9CoRvxp3L7+cna6q9R8tYlQnsllgf+ArjwJ7lBk7KYru5qcQx3c Plj/sJxIrcWIvf9111wbwnJJUsjq0B6nTjYEgldMbJZp0QHxMBHL2hh1W3HcgU9t1GZ/qiOQoVKCy UNLqL467A+jeAt8a7oj7eX6mOkd0pATrIEDc/utTsYjIysFXaRt0EeTEHxgFwppOdXtt5FxdwT82s XYlNA2wiDhBpz5j/+x61V19Hm6xu08vfceK7GHxFmB9cGwi6w+u8tlWY02NZYRnUIChXuZhzMFjVj YpUYdhkwtO40r8uBTo4g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rVBDf-00000003lx0-3d1f; Wed, 31 Jan 2024 14:05:39 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rVBDc-00000003lvH-2urn for linux-arm-kernel@lists.infradead.org; Wed, 31 Jan 2024 14:05:38 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 18D0761764; Wed, 31 Jan 2024 14:05:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DB537C43142; Wed, 31 Jan 2024 14:05:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1706709935; bh=GaMZ0948xeDfn9kR8d8YGemHEgYUzVtp9ZX9hpkstxM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=f75FCj3Qf3BHUDcRepSDwUJmB4/KSARZfJRWONHm6KgZ60sqqfUGZyoBzanJAufgY +CgHzpY1YXKY6wrMYGVBjuJOqRvL+3iVwYtlHmGobjn6quftcHtK2vPBRfXvrJDk+u aSO64ySvHYMJs0WRX6gMvl4M9QLoFyiYbx7Y+vVGvI6zFtcpVPdDMefr/GoJvbI1gH FjaS/AvEsjCU3FiA6ma+VEcagj43uIibqZU/Pp26TClYJAx4qW+iF60dfy2e6DQj34 gAmWHLgEaSsq6TD0h8hynSqudybKM3kuXwHsMwUfrQ1f50wS3JhxnxWaZ1jeNEecRh VFVWBWBCeMoZw== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1rVBDZ-00Gcdh-Ca; Wed, 31 Jan 2024 14:05:33 +0000 Date: Wed, 31 Jan 2024 14:05:32 +0000 Message-ID: <86mssl7g43.wl-maz@kernel.org> From: Marc Zyngier To: Kunkun Jiang Cc: Oliver Upton , James Morse , Suzuki K Poulose , Zenghui Yu , Thomas Gleixner , , , Subject: Re: [PATCH v2 1/2] irqchip/gic-v4.1: Fix GICv4.1 doorbell affinity In-Reply-To: <72090b1b-21ab-6247-6231-1be2e11a2e4d@huawei.com> References: <20240126103012.1020-1-jiangkunkun@huawei.com> <20240126103012.1020-2-jiangkunkun@huawei.com> <86ttn074ev.wl-maz@kernel.org> <44051a41-f82e-57d9-001a-d4d195d59f2e@huawei.com> <86r0i17fkf.wl-maz@kernel.org> <86plxj6i3o.wl-maz@kernel.org> <72090b1b-21ab-6247-6231-1be2e11a2e4d@huawei.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/29.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: jiangkunkun@huawei.com, oliver.upton@linux.dev, james.morse@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, tglx@linutronix.de, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, wanghaibin.wang@huawei.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240131_060536_854601_367788F0 X-CRM114-Status: GOOD ( 46.71 ) 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 Wed, 31 Jan 2024 01:29:40 +0000, Kunkun Jiang wrote: > > Hi Marc, > > On 2024/1/30 21:55, Marc Zyngier wrote: > > On Tue, 30 Jan 2024 13:32:24 +0000, > > Kunkun Jiang wrote: > >> Hi Marc, > >> > >> On 2024/1/28 21:28, Marc Zyngier wrote: > >>> On Sat, 27 Jan 2024 02:41:55 +0000, > >>> Kunkun Jiang wrote: > >>>>> At this stage, I think the VMOVP optimisation is wrong and that we > >>>>> should drop it. > >>>> This is the last resort. Maybe we can think of another way. > >>> I think the only other way is to never elide the VMOVP from > >>> set_affinity, but to only call into set_affinity when strictly > >>> necessary: > >>> > >>> - when making the VPE resident on a CPU that isn't part of the same > >>> affinity group (VPE migration outside of the affinity group) > >>> > >>> - when making the VPE non-resident on a CPU that isn't the doorbell > >>> delivery CPU *AND* that there is a need for a doorbell (VPE > >>> migration inside the affinity group) > >>> > >>> Which means it is the responsibility of the upper layers of the stack > >>> to make these decisions, and that we can keep the low-level driver as > >>> simple as possible. > >>> > >>> In the meantime, here's the patch I'm proposing to merge ASAP. Please > >>> let me know if that works for you. > >> This patch works. But I still have some questions, which I will > >> write down below. > >>> M. > >>> > >>> >From 00c338e91d3b6600f402f56cdb64c9d370f911c8 Mon Sep 17 00:00:00 2001 > >>> From: Marc Zyngier > >>> Date: Sun, 28 Jan 2024 13:05:24 +0000 > >>> Subject: [PATCH] irqchip/gic-v3-its: Fix GICv4.1 VPE affinity update > >>> > >>> When updating the affinity of a VPE, we currently skip the VMOVP > >>> command if the two CPUs are part of the same VPE affinity. > >>> > >>> But this is wrong, as the doorbell corresponding to this VPE > >>> is still delivered on the 'old' CPU, which screws up the balancing. > >>> Furthermore, offlining that 'old' CPU results in doorbell interrupts > >>> generated for this VPE being discarded. > >>> > >>> The harsh reality is that we cannot easily elide VMOVP when > >>> a set_affinity request occurs. It needs to be obeyed, and if > >>> an optimisation is to be made, it is at the point where the affinity > >>> change request is made (such as in KVM). > >>> > >>> Drop the VMOVP elision altogether, and only use the vpe_table_mask > >>> to try and stay within the same ITS affinity group if at all possible. > >>> > >>> Fixes: dd3f050a216e (irqchip/gic-v4.1: Implement the v4.1 flavour of VMOVP) > >>> Reported-by: Kunkun Jiang > >>> Signed-off-by: Marc Zyngier > >>> Cc: stable@vger.kernel.org > >>> --- > >>> drivers/irqchip/irq-gic-v3-its.c | 22 +++++++++++++--------- > >>> 1 file changed, 13 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > >>> index 250b4562f308..78aece62bff5 100644 > >>> --- a/drivers/irqchip/irq-gic-v3-its.c > >>> +++ b/drivers/irqchip/irq-gic-v3-its.c > >>> @@ -3826,8 +3826,9 @@ static int its_vpe_set_affinity(struct irq_data *d, > >>> bool force) > >>> { > >>> struct its_vpe *vpe = irq_data_get_irq_chip_data(d); > >>> - int from, cpu = cpumask_first(mask_val); > >>> + struct cpumask common; > >>> unsigned long flags; > >>> + int from, cpu; > >>> /* > >>> * Changing affinity is mega expensive, so let's be as lazy as > >>> @@ -3843,19 +3844,22 @@ static int its_vpe_set_affinity(struct irq_data *d, > >>> * taken on any vLPI handling path that evaluates vpe->col_idx. > >>> */ > >>> from = vpe_to_cpuid_lock(vpe, &flags); > >>> - if (from == cpu) > >>> - goto out; > >>> - > >>> - vpe->col_idx = cpu; > >>> /* > >>> - * GICv4.1 allows us to skip VMOVP if moving to a cpu whose RD > >>> - * is sharing its VPE table with the current one. > >>> + * If we are offered another CPU in the same GICv4.1 ITS > >>> + * affinity, pick this one. Otherwise, any CPU will do. > >>> */ > >>> - if (gic_data_rdist_cpu(cpu)->vpe_table_mask && > >>> - cpumask_test_cpu(from, gic_data_rdist_cpu(cpu)->vpe_table_mask)) > >>> + if (gic_data_rdist_cpu(from)->vpe_table_mask && > >>> + cpumask_and(&common, mask_val, gic_data_rdist_cpu(from)->vpe_table_mask)) > >>> + cpu = cpumask_first(&common); > >>> + else > >>> + cpu = cpumask_first(mask_val); > >>> + > >>> + if (from == cpu) > >>> goto out; > >> The meaning here should be that VMOVP will be skipped > >> only if from and target are the same. Then why do we > >> still to judge whether they are in the same GICv4.1 > >> ITS affinity? > > Having the same ITS affinity is a performance optimisation, as this > > minimises traffic between ITSs and keep cache locality. If the > > proposed affinity contains CPUs that are in the same affinity as > > current one, then it makes sense to stay there if possible in order to > > keep the benefit of the warm translation caches. > Ok, I see. In another case, is it possible to skip VMOVP? > The from belongs to common, but it is not the first one. You could have something like if (cpumask_test_cpu(from, &common)) cpu = from; else cpu = cpumask_first(&common); M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel