2022-06-16 06:44:57

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 0/6] genirq/irqchip: RISC-V PLIC cleanup and optimization

This series removes the splinlocks and cpumask operations from the PLIC
driver's hot path. To do that, it first makes the IRQ affinity mask
behavior more consistent between uniprocessor and SMP configurations.
(The Allwinner D1 is a uniprocessor SoC containing a PLIC.)

As far as I know, using the priority to mask interrupts is an intended
usage and will work on all existing implementations. See [1] for more
discussion.

A further optimization is to take advantage of the fact that multiple
IRQs can be claimed at once. This allows removing the mask operations
for oneshot IRQs -- i.e. the combination of IRQCHIP_ONESHOT_SAFE and
IRQCHIP_EOI_THREADED, which is not currently supported. I will send
this as a separate series, since it makes more invasive changes to the
generic IRQ code.

[1]: https://lore.kernel.org/lkml/[email protected]/

Changes in v2:
- New patch to prevent GENERIC_IRQ_IPI from being selected on !SMP

Samuel Holland (6):
genirq: GENERIC_IRQ_EFFECTIVE_AFF_MASK depends on SMP
genirq: GENERIC_IRQ_IPI depends on SMP
genirq: Refactor accessors to use irq_data_get_affinity_mask
genirq: Provide an IRQ affinity mask in non-SMP configs
irqchip/sifive-plic: Make better use of the effective affinity mask
irqchip/sifive-plic: Separate the enable and mask operations

arch/arm/mach-hisi/Kconfig | 2 +-
drivers/irqchip/Kconfig | 19 +++++-----
drivers/irqchip/irq-sifive-plic.c | 61 ++++++++++++++++---------------
include/linux/irq.h | 20 ++++++----
kernel/irq/Kconfig | 2 +
5 files changed, 57 insertions(+), 47 deletions(-)

--
2.35.1


2022-06-16 06:45:49

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 2/6] genirq: GENERIC_IRQ_IPI depends on SMP

The generic IPI code depends on the affinity mask being set for IPI
IRQs. The affinity mask will not be allocated if SMP is disabled.

Signed-off-by: Samuel Holland <[email protected]>
---

Changes in v2:
- New patch to prevent GENERIC_IRQ_IPI from being selected on !SMP

drivers/irqchip/Kconfig | 4 ++--
kernel/irq/Kconfig | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 6f74c144a7cc..68be9eccc897 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -177,7 +177,7 @@ config MADERA_IRQ
config IRQ_MIPS_CPU
bool
select GENERIC_IRQ_CHIP
- select GENERIC_IRQ_IPI if SYS_SUPPORTS_MULTITHREADING
+ select GENERIC_IRQ_IPI if SMP && SYS_SUPPORTS_MULTITHREADING
select IRQ_DOMAIN
select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP

@@ -322,7 +322,7 @@ config KEYSTONE_IRQ

config MIPS_GIC
bool
- select GENERIC_IRQ_IPI
+ select GENERIC_IRQ_IPI if SMP
select MIPS_CM

config INGENIC_IRQ
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index a2a8df39c2bc..db3d174c53d4 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -83,6 +83,7 @@ config IRQ_FASTEOI_HIERARCHY_HANDLERS
# Generic IRQ IPI support
config GENERIC_IRQ_IPI
bool
+ depends on SMP
select IRQ_DOMAIN_HIERARCHY

# Generic MSI interrupt support
--
2.35.1

2022-06-16 06:45:52

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 3/6] genirq: Refactor accessors to use irq_data_get_affinity_mask

A couple of functions directly reference the affinity mask. Route them
through irq_data_get_affinity_mask so they will pick up any refactoring
done there.

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

include/linux/irq.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 505308253d23..69ee4e2f36ce 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -879,16 +879,16 @@ static inline int irq_data_get_node(struct irq_data *d)
return irq_common_data_get_node(d->common);
}

-static inline struct cpumask *irq_get_affinity_mask(int irq)
+static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
{
- struct irq_data *d = irq_get_irq_data(irq);
-
- return d ? d->common->affinity : NULL;
+ return d->common->affinity;
}

-static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
+static inline struct cpumask *irq_get_affinity_mask(int irq)
{
- return d->common->affinity;
+ struct irq_data *d = irq_get_irq_data(irq);
+
+ return d ? irq_data_get_affinity_mask(d) : NULL;
}

#ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
@@ -910,7 +910,7 @@ static inline void irq_data_update_effective_affinity(struct irq_data *d,
static inline
struct cpumask *irq_data_get_effective_affinity_mask(struct irq_data *d)
{
- return d->common->affinity;
+ return irq_data_get_affinity_mask(d);
}
#endif

--
2.35.1

2022-06-16 06:54:09

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 1/6] genirq: GENERIC_IRQ_EFFECTIVE_AFF_MASK depends on SMP

An IRQ's effective affinity can only be different from its configured
affinity if there are multiple CPUs. Make it clear that this option is
only meaningful when SMP is enabled. Most of the relevant code in
irqdesc.c is already hidden behind CONFIG_SMP anyway.

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

arch/arm/mach-hisi/Kconfig | 2 +-
drivers/irqchip/Kconfig | 14 +++++++-------
kernel/irq/Kconfig | 1 +
3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index 75cccbd3f05f..7b3440687176 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -40,7 +40,7 @@ config ARCH_HIP04
select HAVE_ARM_ARCH_TIMER
select MCPM if SMP
select MCPM_QUAD_CLUSTER if SMP
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
help
Support for Hisilicon HiP04 SoC family

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 4ab1038b5482..6f74c144a7cc 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -8,7 +8,7 @@ config IRQCHIP
config ARM_GIC
bool
select IRQ_DOMAIN_HIERARCHY
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP

config ARM_GIC_PM
bool
@@ -34,7 +34,7 @@ config ARM_GIC_V3
bool
select IRQ_DOMAIN_HIERARCHY
select PARTITION_PERCPU
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP

config ARM_GIC_V3_ITS
bool
@@ -76,7 +76,7 @@ config ARMADA_370_XP_IRQ
bool
select GENERIC_IRQ_CHIP
select PCI_MSI if PCI
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP

config ALPINE_MSI
bool
@@ -112,7 +112,7 @@ config BCM6345_L1_IRQ
bool
select GENERIC_IRQ_CHIP
select IRQ_DOMAIN
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP

config BCM7038_L1_IRQ
tristate "Broadcom STB 7038-style L1/L2 interrupt controller driver"
@@ -120,7 +120,7 @@ config BCM7038_L1_IRQ
default ARCH_BRCMSTB || BMIPS_GENERIC
select GENERIC_IRQ_CHIP
select IRQ_DOMAIN
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP

config BCM7120_L2_IRQ
tristate "Broadcom STB 7120-style L2 interrupt controller driver"
@@ -179,7 +179,7 @@ config IRQ_MIPS_CPU
select GENERIC_IRQ_CHIP
select GENERIC_IRQ_IPI if SYS_SUPPORTS_MULTITHREADING
select IRQ_DOMAIN
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP

config CLPS711X_IRQCHIP
bool
@@ -294,7 +294,7 @@ config VERSATILE_FPGA_IRQ_NR
config XTENSA_MX
bool
select IRQ_DOMAIN
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP

config XILINX_INTC
bool "Xilinx Interrupt Controller IP"
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 10929eda9825..a2a8df39c2bc 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -24,6 +24,7 @@ config GENERIC_IRQ_SHOW_LEVEL

# Supports effective affinity mask
config GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ depends on SMP
bool

# Support for delayed migration from interrupt context
--
2.35.1

2022-06-16 06:54:35

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 5/6] irqchip/sifive-plic: Make better use of the effective affinity mask

The PLIC driver already updates the effective affinity mask in its
.irq_set_affinity callback. Take advantage of that information to only
touch bits (and take spinlocks) for the specific relevant hart contexts.

First, make sure the effective affinity mask is set before IRQ startup.
Since this mask already takes priv->lmask into account, checking that
mask later is no longer needed (and handler->present is equivalent to
the bit being set in priv->lmask).

Then, when (un)masking or changing affinity, only clear/set the enable
bits in the specific old/new context(s). The cpumask operations in
plic_irq_unmask() are not needed because they duplicate the code in
plic_set_affinity().

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

drivers/irqchip/Kconfig | 1 +
drivers/irqchip/irq-sifive-plic.c | 26 ++++++++------------------
2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 68be9eccc897..ccaa13b727c9 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -530,6 +530,7 @@ config SIFIVE_PLIC
bool "SiFive Platform-Level Interrupt Controller"
depends on RISCV
select IRQ_DOMAIN_HIERARCHY
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
help
This enables support for the PLIC chip found in SiFive (and
potentially other) RISC-V systems. The PLIC controls devices
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index bb87e4c3b88e..bf7d5bee0c0c 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -109,31 +109,18 @@ static inline void plic_irq_toggle(const struct cpumask *mask,
for_each_cpu(cpu, mask) {
struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);

- if (handler->present &&
- cpumask_test_cpu(cpu, &handler->priv->lmask))
- plic_toggle(handler, d->hwirq, enable);
+ plic_toggle(handler, d->hwirq, enable);
}
}

static void plic_irq_unmask(struct irq_data *d)
{
- struct cpumask amask;
- unsigned int cpu;
- struct plic_priv *priv = irq_data_get_irq_chip_data(d);
-
- cpumask_and(&amask, &priv->lmask, cpu_online_mask);
- cpu = cpumask_any_and(irq_data_get_affinity_mask(d),
- &amask);
- if (WARN_ON_ONCE(cpu >= nr_cpu_ids))
- return;
- plic_irq_toggle(cpumask_of(cpu), d, 1);
+ plic_irq_toggle(irq_data_get_effective_affinity_mask(d), d, 1);
}

static void plic_irq_mask(struct irq_data *d)
{
- struct plic_priv *priv = irq_data_get_irq_chip_data(d);
-
- plic_irq_toggle(&priv->lmask, d, 0);
+ plic_irq_toggle(irq_data_get_effective_affinity_mask(d), d, 0);
}

#ifdef CONFIG_SMP
@@ -154,11 +141,13 @@ static int plic_set_affinity(struct irq_data *d,
if (cpu >= nr_cpu_ids)
return -EINVAL;

- plic_irq_toggle(&priv->lmask, d, 0);
- plic_irq_toggle(cpumask_of(cpu), d, !irqd_irq_masked(d));
+ plic_irq_mask(d);

irq_data_update_effective_affinity(d, cpumask_of(cpu));

+ if (!irqd_irq_masked(d))
+ plic_irq_unmask(d);
+
return IRQ_SET_MASK_OK_DONE;
}
#endif
@@ -184,6 +173,7 @@ static struct irq_chip plic_chip = {
#ifdef CONFIG_SMP
.irq_set_affinity = plic_set_affinity,
#endif
+ .flags = IRQCHIP_AFFINITY_PRE_STARTUP,
};

static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
--
2.35.1

2022-06-16 06:55:11

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 4/6] genirq: Provide an IRQ affinity mask in non-SMP configs

IRQ affinity masks are not allocated in uniprocessor configurations.
This requires special case non-SMP code in drivers for irqchips which
have per-CPU enable or mask registers.

Since IRQ affinity is always the same in a uniprocessor configuration,
we can still provide the correct affinity mask without allocating one
per IRQ. We can reuse the system-wide cpu_possible_mask.

By returning a real cpumask from irq_data_get_affinity_mask even when
SMP is disabled, irqchip drivers which iterate over that mask will
automatically do the right thing.

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

include/linux/irq.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 69ee4e2f36ce..d5e958b026aa 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -151,7 +151,9 @@ struct irq_common_data {
#endif
void *handler_data;
struct msi_desc *msi_desc;
+#ifdef CONFIG_SMP
cpumask_var_t affinity;
+#endif
#ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
cpumask_var_t effective_affinity;
#endif
@@ -881,7 +883,11 @@ static inline int irq_data_get_node(struct irq_data *d)

static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
{
+#ifdef CONFIG_SMP
return d->common->affinity;
+#else
+ return &__cpu_possible_mask;
+#endif
}

static inline struct cpumask *irq_get_affinity_mask(int irq)
--
2.35.1

2022-06-16 07:02:00

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 6/6] irqchip/sifive-plic: Separate the enable and mask operations

The PLIC has two per-IRQ checks before sending an IRQ to a hart context.
First, it checks that the IRQ's priority is nonzero. Then, it checks
that the enable bit is set for that combination of IRQ and context.

Currently, the PLIC driver sets both the priority value and the enable
bit in its (un)mask operations. However, modifying the enable bit is
problematic for two reasons:
1) The enable bits are packed, so changes are not atomic and require
taking a spinlock.
2) The following requirement from the PLIC spec, which explains the
racy (un)mask operations in plic_irq_eoi():

If the completion ID does not match an interrupt source
that is currently enabled for the target, the completion
is silently ignored.

Both of these problems are solved by using the priority value to mask
IRQs. Each IRQ has a separate priority register, so writing the priority
value is atomic. And since the enable bit remains set while an IRQ is
masked, the EOI operation works normally. The enable bits are still used
to control the IRQ's affinity.

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

drivers/irqchip/irq-sifive-plic.c | 53 +++++++++++++++++++------------
1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index bf7d5bee0c0c..53d266a571be 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -103,9 +103,7 @@ static inline void plic_irq_toggle(const struct cpumask *mask,
struct irq_data *d, int enable)
{
int cpu;
- struct plic_priv *priv = irq_data_get_irq_chip_data(d);

- writel(enable, priv->regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID);
for_each_cpu(cpu, mask) {
struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);

@@ -113,16 +111,37 @@ static inline void plic_irq_toggle(const struct cpumask *mask,
}
}

-static void plic_irq_unmask(struct irq_data *d)
+static void plic_irq_enable(struct irq_data *d)
{
plic_irq_toggle(irq_data_get_effective_affinity_mask(d), d, 1);
}

-static void plic_irq_mask(struct irq_data *d)
+static void plic_irq_disable(struct irq_data *d)
{
plic_irq_toggle(irq_data_get_effective_affinity_mask(d), d, 0);
}

+static void plic_irq_unmask(struct irq_data *d)
+{
+ struct plic_priv *priv = irq_data_get_irq_chip_data(d);
+
+ writel(1, priv->regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID);
+}
+
+static void plic_irq_mask(struct irq_data *d)
+{
+ struct plic_priv *priv = irq_data_get_irq_chip_data(d);
+
+ writel(0, priv->regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID);
+}
+
+static void plic_irq_eoi(struct irq_data *d)
+{
+ struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
+
+ writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM);
+}
+
#ifdef CONFIG_SMP
static int plic_set_affinity(struct irq_data *d,
const struct cpumask *mask_val, bool force)
@@ -141,32 +160,21 @@ static int plic_set_affinity(struct irq_data *d,
if (cpu >= nr_cpu_ids)
return -EINVAL;

- plic_irq_mask(d);
+ plic_irq_disable(d);

irq_data_update_effective_affinity(d, cpumask_of(cpu));

- if (!irqd_irq_masked(d))
- plic_irq_unmask(d);
+ if (!irqd_irq_disabled(d))
+ plic_irq_enable(d);

return IRQ_SET_MASK_OK_DONE;
}
#endif

-static void plic_irq_eoi(struct irq_data *d)
-{
- struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
-
- if (irqd_irq_masked(d)) {
- plic_irq_unmask(d);
- writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM);
- plic_irq_mask(d);
- } else {
- writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM);
- }
-}
-
static struct irq_chip plic_chip = {
.name = "SiFive PLIC",
+ .irq_enable = plic_irq_enable,
+ .irq_disable = plic_irq_disable,
.irq_mask = plic_irq_mask,
.irq_unmask = plic_irq_unmask,
.irq_eoi = plic_irq_eoi,
@@ -372,8 +380,11 @@ static int __init plic_init(struct device_node *node,
i * CONTEXT_ENABLE_SIZE;
handler->priv = priv;
done:
- for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
+ for (hwirq = 1; hwirq <= nr_irqs; hwirq++) {
plic_toggle(handler, hwirq, 0);
+ writel(1, priv->regs + PRIORITY_BASE +
+ hwirq * PRIORITY_PER_ID);
+ }
nr_handlers++;
}

--
2.35.1

2022-06-18 09:05:34

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] genirq: Provide an IRQ affinity mask in non-SMP configs

Hi Samuel,

On Thu, 16 Jun 2022 07:40:26 +0100,
Samuel Holland <[email protected]> wrote:
>
> IRQ affinity masks are not allocated in uniprocessor configurations.
> This requires special case non-SMP code in drivers for irqchips which
> have per-CPU enable or mask registers.
>
> Since IRQ affinity is always the same in a uniprocessor configuration,
> we can still provide the correct affinity mask without allocating one
> per IRQ. We can reuse the system-wide cpu_possible_mask.
>
> By returning a real cpumask from irq_data_get_affinity_mask even when
> SMP is disabled, irqchip drivers which iterate over that mask will
> automatically do the right thing.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> (no changes since v1)
>
> include/linux/irq.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 69ee4e2f36ce..d5e958b026aa 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -151,7 +151,9 @@ struct irq_common_data {
> #endif
> void *handler_data;
> struct msi_desc *msi_desc;
> +#ifdef CONFIG_SMP
> cpumask_var_t affinity;
> +#endif
> #ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
> cpumask_var_t effective_affinity;
> #endif
> @@ -881,7 +883,11 @@ static inline int irq_data_get_node(struct irq_data *d)
>
> static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
> {
> +#ifdef CONFIG_SMP
> return d->common->affinity;
> +#else
> + return &__cpu_possible_mask;
> +#endif

I have a bad feeling about this one. Being in a !SMP configuration
doesn't necessarily mean that __cpu_possible_mask only contains a
single CPU, specially with things like CONFIG_INIT_ALL_POSSIBLE. I can
also imagine an architecture populating this bitmap from firmware
tables irrespective of the SMP status of the kernel.

Can't you use something like:

return cpumask_of(0);

which is guaranteed to be the right thing on !SMP configuration?

Thanks,

M.

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

2022-06-20 05:26:40

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] genirq: GENERIC_IRQ_IPI depends on SMP

Hi Samuel,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/irq/core]
[also build test ERROR on soc/for-next linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Samuel-Holland/genirq-irqchip-RISC-V-PLIC-cleanup-and-optimization/20220616-144300
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git ac165aab469895de059a4a191a2e04ddb5421d0e
config: mips-allnoconfig (https://download.01.org/0day-ci/archive/20220620/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/f8ac62a249f067a05a64eec484ac235137f2c36e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Samuel-Holland/genirq-irqchip-RISC-V-PLIC-cleanup-and-optimization/20220616-144300
git checkout f8ac62a249f067a05a64eec484ac235137f2c36e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/irqchip/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

>> drivers/irqchip/irq-mips-gic.c:479:9: error: call to undeclared function 'irq_domain_set_hwirq_and_chip'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
err = irq_domain_set_hwirq_and_chip(d, virq, hwirq,
^
drivers/irqchip/irq-mips-gic.c:510:9: error: call to undeclared function 'irq_domain_set_hwirq_and_chip'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
err = irq_domain_set_hwirq_and_chip(d, virq, hwirq,
^
drivers/irqchip/irq-mips-gic.c:558:6: warning: no previous prototype for function 'gic_irq_domain_free' [-Wmissing-prototypes]
void gic_irq_domain_free(struct irq_domain *d, unsigned int virq,
^
drivers/irqchip/irq-mips-gic.c:558:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void gic_irq_domain_free(struct irq_domain *d, unsigned int virq,
^
static
>> drivers/irqchip/irq-mips-gic.c:565:3: error: field designator 'alloc' does not refer to any field in type 'const struct irq_domain_ops'
.alloc = gic_irq_domain_alloc,
^
>> drivers/irqchip/irq-mips-gic.c:566:3: error: field designator 'free' does not refer to any field in type 'const struct irq_domain_ops'
.free = gic_irq_domain_free,
^
drivers/irqchip/irq-mips-gic.c:608:9: error: call to undeclared function 'irq_domain_set_hwirq_and_chip'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
ret = irq_domain_set_hwirq_and_chip(d, virq + i, hwirq,
^
>> drivers/irqchip/irq-mips-gic.c:614:42: error: no member named 'parent' in 'struct irq_domain'
ret = irq_domain_set_hwirq_and_chip(d->parent, virq + i, hwirq,
~ ^
drivers/irqchip/irq-mips-gic.c:668:3: error: field designator 'alloc' does not refer to any field in type 'const struct irq_domain_ops'
.alloc = gic_ipi_domain_alloc,
^
drivers/irqchip/irq-mips-gic.c:669:3: error: field designator 'free' does not refer to any field in type 'const struct irq_domain_ops'
.free = gic_ipi_domain_free,
^
>> drivers/irqchip/irq-mips-gic.c:783:19: error: call to undeclared function 'irq_domain_add_hierarchy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
gic_ipi_domain = irq_domain_add_hierarchy(gic_irq_domain,
^
drivers/irqchip/irq-mips-gic.c:783:19: note: did you mean 'irq_domain_is_hierarchy'?
include/linux/irqdomain.h:591:20: note: 'irq_domain_is_hierarchy' declared here
static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
^
>> drivers/irqchip/irq-mips-gic.c:783:17: warning: incompatible integer to pointer conversion assigning to 'struct irq_domain *' from 'int' [-Wint-conversion]
gic_ipi_domain = irq_domain_add_hierarchy(gic_irq_domain,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings and 9 errors generated.


vim +/irq_domain_set_hwirq_and_chip +479 drivers/irqchip/irq-mips-gic.c

b87281e7f205ed Paul Burton 2017-04-20 464
8ada00a650ec7e Matt Redfearn 2017-04-20 465 static int gic_irq_domain_map(struct irq_domain *d, unsigned int virq,
8ada00a650ec7e Matt Redfearn 2017-04-20 466 irq_hw_number_t hwirq)
b87281e7f205ed Paul Burton 2017-04-20 467 {
da61fcf9d62a05 Paul Burton 2017-10-31 468 struct gic_all_vpes_chip_data *cd;
63b746b19fa660 Paul Burton 2017-10-31 469 unsigned long flags;
63b746b19fa660 Paul Burton 2017-10-31 470 unsigned int intr;
da61fcf9d62a05 Paul Burton 2017-10-31 471 int err, cpu;
63b746b19fa660 Paul Burton 2017-10-31 472 u32 map;
6a33fa2b87513f Paul Burton 2016-08-19 473
8ada00a650ec7e Matt Redfearn 2017-04-20 474 if (hwirq >= GIC_SHARED_HWIRQ_BASE) {
b87281e7f205ed Paul Burton 2017-04-20 475 /* verify that shared irqs don't conflict with an IPI irq */
b87281e7f205ed Paul Burton 2017-04-20 476 if (test_bit(GIC_HWIRQ_TO_SHARED(hwirq), ipi_resrv))
b87281e7f205ed Paul Burton 2017-04-20 477 return -EBUSY;
b87281e7f205ed Paul Burton 2017-04-20 478
e875bd66dfb68f Paul Burton 2016-09-13 @479 err = irq_domain_set_hwirq_and_chip(d, virq, hwirq,
e875bd66dfb68f Paul Burton 2016-09-13 480 &gic_level_irq_controller,
e875bd66dfb68f Paul Burton 2016-09-13 481 NULL);
b87281e7f205ed Paul Burton 2017-04-20 482 if (err)
b87281e7f205ed Paul Burton 2017-04-20 483 return err;
b87281e7f205ed Paul Burton 2017-04-20 484
18416e45b76189 Marc Zyngier 2017-08-18 485 irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(virq)));
b87281e7f205ed Paul Burton 2017-04-20 486 return gic_shared_irq_domain_map(d, virq, hwirq, 0);
b87281e7f205ed Paul Burton 2017-04-20 487 }
b87281e7f205ed Paul Burton 2017-04-20 488
63b746b19fa660 Paul Burton 2017-10-31 489 intr = GIC_HWIRQ_TO_LOCAL(hwirq);
63b746b19fa660 Paul Burton 2017-10-31 490 map = GIC_MAP_PIN_MAP_TO_PIN | gic_cpu_pin;
63b746b19fa660 Paul Burton 2017-10-31 491
dd098a0e031928 Marc Zyngier 2021-10-21 492 /*
dd098a0e031928 Marc Zyngier 2021-10-21 493 * If adding support for more per-cpu interrupts, keep the the
dd098a0e031928 Marc Zyngier 2021-10-21 494 * array in gic_all_vpes_irq_cpu_online() in sync.
dd098a0e031928 Marc Zyngier 2021-10-21 495 */
63b746b19fa660 Paul Burton 2017-10-31 496 switch (intr) {
e875bd66dfb68f Paul Burton 2016-09-13 497 case GIC_LOCAL_INT_TIMER:
63b746b19fa660 Paul Burton 2017-10-31 498 /* CONFIG_MIPS_CMP workaround (see __gic_init) */
63b746b19fa660 Paul Burton 2017-10-31 499 map = GIC_MAP_PIN_MAP_TO_PIN | timer_cpu_pin;
df561f6688fef7 Gustavo A. R. Silva 2020-08-23 500 fallthrough;
e875bd66dfb68f Paul Burton 2016-09-13 501 case GIC_LOCAL_INT_PERFCTR:
e875bd66dfb68f Paul Burton 2016-09-13 502 case GIC_LOCAL_INT_FDC:
e875bd66dfb68f Paul Burton 2016-09-13 503 /*
e875bd66dfb68f Paul Burton 2016-09-13 504 * HACK: These are all really percpu interrupts, but
e875bd66dfb68f Paul Burton 2016-09-13 505 * the rest of the MIPS kernel code does not use the
e875bd66dfb68f Paul Burton 2016-09-13 506 * percpu IRQ API for them.
e875bd66dfb68f Paul Burton 2016-09-13 507 */
da61fcf9d62a05 Paul Burton 2017-10-31 508 cd = &gic_all_vpes_chip_data[intr];
da61fcf9d62a05 Paul Burton 2017-10-31 509 cd->map = map;
b87281e7f205ed Paul Burton 2017-04-20 510 err = irq_domain_set_hwirq_and_chip(d, virq, hwirq,
b87281e7f205ed Paul Burton 2017-04-20 511 &gic_all_vpes_local_irq_controller,
da61fcf9d62a05 Paul Burton 2017-10-31 512 cd);
b87281e7f205ed Paul Burton 2017-04-20 513 if (err)
b87281e7f205ed Paul Burton 2017-04-20 514 return err;
b87281e7f205ed Paul Burton 2017-04-20 515
e875bd66dfb68f Paul Burton 2016-09-13 516 irq_set_handler(virq, handle_percpu_irq);
e875bd66dfb68f Paul Burton 2016-09-13 517 break;
e875bd66dfb68f Paul Burton 2016-09-13 518
e875bd66dfb68f Paul Burton 2016-09-13 519 default:
b87281e7f205ed Paul Burton 2017-04-20 520 err = irq_domain_set_hwirq_and_chip(d, virq, hwirq,
b87281e7f205ed Paul Burton 2017-04-20 521 &gic_local_irq_controller,
b87281e7f205ed Paul Burton 2017-04-20 522 NULL);
b87281e7f205ed Paul Burton 2017-04-20 523 if (err)
b87281e7f205ed Paul Burton 2017-04-20 524 return err;
b87281e7f205ed Paul Burton 2017-04-20 525
e875bd66dfb68f Paul Burton 2016-09-13 526 irq_set_handler(virq, handle_percpu_devid_irq);
e875bd66dfb68f Paul Burton 2016-09-13 527 irq_set_percpu_devid(virq);
e875bd66dfb68f Paul Burton 2016-09-13 528 break;
e875bd66dfb68f Paul Burton 2016-09-13 529 }
e875bd66dfb68f Paul Burton 2016-09-13 530
63b746b19fa660 Paul Burton 2017-10-31 531 if (!gic_local_irq_is_routable(intr))
63b746b19fa660 Paul Burton 2017-10-31 532 return -EPERM;
63b746b19fa660 Paul Burton 2017-10-31 533
63b746b19fa660 Paul Burton 2017-10-31 534 spin_lock_irqsave(&gic_lock, flags);
da61fcf9d62a05 Paul Burton 2017-10-31 535 for_each_online_cpu(cpu) {
da61fcf9d62a05 Paul Burton 2017-10-31 536 write_gic_vl_other(mips_cm_vp_id(cpu));
6d4d367d0e9ffa Paul Burton 2019-06-05 537 write_gic_vo_map(mips_gic_vx_map_reg(intr), map);
63b746b19fa660 Paul Burton 2017-10-31 538 }
63b746b19fa660 Paul Burton 2017-10-31 539 spin_unlock_irqrestore(&gic_lock, flags);
63b746b19fa660 Paul Burton 2017-10-31 540
63b746b19fa660 Paul Burton 2017-10-31 541 return 0;
e875bd66dfb68f Paul Burton 2016-09-13 542 }
6a33fa2b87513f Paul Burton 2016-08-19 543
8ada00a650ec7e Matt Redfearn 2017-04-20 544 static int gic_irq_domain_alloc(struct irq_domain *d, unsigned int virq,
8ada00a650ec7e Matt Redfearn 2017-04-20 545 unsigned int nr_irqs, void *arg)
8ada00a650ec7e Matt Redfearn 2017-04-20 546 {
8ada00a650ec7e Matt Redfearn 2017-04-20 547 struct irq_fwspec *fwspec = arg;
8ada00a650ec7e Matt Redfearn 2017-04-20 548 irq_hw_number_t hwirq;
8ada00a650ec7e Matt Redfearn 2017-04-20 549
8ada00a650ec7e Matt Redfearn 2017-04-20 550 if (fwspec->param[0] == GIC_SHARED)
8ada00a650ec7e Matt Redfearn 2017-04-20 551 hwirq = GIC_SHARED_TO_HWIRQ(fwspec->param[1]);
8ada00a650ec7e Matt Redfearn 2017-04-20 552 else
8ada00a650ec7e Matt Redfearn 2017-04-20 553 hwirq = GIC_LOCAL_TO_HWIRQ(fwspec->param[1]);
8ada00a650ec7e Matt Redfearn 2017-04-20 554
8ada00a650ec7e Matt Redfearn 2017-04-20 555 return gic_irq_domain_map(d, virq, hwirq);
8ada00a650ec7e Matt Redfearn 2017-04-20 556 }
8ada00a650ec7e Matt Redfearn 2017-04-20 557
b87281e7f205ed Paul Burton 2017-04-20 558 void gic_irq_domain_free(struct irq_domain *d, unsigned int virq,
b87281e7f205ed Paul Burton 2017-04-20 559 unsigned int nr_irqs)
b87281e7f205ed Paul Burton 2017-04-20 560 {
e9de688dac6534 Andrew Bresticker 2014-09-18 561 }
e9de688dac6534 Andrew Bresticker 2014-09-18 562
b87281e7f205ed Paul Burton 2017-04-20 563 static const struct irq_domain_ops gic_irq_domain_ops = {
b87281e7f205ed Paul Burton 2017-04-20 564 .xlate = gic_irq_domain_xlate,
b87281e7f205ed Paul Burton 2017-04-20 @565 .alloc = gic_irq_domain_alloc,
b87281e7f205ed Paul Burton 2017-04-20 @566 .free = gic_irq_domain_free,
8ada00a650ec7e Matt Redfearn 2017-04-20 567 .map = gic_irq_domain_map,
b87281e7f205ed Paul Burton 2017-04-20 568 };
b87281e7f205ed Paul Burton 2017-04-20 569
b87281e7f205ed Paul Burton 2017-04-20 570 static int gic_ipi_domain_xlate(struct irq_domain *d, struct device_node *ctrlr,
b87281e7f205ed Paul Burton 2017-04-20 571 const u32 *intspec, unsigned int intsize,
b87281e7f205ed Paul Burton 2017-04-20 572 irq_hw_number_t *out_hwirq,
b87281e7f205ed Paul Burton 2017-04-20 573 unsigned int *out_type)
b87281e7f205ed Paul Burton 2017-04-20 574 {
b87281e7f205ed Paul Burton 2017-04-20 575 /*
b87281e7f205ed Paul Burton 2017-04-20 576 * There's nothing to translate here. hwirq is dynamically allocated and
b87281e7f205ed Paul Burton 2017-04-20 577 * the irq type is always edge triggered.
b87281e7f205ed Paul Burton 2017-04-20 578 * */
b87281e7f205ed Paul Burton 2017-04-20 579 *out_hwirq = 0;
b87281e7f205ed Paul Burton 2017-04-20 580 *out_type = IRQ_TYPE_EDGE_RISING;
b87281e7f205ed Paul Burton 2017-04-20 581
b87281e7f205ed Paul Burton 2017-04-20 582 return 0;
b87281e7f205ed Paul Burton 2017-04-20 583 }
b87281e7f205ed Paul Burton 2017-04-20 584
b87281e7f205ed Paul Burton 2017-04-20 585 static int gic_ipi_domain_alloc(struct irq_domain *d, unsigned int virq,
2af70a962070fd Qais Yousef 2015-12-08 586 unsigned int nr_irqs, void *arg)
2af70a962070fd Qais Yousef 2015-12-08 587 {
b87281e7f205ed Paul Burton 2017-04-20 588 struct cpumask *ipimask = arg;
2af70a962070fd Qais Yousef 2015-12-08 589 irq_hw_number_t hwirq, base_hwirq;
2af70a962070fd Qais Yousef 2015-12-08 590 int cpu, ret, i;
2af70a962070fd Qais Yousef 2015-12-08 591
f8dcd9e81797ae Paul Burton 2017-04-20 592 base_hwirq = find_first_bit(ipi_available, gic_shared_intrs);
b87281e7f205ed Paul Burton 2017-04-20 593 if (base_hwirq == gic_shared_intrs)
2af70a962070fd Qais Yousef 2015-12-08 594 return -ENOMEM;
2af70a962070fd Qais Yousef 2015-12-08 595
2af70a962070fd Qais Yousef 2015-12-08 596 /* check that we have enough space */
2af70a962070fd Qais Yousef 2015-12-08 597 for (i = base_hwirq; i < nr_irqs; i++) {
f8dcd9e81797ae Paul Burton 2017-04-20 598 if (!test_bit(i, ipi_available))
2af70a962070fd Qais Yousef 2015-12-08 599 return -EBUSY;
2af70a962070fd Qais Yousef 2015-12-08 600 }
f8dcd9e81797ae Paul Burton 2017-04-20 601 bitmap_clear(ipi_available, base_hwirq, nr_irqs);
2af70a962070fd Qais Yousef 2015-12-08 602
2af70a962070fd Qais Yousef 2015-12-08 603 /* map the hwirq for each cpu consecutively */
2af70a962070fd Qais Yousef 2015-12-08 604 i = 0;
b87281e7f205ed Paul Burton 2017-04-20 605 for_each_cpu(cpu, ipimask) {
2af70a962070fd Qais Yousef 2015-12-08 606 hwirq = GIC_SHARED_TO_HWIRQ(base_hwirq + i);
2af70a962070fd Qais Yousef 2015-12-08 607
2af70a962070fd Qais Yousef 2015-12-08 608 ret = irq_domain_set_hwirq_and_chip(d, virq + i, hwirq,
b87281e7f205ed Paul Burton 2017-04-20 609 &gic_edge_irq_controller,
b87281e7f205ed Paul Burton 2017-04-20 610 NULL);
b87281e7f205ed Paul Burton 2017-04-20 611 if (ret)
b87281e7f205ed Paul Burton 2017-04-20 612 goto error;
b87281e7f205ed Paul Burton 2017-04-20 613
b87281e7f205ed Paul Burton 2017-04-20 @614 ret = irq_domain_set_hwirq_and_chip(d->parent, virq + i, hwirq,
b87281e7f205ed Paul Burton 2017-04-20 615 &gic_edge_irq_controller,
2af70a962070fd Qais Yousef 2015-12-08 616 NULL);
2af70a962070fd Qais Yousef 2015-12-08 617 if (ret)
2af70a962070fd Qais Yousef 2015-12-08 618 goto error;
2af70a962070fd Qais Yousef 2015-12-08 619
b87281e7f205ed Paul Burton 2017-04-20 620 ret = irq_set_irq_type(virq + i, IRQ_TYPE_EDGE_RISING);
b87281e7f205ed Paul Burton 2017-04-20 621 if (ret)
b87281e7f205ed Paul Burton 2017-04-20 622 goto error;
6a33fa2b87513f Paul Burton 2016-08-19 623
2af70a962070fd Qais Yousef 2015-12-08 624 ret = gic_shared_irq_domain_map(d, virq + i, hwirq, cpu);
2af70a962070fd Qais Yousef 2015-12-08 625 if (ret)
2af70a962070fd Qais Yousef 2015-12-08 626 goto error;
2af70a962070fd Qais Yousef 2015-12-08 627
2af70a962070fd Qais Yousef 2015-12-08 628 i++;
2af70a962070fd Qais Yousef 2015-12-08 629 }
2af70a962070fd Qais Yousef 2015-12-08 630
2af70a962070fd Qais Yousef 2015-12-08 631 return 0;
2af70a962070fd Qais Yousef 2015-12-08 632 error:
f8dcd9e81797ae Paul Burton 2017-04-20 633 bitmap_set(ipi_available, base_hwirq, nr_irqs);
2af70a962070fd Qais Yousef 2015-12-08 634 return ret;
2af70a962070fd Qais Yousef 2015-12-08 635 }
2af70a962070fd Qais Yousef 2015-12-08 636

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-21 04:36:13

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] genirq: Provide an IRQ affinity mask in non-SMP configs

On 6/18/22 4:01 AM, Marc Zyngier wrote:
> Hi Samuel,
>
> On Thu, 16 Jun 2022 07:40:26 +0100,
> Samuel Holland <[email protected]> wrote:
>>
>> IRQ affinity masks are not allocated in uniprocessor configurations.
>> This requires special case non-SMP code in drivers for irqchips which
>> have per-CPU enable or mask registers.
>>
>> Since IRQ affinity is always the same in a uniprocessor configuration,
>> we can still provide the correct affinity mask without allocating one
>> per IRQ. We can reuse the system-wide cpu_possible_mask.
>>
>> By returning a real cpumask from irq_data_get_affinity_mask even when
>> SMP is disabled, irqchip drivers which iterate over that mask will
>> automatically do the right thing.
>>
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>>
>> (no changes since v1)
>>
>> include/linux/irq.h | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index 69ee4e2f36ce..d5e958b026aa 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -151,7 +151,9 @@ struct irq_common_data {
>> #endif
>> void *handler_data;
>> struct msi_desc *msi_desc;
>> +#ifdef CONFIG_SMP
>> cpumask_var_t affinity;
>> +#endif
>> #ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
>> cpumask_var_t effective_affinity;
>> #endif
>> @@ -881,7 +883,11 @@ static inline int irq_data_get_node(struct irq_data *d)
>>
>> static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
>> {
>> +#ifdef CONFIG_SMP
>> return d->common->affinity;
>> +#else
>> + return &__cpu_possible_mask;
>> +#endif
>
> I have a bad feeling about this one. Being in a !SMP configuration
> doesn't necessarily mean that __cpu_possible_mask only contains a
> single CPU, specially with things like CONFIG_INIT_ALL_POSSIBLE. I can
> also imagine an architecture populating this bitmap from firmware
> tables irrespective of the SMP status of the kernel.
>
> Can't you use something like:
>
> return cpumask_of(0);
>
> which is guaranteed to be the right thing on !SMP configuration?

I can if I cast away the const. However I see a lot of:

cpumask_copy(irq_data_get_affinity_mask(d), foo);

which I suppose is a great reason not to do what I am doing. The right solution
seems to be adding irq_data_update_affinity() to match
irq_data_update_effective_affinity(), and making both getters return a const
cpumask. Then I can use cpumask_of(0).

Regards,
Samuel

2022-06-21 08:24:11

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] genirq: Provide an IRQ affinity mask in non-SMP configs

On Tue, 21 Jun 2022 05:03:43 +0100,
Samuel Holland <[email protected]> wrote:
>
> On 6/18/22 4:01 AM, Marc Zyngier wrote:
> > Hi Samuel,
> >
> > On Thu, 16 Jun 2022 07:40:26 +0100,
> > Samuel Holland <[email protected]> wrote:
> >>
> >> IRQ affinity masks are not allocated in uniprocessor configurations.
> >> This requires special case non-SMP code in drivers for irqchips which
> >> have per-CPU enable or mask registers.
> >>
> >> Since IRQ affinity is always the same in a uniprocessor configuration,
> >> we can still provide the correct affinity mask without allocating one
> >> per IRQ. We can reuse the system-wide cpu_possible_mask.
> >>
> >> By returning a real cpumask from irq_data_get_affinity_mask even when
> >> SMP is disabled, irqchip drivers which iterate over that mask will
> >> automatically do the right thing.
> >>
> >> Signed-off-by: Samuel Holland <[email protected]>
> >> ---
> >>
> >> (no changes since v1)
> >>
> >> include/linux/irq.h | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/include/linux/irq.h b/include/linux/irq.h
> >> index 69ee4e2f36ce..d5e958b026aa 100644
> >> --- a/include/linux/irq.h
> >> +++ b/include/linux/irq.h
> >> @@ -151,7 +151,9 @@ struct irq_common_data {
> >> #endif
> >> void *handler_data;
> >> struct msi_desc *msi_desc;
> >> +#ifdef CONFIG_SMP
> >> cpumask_var_t affinity;
> >> +#endif
> >> #ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
> >> cpumask_var_t effective_affinity;
> >> #endif
> >> @@ -881,7 +883,11 @@ static inline int irq_data_get_node(struct irq_data *d)
> >>
> >> static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
> >> {
> >> +#ifdef CONFIG_SMP
> >> return d->common->affinity;
> >> +#else
> >> + return &__cpu_possible_mask;
> >> +#endif
> >
> > I have a bad feeling about this one. Being in a !SMP configuration
> > doesn't necessarily mean that __cpu_possible_mask only contains a
> > single CPU, specially with things like CONFIG_INIT_ALL_POSSIBLE. I can
> > also imagine an architecture populating this bitmap from firmware
> > tables irrespective of the SMP status of the kernel.
> >
> > Can't you use something like:
> >
> > return cpumask_of(0);
> >
> > which is guaranteed to be the right thing on !SMP configuration?
>
> I can if I cast away the const. However I see a lot of:
>
> cpumask_copy(irq_data_get_affinity_mask(d), foo);
>
> which I suppose is a great reason not to do what I am doing.

Ah, indeed. Not going to work very well...

> The right solution seems to be adding irq_data_update_affinity() to
> match irq_data_update_effective_affinity(), and making both getters
> return a const cpumask. Then I can use cpumask_of(0).

Sounds like a plan.

Thanks,

M.

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