From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753307AbdHOQsj (ORCPT ); Tue, 15 Aug 2017 12:48:39 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:65264 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752617AbdHOQsi (ORCPT ); Tue, 15 Aug 2017 12:48:38 -0400 From: Paul Burton To: Marc Zyngier CC: , Wei Xu , James Hogan , Thomas Gleixner , Jason Cooper , Andrew Lunn , Gregory Clement , Sebastian Hesselbarth , Kevin Cernekee , Florian Fainelli , Chris Zankel , "Max Filippov" , Matt Redfearn Subject: Re: [PATCH 11/12] irqchip/mips-gic: Report that effective affinity is a single target Date: Tue, 15 Aug 2017 09:48:25 -0700 Message-ID: <8369006.oQ7XfiVv7q@np-p-burton> Organization: Imagination Technologies In-Reply-To: <4d46ab22-dc61-b332-4c98-e6a8bb1921a7@arm.com> References: <20170815113447.14507-1-marc.zyngier@arm.com> <13319270.gRd1cmLsry@np-p-burton> <4d46ab22-dc61-b332-4c98-e6a8bb1921a7@arm.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart4160018.j9BXCh6zPA"; micalg=pgp-sha256; protocol="application/pgp-signature" X-Originating-IP: [10.20.1.88] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --nextPart4160018.j9BXCh6zPA Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Hi Marc, On Tuesday, 15 August 2017 09:44:24 PDT Marc Zyngier wrote: > On 15/08/17 17:23, Paul Burton wrote: > > Hi Marc, > > > > On Tuesday, 15 August 2017 04:34:46 PDT Marc Zyngier wrote: > >> The MIPS GIC driver only targets a single CPU at a time, even if > >> the notional affinity is wider. Let's inform the core code > >> about this. > >> > >> Signed-off-by: Marc Zyngier > >> --- > >> > >> drivers/irqchip/Kconfig | 1 + > >> drivers/irqchip/irq-mips-gic.c | 2 ++ > >> 2 files changed, 3 insertions(+) > >> > >> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > >> index 39bfa5b25b54..bca9a88012f0 100644 > >> --- a/drivers/irqchip/Kconfig > >> +++ b/drivers/irqchip/Kconfig > >> @@ -141,6 +141,7 @@ config IRQ_MIPS_CPU > >> > >> select GENERIC_IRQ_IPI if SYS_SUPPORTS_MULTITHREADING > >> select IRQ_DOMAIN > >> select IRQ_DOMAIN_HIERARCHY if GENERIC_IRQ_IPI > >> > >> + select GENERIC_IRQ_EFFECTIVE_AFF_MASK > >> > >> config CLPS711X_IRQCHIP > >> > >> bool > >> > >> diff --git a/drivers/irqchip/irq-mips-gic.c > >> b/drivers/irqchip/irq-mips-gic.c index 6ab1d3afec02..e075cb25fad6 100644 > >> --- a/drivers/irqchip/irq-mips-gic.c > >> +++ b/drivers/irqchip/irq-mips-gic.c > >> @@ -463,6 +463,7 @@ static int gic_set_affinity(struct irq_data *d, const > >> struct cpumask *cpumask, set_bit(irq, > >> pcpu_masks[cpumask_first(&tmp)].pcpu_mask); > >> > >> cpumask_copy(irq_data_get_affinity_mask(d), cpumask); > >> > >> + irq_data_update_effective_affinity(d, cpumask); > > > > This doesn't seem right - it's just setting the effective affinity to the > > same as affinity, not taking into account CPU restrictions at all. I > > think this> > > should be: > > irq_data_update_effective_affinity(d, cpumask_of(cpumask_first(&tmp))); > > > > (or something cleaner but to that effect) > > Gah, you're absolutely right (/me plugs brain back in). How about something > like this: > > diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c > index e075cb25fad6..6461380ff1a4 100644 > --- a/drivers/irqchip/irq-mips-gic.c > +++ b/drivers/irqchip/irq-mips-gic.c > @@ -445,25 +445,27 @@ static int gic_set_affinity(struct irq_data *d, const > struct cpumask *cpumask, unsigned int irq = GIC_HWIRQ_TO_SHARED(d->hwirq); > cpumask_t tmp = CPU_MASK_NONE; > unsigned long flags; > - int i; > + int i, cpu; > > cpumask_and(&tmp, cpumask, cpu_online_mask); > if (cpumask_empty(&tmp)) > return -EINVAL; > > + cpu = cpumask_first(&tmp); > + > /* Assumption : cpumask refers to a single CPU */ > spin_lock_irqsave(&gic_lock, flags); > > /* Re-route this IRQ */ > - gic_map_to_vpe(irq, mips_cm_vp_id(cpumask_first(&tmp))); > + gic_map_to_vpe(irq, mips_cm_vp_id(cpu)); > > /* Update the pcpu_masks */ > for (i = 0; i < min(gic_vpes, NR_CPUS); i++) > clear_bit(irq, pcpu_masks[i].pcpu_mask); > - set_bit(irq, pcpu_masks[cpumask_first(&tmp)].pcpu_mask); > + set_bit(irq, pcpu_masks[cpu].pcpu_mask); > > cpumask_copy(irq_data_get_affinity_mask(d), cpumask); > - irq_data_update_effective_affinity(d, cpumask); > + irq_data_update_effective_affinity(d, cpumask_of(cpu)); > spin_unlock_irqrestore(&gic_lock, flags); > > return IRQ_SET_MASK_OK_NOCOPY; > > Thanks, > > M. That looks better (though patch 37 of my "irqchip: mips-gic: Cleanup & optimisation" series would get you that cpu variable already ;) ). Thanks, Paul --nextPart4160018.j9BXCh6zPA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEELIGR03D5+Fg+69wPgiDZ+mk8HGUFAlmTJdkACgkQgiDZ+mk8 HGXY3xAAhUYMFp/JohOnp/3rCa0+Rs4klUAN5u08H30KeXSdJ72Ctvt5Ax5jREaH bp2De8SActgoeTtpBKkoIq6MJYQnMsK44X6NAY47/LfCzD+xkTuxHuJ03584wgGW LDfbvEWtFsH4FEWtj21N49Vg+y1ZQCEZcwrGIIlfb796GLIjES+qmQgF4AVFXT/S +1XxwvYqRq9Bh++ZR0JqkqneRC6mU8uGCwEHJ2TKYU2IwedZBdBEec1RftF0SOlu B4aI8p+WPzLAXL10ygV/cJLunVpxo2+xf5p+roKz1Dbp9NUPVPVFd4HaJjePiPZX cM9hblTauU/TlwuOvdSVf4oQK7vZbuls0uSpsrBT/6HqTNKAYCn2AU1kxDOUtfBb 9ua3EkNk/TTlG5z1O6j6WJsGPUvoTA7DPobQrf4/eyO/JpJSBI3tvcjtj3W3nzbI soJIDUEV7QYaIZk2GkaG5GYAA1u+DlKB/70rtu1DC4XysEWenSqN2hAPyIR7ceVQ s/CzPdQJIbcU9+suLNa57O7fES2ewB8gLzHstp9T6GbQ/ZouxjLI5B3wz5Zrt7jE JKphYOV8E/AiBoAMjtnLowh7b0n5LUuLNGVZShF3vOUJkSrjdcWChH+vSZE3+LuK Nj+bx0jy1q1os5d0AMiVDo+7/TNtXa3Z3oxD2Adi0GXDlTFKydQ= =CIQB -----END PGP SIGNATURE----- --nextPart4160018.j9BXCh6zPA--