From mboxrd@z Thu Jan 1 00:00:00 1970 Received: with ECARTIS (v1.0.0; list linux-mips); Fri, 18 Aug 2017 19:19:06 +0200 (CEST) Received: from foss.arm.com ([217.140.101.70]:42600 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by eddie.linux-mips.org with ESMTP id S23994922AbdHRRSxzhDvq (ORCPT ); Fri, 18 Aug 2017 19:18:53 +0200 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C40812B; Fri, 18 Aug 2017 10:18:46 -0700 (PDT) Received: from [10.1.207.16] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CF7E73F483; Fri, 18 Aug 2017 10:18:45 -0700 (PDT) Subject: Re: [PATCH 34/38] irqchip: mips-gic: Make pcpu_masks a per-cpu variable To: Paul Burton Cc: linux-mips@linux-mips.org, Jason Cooper , Thomas Gleixner , Ralf Baechle References: <20170813043646.25821-1-paul.burton@imgtec.com> <20170813043646.25821-35-paul.burton@imgtec.com> <80cfe904-c724-26dd-6802-b2f1b49062be@arm.com> <4849840.2gNksj1pBB@np-p-burton> From: Marc Zyngier Organization: ARM Ltd Message-ID: Date: Fri, 18 Aug 2017 18:18:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <4849840.2gNksj1pBB@np-p-burton> Content-Type: text/plain; charset=windows-1252 Content-Language: en-GB Content-Transfer-Encoding: 7bit Return-Path: X-Envelope-To: <"|/home/ecartis/ecartis -s linux-mips"> (uid 0) X-Orcpt: rfc822;linux-mips@linux-mips.org Original-Recipient: rfc822;linux-mips@linux-mips.org X-archive-position: 59682 X-ecartis-version: Ecartis v1.0.0 Sender: linux-mips-bounce@linux-mips.org Errors-to: linux-mips-bounce@linux-mips.org X-original-sender: marc.zyngier@arm.com Precedence: bulk List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: linux-mips X-List-ID: linux-mips List-subscribe: List-owner: List-post: List-archive: X-list: linux-mips On 18/08/17 18:02, Paul Burton wrote: > Hi Marc, > > On Friday, 18 August 2017 08:37:03 PDT Marc Zyngier wrote: >> On 13/08/17 05:36, Paul Burton wrote: >>> Define the pcpu_masks variable using the kernel's standard per-cpu >>> variable support, rather than an open-coded array of structs containing >>> bitmaps. >>> >>> Signed-off-by: Paul Burton >>> Cc: Jason Cooper >>> Cc: Marc Zyngier >>> Cc: Ralf Baechle >>> Cc: Thomas Gleixner >>> Cc: linux-mips@linux-mips.org >>> --- >>> >>> drivers/irqchip/irq-mips-gic.c | 17 ++++++++--------- >>> 1 file changed, 8 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/irqchip/irq-mips-gic.c >>> b/drivers/irqchip/irq-mips-gic.c index feff4bf97577..00153231376a 100644 >>> --- a/drivers/irqchip/irq-mips-gic.c >>> +++ b/drivers/irqchip/irq-mips-gic.c >>> @@ -13,6 +13,7 @@ >>> >>> #include >>> #include >>> #include >>> >>> +#include >>> >>> #include >>> #include >>> >>> @@ -23,6 +24,7 @@ >>> >>> #include >>> >>> #define GIC_MAX_INTRS 256 >>> >>> +#define GIC_MAX_LONGS BITS_TO_LONGS(GIC_MAX_INTRS) >>> >>> /* Add 2 to convert GIC CPU pin to core interrupt */ >>> #define GIC_CPU_PIN_OFFSET 2 >>> >>> @@ -40,11 +42,8 @@ >>> >>> void __iomem *mips_gic_base; >>> >>> -struct gic_pcpu_mask { >>> - DECLARE_BITMAP(pcpu_mask, GIC_MAX_INTRS); >>> -}; >>> +DEFINE_PER_CPU_READ_MOSTLY(unsigned long[GIC_MAX_LONGS], pcpu_masks); >>> >>> -static struct gic_pcpu_mask pcpu_masks[NR_CPUS]; >>> >>> static DEFINE_SPINLOCK(gic_lock); >>> static struct irq_domain *gic_irq_domain; >>> static struct irq_domain *gic_ipi_domain; >>> >>> @@ -137,7 +136,7 @@ static void gic_handle_shared_int(bool chained) >>> >>> DECLARE_BITMAP(intrmask, GIC_MAX_INTRS); >>> >>> /* Get per-cpu bitmaps */ >>> >>> - pcpu_mask = pcpu_masks[smp_processor_id()].pcpu_mask; >>> + pcpu_mask = this_cpu_ptr(pcpu_masks); >>> >>> if (mips_cm_is64) { >>> >>> __ioread64_copy(pending, addr_gic_pend(), >>> >>> @@ -254,8 +253,8 @@ static int gic_set_affinity(struct irq_data *d, const >>> struct cpumask *cpumask,> >>> /* Update the pcpu_masks */ >>> for (i = 0; i < min(gic_vpes, NR_CPUS); i++) >> >> Is there any case where gic_vpes is not equal to nr_cpus? > > Yes - if the kernel is built with CONFIG_NR_CPUS set to something other than > the actual number of VPs (Virtual Processors, ie. hardware threads) in the > system. (VPE, or Virtual Processing Element, is a roughly equivalent term used > in older revisions of the MIPS architecture). > > To be honest I suspect gic_vpes should go away, and for example in this case > we should juse use for_each_possible_cpu(). That's exactly where I was implicitly aiming to. Either NR_CPUS is set to something bigger than the physical number of CPU/threads, and nr_cpus_ids is then the right thing, or NR_CPUS is smaller than that number, and nr_cpu_ids == NR_CPUS. So for_each_possible_cpus should always do the right thing. > The use of gic_vpes here was added > by commit 2a0787051182 ("irqchip/mips-gic: Use gic_vpes instead of NR_CPUS") > but it doesn't really do a good job of explaining why - I suspect Qais felt it > was an optimisation, but that's debatable. Quite. > This code will need adjusting to add multi-cluster support, which is my > ultimate goal here, anyway. So that'll be one of the next things for me to > tackle. Sounds good to me. M. -- Jazz is not dead. It just smells funny...