From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 175F86DCFE for ; Tue, 30 Jan 2024 13:55:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706622942; cv=none; b=OCFwy7HraixnZi+b/GZlLJk3j2kUduMZE58HJToUj7TnKprvvC61DMJgiKH1GX9DfsT06FnjdpvfIuTFA0sS2zcmYVH1dXFSmKiXHKsfClkRglenH4ba6TkcD9cyPxukxPo+JeuF2qp9kEIAS9KjKuCyXLN2xIC3wUnyIurs3xw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706622942; c=relaxed/simple; bh=O9wk5zk5KB/gZrbFf6M01U2u6S9XPe+MZFQGHThXnO4=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=UMEAzT4uGyh5MBtoifyKAKvSghIaG6IYU5wLLxFdf7pZDDnuqHd0EKRhYlfWNe3EXB0VXNWQjbjz36pxfwRPPVwGVotnGwF5jCvL22PqE9dxPqDFxjR1dFwMVGVsssd13iiobvGWIFSOSYU1Lm7fVAazdEkPO0n5vMdGH5Q7vc0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hEdENMYO; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hEdENMYO" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A2009C433F1; Tue, 30 Jan 2024 13:55:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1706622941; bh=O9wk5zk5KB/gZrbFf6M01U2u6S9XPe+MZFQGHThXnO4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=hEdENMYOdzk5wwBTJ19vRkgpleszmRuKf1i5cb3vFUvdeKXTNueV6YvchwOlUzSjn FNULzn9HVs3YLHBxmzgSGXC45Iiel1/C90Ipy3V0pLOokuSfY4KRCnE2eymQOnOMrU dHqIxgZQEda9vLW8eBrazaE/5+WBDhauibSM6JaZykTQ3wDAfNKnqUxuAemRLW61vE nyaxux5x9kf6cJYWvIz93TGuKdbSM13YB36uc94VEQulIutw+GXXLHMiXfPNO6rsNy HELa9KcDyd98MlobnSTQEusDeYVEPe/VUlqeoEyqDS9whcvvA0NtqVloqcsxhylTk5 Ju4Ztur0+kLzg== 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 1rUoaR-00GHZZ-G6; Tue, 30 Jan 2024 13:55:39 +0000 Date: Tue, 30 Jan 2024 13:55:39 +0000 Message-ID: <86plxj6i3o.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: 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> 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) Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII 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 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. M. -- Without deviation from the norm, progress is not possible. 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 0D50BC46CD2 for ; Tue, 30 Jan 2024 13:55:54 +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=QByAEdwduoDmtWj3vUFXPZkDus7Gnn8trVg87pvNLcQ=; b=dYt5A7UnsYXo4U RYo9CPSVrKUynDp5hD4HpLToOTklZMijbGDMKAP7sHVoQMnHYLhqZfMwcj12rS+PztbzDmiFswxg3 toxOM/fHUrMNCzxFmHqxkR4IFBrQhLOqwEgbNyP2y/KaMubyQ4BEZnHKe4PRNZ7Q2iB7meUwtIcUO 78LPW5R4dC3hGIx7H1xOrlioj46RONm4BpHAmjeOi9cpll0SGpwMlweofyGa7kGQ8KDWiMxK9nERE nine4c6kqOhxdBd98SP6nsFj/UlEEP/tbcV2jY5ei8wNd/9C9S3q3zLMIMEQwkzr8A5q67C7EA2f+ O6K0f3WtEP0mpcLDvBOw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rUoaX-0000000Gu9M-1bUA; Tue, 30 Jan 2024 13:55:45 +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 1rUoaU-0000000Gu8K-2Og4 for linux-arm-kernel@lists.infradead.org; Tue, 30 Jan 2024 13:55:43 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0149060C00; Tue, 30 Jan 2024 13:55:42 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A2009C433F1; Tue, 30 Jan 2024 13:55:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1706622941; bh=O9wk5zk5KB/gZrbFf6M01U2u6S9XPe+MZFQGHThXnO4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=hEdENMYOdzk5wwBTJ19vRkgpleszmRuKf1i5cb3vFUvdeKXTNueV6YvchwOlUzSjn FNULzn9HVs3YLHBxmzgSGXC45Iiel1/C90Ipy3V0pLOokuSfY4KRCnE2eymQOnOMrU dHqIxgZQEda9vLW8eBrazaE/5+WBDhauibSM6JaZykTQ3wDAfNKnqUxuAemRLW61vE nyaxux5x9kf6cJYWvIz93TGuKdbSM13YB36uc94VEQulIutw+GXXLHMiXfPNO6rsNy HELa9KcDyd98MlobnSTQEusDeYVEPe/VUlqeoEyqDS9whcvvA0NtqVloqcsxhylTk5 Ju4Ztur0+kLzg== 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 1rUoaR-00GHZZ-G6; Tue, 30 Jan 2024 13:55:39 +0000 Date: Tue, 30 Jan 2024 13:55:39 +0000 Message-ID: <86plxj6i3o.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: 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> 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-20240130_055542_719090_7050152A X-CRM114-Status: GOOD ( 49.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: , 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 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. 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