2022-07-01 20:25:40

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v3 6/8] genirq: Add and use an irq_data_update_affinity helper

Some architectures and irqchip drivers modify the cpumask returned by
irq_data_get_affinity_mask, usually by copying in to it. This is
problematic for uniprocessor configurations, where the affinity mask
should be constant, as it is known at compile time.

Add and use a setter for the affinity mask, following the pattern of
irq_data_update_effective_affinity. This allows the getter function to
return a const cpumask pointer.

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

Changes in v3:
- New patch to introduce irq_data_update_affinity

arch/alpha/kernel/irq.c | 2 +-
arch/ia64/kernel/iosapic.c | 2 +-
arch/ia64/kernel/irq.c | 4 ++--
arch/ia64/kernel/msi_ia64.c | 4 ++--
arch/parisc/kernel/irq.c | 2 +-
drivers/irqchip/irq-bcm6345-l1.c | 4 ++--
drivers/parisc/iosapic.c | 2 +-
drivers/sh/intc/chip.c | 2 +-
drivers/xen/events/events_base.c | 7 ++++---
include/linux/irq.h | 6 ++++++
10 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/arch/alpha/kernel/irq.c b/arch/alpha/kernel/irq.c
index f6d2946edbd2..15f2effd6baf 100644
--- a/arch/alpha/kernel/irq.c
+++ b/arch/alpha/kernel/irq.c
@@ -60,7 +60,7 @@ int irq_select_affinity(unsigned int irq)
cpu = (cpu < (NR_CPUS-1) ? cpu + 1 : 0);
last_cpu = cpu;

- cpumask_copy(irq_data_get_affinity_mask(data), cpumask_of(cpu));
+ irq_data_update_affinity(data, cpumask_of(cpu));
chip->irq_set_affinity(data, cpumask_of(cpu), false);
return 0;
}
diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
index 35adcf89035a..99300850abc1 100644
--- a/arch/ia64/kernel/iosapic.c
+++ b/arch/ia64/kernel/iosapic.c
@@ -834,7 +834,7 @@ iosapic_unregister_intr (unsigned int gsi)
if (iosapic_intr_info[irq].count == 0) {
#ifdef CONFIG_SMP
/* Clear affinity */
- cpumask_setall(irq_get_affinity_mask(irq));
+ irq_data_update_affinity(irq_get_irq_data(irq), cpu_all_mask);
#endif
/* Clear the interrupt information */
iosapic_intr_info[irq].dest = 0;
diff --git a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
index ecef17c7c35b..275b9ea58c64 100644
--- a/arch/ia64/kernel/irq.c
+++ b/arch/ia64/kernel/irq.c
@@ -57,8 +57,8 @@ static char irq_redir [NR_IRQS]; // = { [0 ... NR_IRQS-1] = 1 };
void set_irq_affinity_info (unsigned int irq, int hwid, int redir)
{
if (irq < NR_IRQS) {
- cpumask_copy(irq_get_affinity_mask(irq),
- cpumask_of(cpu_logical_id(hwid)));
+ irq_data_update_affinity(irq_get_irq_data(irq),
+ cpumask_of(cpu_logical_id(hwid)));
irq_redir[irq] = (char) (redir & 0xff);
}
}
diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
index df5c28f252e3..025e5133c860 100644
--- a/arch/ia64/kernel/msi_ia64.c
+++ b/arch/ia64/kernel/msi_ia64.c
@@ -37,7 +37,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
msg.data = data;

pci_write_msi_msg(irq, &msg);
- cpumask_copy(irq_data_get_affinity_mask(idata), cpumask_of(cpu));
+ irq_data_update_affinity(idata, cpumask_of(cpu));

return 0;
}
@@ -132,7 +132,7 @@ static int dmar_msi_set_affinity(struct irq_data *data,
msg.address_lo |= MSI_ADDR_DEST_ID_CPU(cpu_physical_id(cpu));

dmar_msi_write(irq, &msg);
- cpumask_copy(irq_data_get_affinity_mask(data), mask);
+ irq_data_update_affinity(data, mask);

return 0;
}
diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
index 0fe2d79fb123..5ebb1771b4ab 100644
--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -315,7 +315,7 @@ unsigned long txn_affinity_addr(unsigned int irq, int cpu)
{
#ifdef CONFIG_SMP
struct irq_data *d = irq_get_irq_data(irq);
- cpumask_copy(irq_data_get_affinity_mask(d), cpumask_of(cpu));
+ irq_data_update_affinity(d, cpumask_of(cpu));
#endif

return per_cpu(cpu_data, cpu).txn_addr;
diff --git a/drivers/irqchip/irq-bcm6345-l1.c b/drivers/irqchip/irq-bcm6345-l1.c
index 142a7431745f..6899e37810a8 100644
--- a/drivers/irqchip/irq-bcm6345-l1.c
+++ b/drivers/irqchip/irq-bcm6345-l1.c
@@ -216,11 +216,11 @@ static int bcm6345_l1_set_affinity(struct irq_data *d,
enabled = intc->cpus[old_cpu]->enable_cache[word] & mask;
if (enabled)
__bcm6345_l1_mask(d);
- cpumask_copy(irq_data_get_affinity_mask(d), dest);
+ irq_data_update_affinity(d, dest);
if (enabled)
__bcm6345_l1_unmask(d);
} else {
- cpumask_copy(irq_data_get_affinity_mask(d), dest);
+ irq_data_update_affinity(d, dest);
}
raw_spin_unlock_irqrestore(&intc->lock, flags);

diff --git a/drivers/parisc/iosapic.c b/drivers/parisc/iosapic.c
index 8a3b0c3a1e92..3a8c98615634 100644
--- a/drivers/parisc/iosapic.c
+++ b/drivers/parisc/iosapic.c
@@ -677,7 +677,7 @@ static int iosapic_set_affinity_irq(struct irq_data *d,
if (dest_cpu < 0)
return -1;

- cpumask_copy(irq_data_get_affinity_mask(d), cpumask_of(dest_cpu));
+ irq_data_update_affinity(d, cpumask_of(dest_cpu));
vi->txn_addr = txn_affinity_addr(d->irq, dest_cpu);

spin_lock_irqsave(&iosapic_lock, flags);
diff --git a/drivers/sh/intc/chip.c b/drivers/sh/intc/chip.c
index 358df7510186..828d81e02b37 100644
--- a/drivers/sh/intc/chip.c
+++ b/drivers/sh/intc/chip.c
@@ -72,7 +72,7 @@ static int intc_set_affinity(struct irq_data *data,
if (!cpumask_intersects(cpumask, cpu_online_mask))
return -1;

- cpumask_copy(irq_data_get_affinity_mask(data), cpumask);
+ irq_data_update_affinity(data, cpumask);

return IRQ_SET_MASK_OK_NOCOPY;
}
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 46d9295d9a6e..5e8321f43cbd 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -528,9 +528,10 @@ static void bind_evtchn_to_cpu(evtchn_port_t evtchn, unsigned int cpu,
BUG_ON(irq == -1);

if (IS_ENABLED(CONFIG_SMP) && force_affinity) {
- cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
- cpumask_copy(irq_get_effective_affinity_mask(irq),
- cpumask_of(cpu));
+ struct irq_data *data = irq_get_irq_data(irq);
+
+ irq_data_update_affinity(data, cpumask_of(cpu));
+ irq_data_update_effective_affinity(data, cpumask_of(cpu));
}

xen_evtchn_port_bind_to_cpu(evtchn, cpu, info->cpu);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 69ee4e2f36ce..adcfebceb777 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -884,6 +884,12 @@ static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
return d->common->affinity;
}

+static inline void irq_data_update_affinity(struct irq_data *d,
+ const struct cpumask *m)
+{
+ cpumask_copy(d->common->affinity, m);
+}
+
static inline struct cpumask *irq_get_affinity_mask(int irq)
{
struct irq_data *d = irq_get_irq_data(irq);
--
2.35.1


2022-07-03 15:24:21

by Oleksandr Tyshchenko

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] genirq: Add and use an irq_data_update_affinity helper


On 01.07.22 23:00, Samuel Holland wrote:


Hello Samuel

> Some architectures and irqchip drivers modify the cpumask returned by
> irq_data_get_affinity_mask, usually by copying in to it. This is
> problematic for uniprocessor configurations, where the affinity mask
> should be constant, as it is known at compile time.
>
> Add and use a setter for the affinity mask, following the pattern of
> irq_data_update_effective_affinity. This allows the getter function to
> return a const cpumask pointer.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v3:
> - New patch to introduce irq_data_update_affinity
>
> arch/alpha/kernel/irq.c | 2 +-
> arch/ia64/kernel/iosapic.c | 2 +-
> arch/ia64/kernel/irq.c | 4 ++--
> arch/ia64/kernel/msi_ia64.c | 4 ++--
> arch/parisc/kernel/irq.c | 2 +-
> drivers/irqchip/irq-bcm6345-l1.c | 4 ++--
> drivers/parisc/iosapic.c | 2 +-
> drivers/sh/intc/chip.c | 2 +-
> drivers/xen/events/events_base.c | 7 ++++---
> include/linux/irq.h | 6 ++++++
> 10 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/arch/alpha/kernel/irq.c b/arch/alpha/kernel/irq.c
> index f6d2946edbd2..15f2effd6baf 100644
> --- a/arch/alpha/kernel/irq.c
> +++ b/arch/alpha/kernel/irq.c
> @@ -60,7 +60,7 @@ int irq_select_affinity(unsigned int irq)
> cpu = (cpu < (NR_CPUS-1) ? cpu + 1 : 0);
> last_cpu = cpu;
>
> - cpumask_copy(irq_data_get_affinity_mask(data), cpumask_of(cpu));
> + irq_data_update_affinity(data, cpumask_of(cpu));
> chip->irq_set_affinity(data, cpumask_of(cpu), false);
> return 0;
> }
> diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
> index 35adcf89035a..99300850abc1 100644
> --- a/arch/ia64/kernel/iosapic.c
> +++ b/arch/ia64/kernel/iosapic.c
> @@ -834,7 +834,7 @@ iosapic_unregister_intr (unsigned int gsi)
> if (iosapic_intr_info[irq].count == 0) {
> #ifdef CONFIG_SMP
> /* Clear affinity */
> - cpumask_setall(irq_get_affinity_mask(irq));
> + irq_data_update_affinity(irq_get_irq_data(irq), cpu_all_mask);
> #endif
> /* Clear the interrupt information */
> iosapic_intr_info[irq].dest = 0;
> diff --git a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
> index ecef17c7c35b..275b9ea58c64 100644
> --- a/arch/ia64/kernel/irq.c
> +++ b/arch/ia64/kernel/irq.c
> @@ -57,8 +57,8 @@ static char irq_redir [NR_IRQS]; // = { [0 ... NR_IRQS-1] = 1 };
> void set_irq_affinity_info (unsigned int irq, int hwid, int redir)
> {
> if (irq < NR_IRQS) {
> - cpumask_copy(irq_get_affinity_mask(irq),
> - cpumask_of(cpu_logical_id(hwid)));
> + irq_data_update_affinity(irq_get_irq_data(irq),
> + cpumask_of(cpu_logical_id(hwid)));
> irq_redir[irq] = (char) (redir & 0xff);
> }
> }
> diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
> index df5c28f252e3..025e5133c860 100644
> --- a/arch/ia64/kernel/msi_ia64.c
> +++ b/arch/ia64/kernel/msi_ia64.c
> @@ -37,7 +37,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
> msg.data = data;
>
> pci_write_msi_msg(irq, &msg);
> - cpumask_copy(irq_data_get_affinity_mask(idata), cpumask_of(cpu));
> + irq_data_update_affinity(idata, cpumask_of(cpu));
>
> return 0;
> }
> @@ -132,7 +132,7 @@ static int dmar_msi_set_affinity(struct irq_data *data,
> msg.address_lo |= MSI_ADDR_DEST_ID_CPU(cpu_physical_id(cpu));
>
> dmar_msi_write(irq, &msg);
> - cpumask_copy(irq_data_get_affinity_mask(data), mask);
> + irq_data_update_affinity(data, mask);
>
> return 0;
> }
> diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
> index 0fe2d79fb123..5ebb1771b4ab 100644
> --- a/arch/parisc/kernel/irq.c
> +++ b/arch/parisc/kernel/irq.c
> @@ -315,7 +315,7 @@ unsigned long txn_affinity_addr(unsigned int irq, int cpu)
> {
> #ifdef CONFIG_SMP
> struct irq_data *d = irq_get_irq_data(irq);
> - cpumask_copy(irq_data_get_affinity_mask(d), cpumask_of(cpu));
> + irq_data_update_affinity(d, cpumask_of(cpu));
> #endif
>
> return per_cpu(cpu_data, cpu).txn_addr;
> diff --git a/drivers/irqchip/irq-bcm6345-l1.c b/drivers/irqchip/irq-bcm6345-l1.c
> index 142a7431745f..6899e37810a8 100644
> --- a/drivers/irqchip/irq-bcm6345-l1.c
> +++ b/drivers/irqchip/irq-bcm6345-l1.c
> @@ -216,11 +216,11 @@ static int bcm6345_l1_set_affinity(struct irq_data *d,
> enabled = intc->cpus[old_cpu]->enable_cache[word] & mask;
> if (enabled)
> __bcm6345_l1_mask(d);
> - cpumask_copy(irq_data_get_affinity_mask(d), dest);
> + irq_data_update_affinity(d, dest);
> if (enabled)
> __bcm6345_l1_unmask(d);
> } else {
> - cpumask_copy(irq_data_get_affinity_mask(d), dest);
> + irq_data_update_affinity(d, dest);
> }
> raw_spin_unlock_irqrestore(&intc->lock, flags);
>
> diff --git a/drivers/parisc/iosapic.c b/drivers/parisc/iosapic.c
> index 8a3b0c3a1e92..3a8c98615634 100644
> --- a/drivers/parisc/iosapic.c
> +++ b/drivers/parisc/iosapic.c
> @@ -677,7 +677,7 @@ static int iosapic_set_affinity_irq(struct irq_data *d,
> if (dest_cpu < 0)
> return -1;
>
> - cpumask_copy(irq_data_get_affinity_mask(d), cpumask_of(dest_cpu));
> + irq_data_update_affinity(d, cpumask_of(dest_cpu));
> vi->txn_addr = txn_affinity_addr(d->irq, dest_cpu);
>
> spin_lock_irqsave(&iosapic_lock, flags);
> diff --git a/drivers/sh/intc/chip.c b/drivers/sh/intc/chip.c
> index 358df7510186..828d81e02b37 100644
> --- a/drivers/sh/intc/chip.c
> +++ b/drivers/sh/intc/chip.c
> @@ -72,7 +72,7 @@ static int intc_set_affinity(struct irq_data *data,
> if (!cpumask_intersects(cpumask, cpu_online_mask))
> return -1;
>
> - cpumask_copy(irq_data_get_affinity_mask(data), cpumask);
> + irq_data_update_affinity(data, cpumask);
>
> return IRQ_SET_MASK_OK_NOCOPY;
> }
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 46d9295d9a6e..5e8321f43cbd 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -528,9 +528,10 @@ static void bind_evtchn_to_cpu(evtchn_port_t evtchn, unsigned int cpu,
> BUG_ON(irq == -1);
>
> if (IS_ENABLED(CONFIG_SMP) && force_affinity) {
> - cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
> - cpumask_copy(irq_get_effective_affinity_mask(irq),
> - cpumask_of(cpu));
> + struct irq_data *data = irq_get_irq_data(irq);
> +
> + irq_data_update_affinity(data, cpumask_of(cpu));
> + irq_data_update_effective_affinity(data, cpumask_of(cpu));
> }



Nit: commit description says about reusing irq_data_update_affinity()
only, but here we also reuse irq_data_update_effective_affinity(), so I
would mention that in the description.

Reviewed-by: Oleksandr Tyshchenko <[email protected]> # Xen bits


[snip]

--
Regards,

Oleksandr Tyshchenko

2022-07-07 08:48:23

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] genirq: Add and use an irq_data_update_affinity helper

On Sun, 03 Jul 2022 16:22:03 +0100,
Oleksandr <[email protected]> wrote:
>
>
> On 01.07.22 23:00, Samuel Holland wrote:
>
>
> Hello Samuel
>
> > Some architectures and irqchip drivers modify the cpumask returned by
> > irq_data_get_affinity_mask, usually by copying in to it. This is
> > problematic for uniprocessor configurations, where the affinity mask
> > should be constant, as it is known at compile time.
> >
> > Add and use a setter for the affinity mask, following the pattern of
> > irq_data_update_effective_affinity. This allows the getter function to
> > return a const cpumask pointer.
> >
> > Signed-off-by: Samuel Holland <[email protected]>
> > ---
> >
> > Changes in v3:
> > - New patch to introduce irq_data_update_affinity
> >
> > arch/alpha/kernel/irq.c | 2 +-
> > arch/ia64/kernel/iosapic.c | 2 +-
> > arch/ia64/kernel/irq.c | 4 ++--
> > arch/ia64/kernel/msi_ia64.c | 4 ++--
> > arch/parisc/kernel/irq.c | 2 +-
> > drivers/irqchip/irq-bcm6345-l1.c | 4 ++--
> > drivers/parisc/iosapic.c | 2 +-
> > drivers/sh/intc/chip.c | 2 +-
> > drivers/xen/events/events_base.c | 7 ++++---
> > include/linux/irq.h | 6 ++++++
> > 10 files changed, 21 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/alpha/kernel/irq.c b/arch/alpha/kernel/irq.c
> > index f6d2946edbd2..15f2effd6baf 100644
> > --- a/arch/alpha/kernel/irq.c
> > +++ b/arch/alpha/kernel/irq.c
> > @@ -60,7 +60,7 @@ int irq_select_affinity(unsigned int irq)
> > cpu = (cpu < (NR_CPUS-1) ? cpu + 1 : 0);
> > last_cpu = cpu;
> > - cpumask_copy(irq_data_get_affinity_mask(data),
> > cpumask_of(cpu));
> > + irq_data_update_affinity(data, cpumask_of(cpu));
> > chip->irq_set_affinity(data, cpumask_of(cpu), false);
> > return 0;
> > }
> > diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
> > index 35adcf89035a..99300850abc1 100644
> > --- a/arch/ia64/kernel/iosapic.c
> > +++ b/arch/ia64/kernel/iosapic.c
> > @@ -834,7 +834,7 @@ iosapic_unregister_intr (unsigned int gsi)
> > if (iosapic_intr_info[irq].count == 0) {
> > #ifdef CONFIG_SMP
> > /* Clear affinity */
> > - cpumask_setall(irq_get_affinity_mask(irq));
> > + irq_data_update_affinity(irq_get_irq_data(irq), cpu_all_mask);
> > #endif
> > /* Clear the interrupt information */
> > iosapic_intr_info[irq].dest = 0;
> > diff --git a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
> > index ecef17c7c35b..275b9ea58c64 100644
> > --- a/arch/ia64/kernel/irq.c
> > +++ b/arch/ia64/kernel/irq.c
> > @@ -57,8 +57,8 @@ static char irq_redir [NR_IRQS]; // = { [0 ... NR_IRQS-1] = 1 };
> > void set_irq_affinity_info (unsigned int irq, int hwid, int redir)
> > {
> > if (irq < NR_IRQS) {
> > - cpumask_copy(irq_get_affinity_mask(irq),
> > - cpumask_of(cpu_logical_id(hwid)));
> > + irq_data_update_affinity(irq_get_irq_data(irq),
> > + cpumask_of(cpu_logical_id(hwid)));
> > irq_redir[irq] = (char) (redir & 0xff);
> > }
> > }
> > diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
> > index df5c28f252e3..025e5133c860 100644
> > --- a/arch/ia64/kernel/msi_ia64.c
> > +++ b/arch/ia64/kernel/msi_ia64.c
> > @@ -37,7 +37,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
> > msg.data = data;
> > pci_write_msi_msg(irq, &msg);
> > - cpumask_copy(irq_data_get_affinity_mask(idata), cpumask_of(cpu));
> > + irq_data_update_affinity(idata, cpumask_of(cpu));
> > return 0;
> > }
> > @@ -132,7 +132,7 @@ static int dmar_msi_set_affinity(struct irq_data *data,
> > msg.address_lo |= MSI_ADDR_DEST_ID_CPU(cpu_physical_id(cpu));
> > dmar_msi_write(irq, &msg);
> > - cpumask_copy(irq_data_get_affinity_mask(data), mask);
> > + irq_data_update_affinity(data, mask);
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
> > index 0fe2d79fb123..5ebb1771b4ab 100644
> > --- a/arch/parisc/kernel/irq.c
> > +++ b/arch/parisc/kernel/irq.c
> > @@ -315,7 +315,7 @@ unsigned long txn_affinity_addr(unsigned int irq, int cpu)
> > {
> > #ifdef CONFIG_SMP
> > struct irq_data *d = irq_get_irq_data(irq);
> > - cpumask_copy(irq_data_get_affinity_mask(d), cpumask_of(cpu));
> > + irq_data_update_affinity(d, cpumask_of(cpu));
> > #endif
> > return per_cpu(cpu_data, cpu).txn_addr;
> > diff --git a/drivers/irqchip/irq-bcm6345-l1.c b/drivers/irqchip/irq-bcm6345-l1.c
> > index 142a7431745f..6899e37810a8 100644
> > --- a/drivers/irqchip/irq-bcm6345-l1.c
> > +++ b/drivers/irqchip/irq-bcm6345-l1.c
> > @@ -216,11 +216,11 @@ static int bcm6345_l1_set_affinity(struct irq_data *d,
> > enabled = intc->cpus[old_cpu]->enable_cache[word] & mask;
> > if (enabled)
> > __bcm6345_l1_mask(d);
> > - cpumask_copy(irq_data_get_affinity_mask(d), dest);
> > + irq_data_update_affinity(d, dest);
> > if (enabled)
> > __bcm6345_l1_unmask(d);
> > } else {
> > - cpumask_copy(irq_data_get_affinity_mask(d), dest);
> > + irq_data_update_affinity(d, dest);
> > }
> > raw_spin_unlock_irqrestore(&intc->lock, flags);
> > diff --git a/drivers/parisc/iosapic.c b/drivers/parisc/iosapic.c
> > index 8a3b0c3a1e92..3a8c98615634 100644
> > --- a/drivers/parisc/iosapic.c
> > +++ b/drivers/parisc/iosapic.c
> > @@ -677,7 +677,7 @@ static int iosapic_set_affinity_irq(struct irq_data *d,
> > if (dest_cpu < 0)
> > return -1;
> > - cpumask_copy(irq_data_get_affinity_mask(d),
> > cpumask_of(dest_cpu));
> > + irq_data_update_affinity(d, cpumask_of(dest_cpu));
> > vi->txn_addr = txn_affinity_addr(d->irq, dest_cpu);
> > spin_lock_irqsave(&iosapic_lock, flags);
> > diff --git a/drivers/sh/intc/chip.c b/drivers/sh/intc/chip.c
> > index 358df7510186..828d81e02b37 100644
> > --- a/drivers/sh/intc/chip.c
> > +++ b/drivers/sh/intc/chip.c
> > @@ -72,7 +72,7 @@ static int intc_set_affinity(struct irq_data *data,
> > if (!cpumask_intersects(cpumask, cpu_online_mask))
> > return -1;
> > - cpumask_copy(irq_data_get_affinity_mask(data), cpumask);
> > + irq_data_update_affinity(data, cpumask);
> > return IRQ_SET_MASK_OK_NOCOPY;
> > }
> > diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> > index 46d9295d9a6e..5e8321f43cbd 100644
> > --- a/drivers/xen/events/events_base.c
> > +++ b/drivers/xen/events/events_base.c
> > @@ -528,9 +528,10 @@ static void bind_evtchn_to_cpu(evtchn_port_t evtchn, unsigned int cpu,
> > BUG_ON(irq == -1);
> > if (IS_ENABLED(CONFIG_SMP) && force_affinity) {
> > - cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
> > - cpumask_copy(irq_get_effective_affinity_mask(irq),
> > - cpumask_of(cpu));
> > + struct irq_data *data = irq_get_irq_data(irq);
> > +
> > + irq_data_update_affinity(data, cpumask_of(cpu));
> > + irq_data_update_effective_affinity(data, cpumask_of(cpu));
> > }
>
>
>
> Nit: commit description says about reusing irq_data_update_affinity()
> only, but here we also reuse irq_data_update_effective_affinity(), so
> I would mention that in the description.
>
> Reviewed-by: Oleksandr Tyshchenko <[email protected]> # Xen bits

b4 shouts because of your email address:

NOTE: some trailers ignored due to from/email mismatches:
! Trailer: Reviewed-by: Oleksandr Tyshchenko <[email protected]> # Xen bits
Msg From: Oleksandr <[email protected]>

I've used the tag anyway, but you may want to fix your setup in the
future.

Thanks,

M.

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

Subject: [irqchip: irq/irqchip-next] genirq: Add and use an irq_data_update_affinity helper

The following commit has been merged into the irq/irqchip-next branch of irqchip:

Commit-ID: 073352e951f60946452da358d64841066c3142ff
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/073352e951f60946452da358d64841066c3142ff
Author: Samuel Holland <[email protected]>
AuthorDate: Fri, 01 Jul 2022 15:00:54 -05:00
Committer: Marc Zyngier <[email protected]>
CommitterDate: Thu, 07 Jul 2022 09:38:04 +01:00

genirq: Add and use an irq_data_update_affinity helper

Some architectures and irqchip drivers modify the cpumask returned by
irq_data_get_affinity_mask, usually by copying in to it. This is
problematic for uniprocessor configurations, where the affinity mask
should be constant, as it is known at compile time.

Add and use a setter for the affinity mask, following the pattern of
irq_data_update_effective_affinity. This allows the getter function to
return a const cpumask pointer.

Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Oleksandr Tyshchenko <[email protected]> # Xen bits
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/alpha/kernel/irq.c | 2 +-
arch/ia64/kernel/iosapic.c | 2 +-
arch/ia64/kernel/irq.c | 4 ++--
arch/ia64/kernel/msi_ia64.c | 4 ++--
arch/parisc/kernel/irq.c | 2 +-
drivers/irqchip/irq-bcm6345-l1.c | 4 ++--
drivers/parisc/iosapic.c | 2 +-
drivers/sh/intc/chip.c | 2 +-
drivers/xen/events/events_base.c | 7 ++++---
include/linux/irq.h | 6 ++++++
10 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/arch/alpha/kernel/irq.c b/arch/alpha/kernel/irq.c
index f6d2946..15f2eff 100644
--- a/arch/alpha/kernel/irq.c
+++ b/arch/alpha/kernel/irq.c
@@ -60,7 +60,7 @@ int irq_select_affinity(unsigned int irq)
cpu = (cpu < (NR_CPUS-1) ? cpu + 1 : 0);
last_cpu = cpu;

- cpumask_copy(irq_data_get_affinity_mask(data), cpumask_of(cpu));
+ irq_data_update_affinity(data, cpumask_of(cpu));
chip->irq_set_affinity(data, cpumask_of(cpu), false);
return 0;
}
diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
index 35adcf8..9930085 100644
--- a/arch/ia64/kernel/iosapic.c
+++ b/arch/ia64/kernel/iosapic.c
@@ -834,7 +834,7 @@ iosapic_unregister_intr (unsigned int gsi)
if (iosapic_intr_info[irq].count == 0) {
#ifdef CONFIG_SMP
/* Clear affinity */
- cpumask_setall(irq_get_affinity_mask(irq));
+ irq_data_update_affinity(irq_get_irq_data(irq), cpu_all_mask);
#endif
/* Clear the interrupt information */
iosapic_intr_info[irq].dest = 0;
diff --git a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
index ecef17c..275b9ea 100644
--- a/arch/ia64/kernel/irq.c
+++ b/arch/ia64/kernel/irq.c
@@ -57,8 +57,8 @@ static char irq_redir [NR_IRQS]; // = { [0 ... NR_IRQS-1] = 1 };
void set_irq_affinity_info (unsigned int irq, int hwid, int redir)
{
if (irq < NR_IRQS) {
- cpumask_copy(irq_get_affinity_mask(irq),
- cpumask_of(cpu_logical_id(hwid)));
+ irq_data_update_affinity(irq_get_irq_data(irq),
+ cpumask_of(cpu_logical_id(hwid)));
irq_redir[irq] = (char) (redir & 0xff);
}
}
diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
index df5c28f..025e513 100644
--- a/arch/ia64/kernel/msi_ia64.c
+++ b/arch/ia64/kernel/msi_ia64.c
@@ -37,7 +37,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
msg.data = data;

pci_write_msi_msg(irq, &msg);
- cpumask_copy(irq_data_get_affinity_mask(idata), cpumask_of(cpu));
+ irq_data_update_affinity(idata, cpumask_of(cpu));

return 0;
}
@@ -132,7 +132,7 @@ static int dmar_msi_set_affinity(struct irq_data *data,
msg.address_lo |= MSI_ADDR_DEST_ID_CPU(cpu_physical_id(cpu));

dmar_msi_write(irq, &msg);
- cpumask_copy(irq_data_get_affinity_mask(data), mask);
+ irq_data_update_affinity(data, mask);

return 0;
}
diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
index 0fe2d79..5ebb177 100644
--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -315,7 +315,7 @@ unsigned long txn_affinity_addr(unsigned int irq, int cpu)
{
#ifdef CONFIG_SMP
struct irq_data *d = irq_get_irq_data(irq);
- cpumask_copy(irq_data_get_affinity_mask(d), cpumask_of(cpu));
+ irq_data_update_affinity(d, cpumask_of(cpu));
#endif

return per_cpu(cpu_data, cpu).txn_addr;
diff --git a/drivers/irqchip/irq-bcm6345-l1.c b/drivers/irqchip/irq-bcm6345-l1.c
index 142a743..6899e37 100644
--- a/drivers/irqchip/irq-bcm6345-l1.c
+++ b/drivers/irqchip/irq-bcm6345-l1.c
@@ -216,11 +216,11 @@ static int bcm6345_l1_set_affinity(struct irq_data *d,
enabled = intc->cpus[old_cpu]->enable_cache[word] & mask;
if (enabled)
__bcm6345_l1_mask(d);
- cpumask_copy(irq_data_get_affinity_mask(d), dest);
+ irq_data_update_affinity(d, dest);
if (enabled)
__bcm6345_l1_unmask(d);
} else {
- cpumask_copy(irq_data_get_affinity_mask(d), dest);
+ irq_data_update_affinity(d, dest);
}
raw_spin_unlock_irqrestore(&intc->lock, flags);

diff --git a/drivers/parisc/iosapic.c b/drivers/parisc/iosapic.c
index 8a3b0c3..3a8c986 100644
--- a/drivers/parisc/iosapic.c
+++ b/drivers/parisc/iosapic.c
@@ -677,7 +677,7 @@ static int iosapic_set_affinity_irq(struct irq_data *d,
if (dest_cpu < 0)
return -1;

- cpumask_copy(irq_data_get_affinity_mask(d), cpumask_of(dest_cpu));
+ irq_data_update_affinity(d, cpumask_of(dest_cpu));
vi->txn_addr = txn_affinity_addr(d->irq, dest_cpu);

spin_lock_irqsave(&iosapic_lock, flags);
diff --git a/drivers/sh/intc/chip.c b/drivers/sh/intc/chip.c
index 358df75..828d81e 100644
--- a/drivers/sh/intc/chip.c
+++ b/drivers/sh/intc/chip.c
@@ -72,7 +72,7 @@ static int intc_set_affinity(struct irq_data *data,
if (!cpumask_intersects(cpumask, cpu_online_mask))
return -1;

- cpumask_copy(irq_data_get_affinity_mask(data), cpumask);
+ irq_data_update_affinity(data, cpumask);

return IRQ_SET_MASK_OK_NOCOPY;
}
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 46d9295..5e8321f 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -528,9 +528,10 @@ static void bind_evtchn_to_cpu(evtchn_port_t evtchn, unsigned int cpu,
BUG_ON(irq == -1);

if (IS_ENABLED(CONFIG_SMP) && force_affinity) {
- cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
- cpumask_copy(irq_get_effective_affinity_mask(irq),
- cpumask_of(cpu));
+ struct irq_data *data = irq_get_irq_data(irq);
+
+ irq_data_update_affinity(data, cpumask_of(cpu));
+ irq_data_update_effective_affinity(data, cpumask_of(cpu));
}

xen_evtchn_port_bind_to_cpu(evtchn, cpu, info->cpu);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 69ee4e2..adcfebc 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -884,6 +884,12 @@ static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
return d->common->affinity;
}

+static inline void irq_data_update_affinity(struct irq_data *d,
+ const struct cpumask *m)
+{
+ cpumask_copy(d->common->affinity, m);
+}
+
static inline struct cpumask *irq_get_affinity_mask(int irq)
{
struct irq_data *d = irq_get_irq_data(irq);

2022-07-07 13:43:49

by Oleksandr Tyshchenko

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] genirq: Add and use an irq_data_update_affinity helper


On 07.07.22 11:39, Marc Zyngier wrote:


Hello Marc

> On Sun, 03 Jul 2022 16:22:03 +0100,
> Oleksandr <[email protected]> wrote:
>>
>> On 01.07.22 23:00, Samuel Holland wrote:
>>
>>
>> Hello Samuel
>>
>>> Some architectures and irqchip drivers modify the cpumask returned by
>>> irq_data_get_affinity_mask, usually by copying in to it. This is
>>> problematic for uniprocessor configurations, where the affinity mask
>>> should be constant, as it is known at compile time.
>>>
>>> Add and use a setter for the affinity mask, following the pattern of
>>> irq_data_update_effective_affinity. This allows the getter function to
>>> return a const cpumask pointer.
>>>
>>> Signed-off-by: Samuel Holland <[email protected]>
>>> ---
>>>
>>> Changes in v3:
>>> - New patch to introduce irq_data_update_affinity
>>>
>>> arch/alpha/kernel/irq.c | 2 +-
>>> arch/ia64/kernel/iosapic.c | 2 +-
>>> arch/ia64/kernel/irq.c | 4 ++--
>>> arch/ia64/kernel/msi_ia64.c | 4 ++--
>>> arch/parisc/kernel/irq.c | 2 +-
>>> drivers/irqchip/irq-bcm6345-l1.c | 4 ++--
>>> drivers/parisc/iosapic.c | 2 +-
>>> drivers/sh/intc/chip.c | 2 +-
>>> drivers/xen/events/events_base.c | 7 ++++---
>>> include/linux/irq.h | 6 ++++++
>>> 10 files changed, 21 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/arch/alpha/kernel/irq.c b/arch/alpha/kernel/irq.c
>>> index f6d2946edbd2..15f2effd6baf 100644
>>> --- a/arch/alpha/kernel/irq.c
>>> +++ b/arch/alpha/kernel/irq.c
>>> @@ -60,7 +60,7 @@ int irq_select_affinity(unsigned int irq)
>>> cpu = (cpu < (NR_CPUS-1) ? cpu + 1 : 0);
>>> last_cpu = cpu;
>>> - cpumask_copy(irq_data_get_affinity_mask(data),
>>> cpumask_of(cpu));
>>> + irq_data_update_affinity(data, cpumask_of(cpu));
>>> chip->irq_set_affinity(data, cpumask_of(cpu), false);
>>> return 0;
>>> }
>>> diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
>>> index 35adcf89035a..99300850abc1 100644
>>> --- a/arch/ia64/kernel/iosapic.c
>>> +++ b/arch/ia64/kernel/iosapic.c
>>> @@ -834,7 +834,7 @@ iosapic_unregister_intr (unsigned int gsi)
>>> if (iosapic_intr_info[irq].count == 0) {
>>> #ifdef CONFIG_SMP
>>> /* Clear affinity */
>>> - cpumask_setall(irq_get_affinity_mask(irq));
>>> + irq_data_update_affinity(irq_get_irq_data(irq), cpu_all_mask);
>>> #endif
>>> /* Clear the interrupt information */
>>> iosapic_intr_info[irq].dest = 0;
>>> diff --git a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
>>> index ecef17c7c35b..275b9ea58c64 100644
>>> --- a/arch/ia64/kernel/irq.c
>>> +++ b/arch/ia64/kernel/irq.c
>>> @@ -57,8 +57,8 @@ static char irq_redir [NR_IRQS]; // = { [0 ... NR_IRQS-1] = 1 };
>>> void set_irq_affinity_info (unsigned int irq, int hwid, int redir)
>>> {
>>> if (irq < NR_IRQS) {
>>> - cpumask_copy(irq_get_affinity_mask(irq),
>>> - cpumask_of(cpu_logical_id(hwid)));
>>> + irq_data_update_affinity(irq_get_irq_data(irq),
>>> + cpumask_of(cpu_logical_id(hwid)));
>>> irq_redir[irq] = (char) (redir & 0xff);
>>> }
>>> }
>>> diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
>>> index df5c28f252e3..025e5133c860 100644
>>> --- a/arch/ia64/kernel/msi_ia64.c
>>> +++ b/arch/ia64/kernel/msi_ia64.c
>>> @@ -37,7 +37,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
>>> msg.data = data;
>>> pci_write_msi_msg(irq, &msg);
>>> - cpumask_copy(irq_data_get_affinity_mask(idata), cpumask_of(cpu));
>>> + irq_data_update_affinity(idata, cpumask_of(cpu));
>>> return 0;
>>> }
>>> @@ -132,7 +132,7 @@ static int dmar_msi_set_affinity(struct irq_data *data,
>>> msg.address_lo |= MSI_ADDR_DEST_ID_CPU(cpu_physical_id(cpu));
>>> dmar_msi_write(irq, &msg);
>>> - cpumask_copy(irq_data_get_affinity_mask(data), mask);
>>> + irq_data_update_affinity(data, mask);
>>> return 0;
>>> }
>>> diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
>>> index 0fe2d79fb123..5ebb1771b4ab 100644
>>> --- a/arch/parisc/kernel/irq.c
>>> +++ b/arch/parisc/kernel/irq.c
>>> @@ -315,7 +315,7 @@ unsigned long txn_affinity_addr(unsigned int irq, int cpu)
>>> {
>>> #ifdef CONFIG_SMP
>>> struct irq_data *d = irq_get_irq_data(irq);
>>> - cpumask_copy(irq_data_get_affinity_mask(d), cpumask_of(cpu));
>>> + irq_data_update_affinity(d, cpumask_of(cpu));
>>> #endif
>>> return per_cpu(cpu_data, cpu).txn_addr;
>>> diff --git a/drivers/irqchip/irq-bcm6345-l1.c b/drivers/irqchip/irq-bcm6345-l1.c
>>> index 142a7431745f..6899e37810a8 100644
>>> --- a/drivers/irqchip/irq-bcm6345-l1.c
>>> +++ b/drivers/irqchip/irq-bcm6345-l1.c
>>> @@ -216,11 +216,11 @@ static int bcm6345_l1_set_affinity(struct irq_data *d,
>>> enabled = intc->cpus[old_cpu]->enable_cache[word] & mask;
>>> if (enabled)
>>> __bcm6345_l1_mask(d);
>>> - cpumask_copy(irq_data_get_affinity_mask(d), dest);
>>> + irq_data_update_affinity(d, dest);
>>> if (enabled)
>>> __bcm6345_l1_unmask(d);
>>> } else {
>>> - cpumask_copy(irq_data_get_affinity_mask(d), dest);
>>> + irq_data_update_affinity(d, dest);
>>> }
>>> raw_spin_unlock_irqrestore(&intc->lock, flags);
>>> diff --git a/drivers/parisc/iosapic.c b/drivers/parisc/iosapic.c
>>> index 8a3b0c3a1e92..3a8c98615634 100644
>>> --- a/drivers/parisc/iosapic.c
>>> +++ b/drivers/parisc/iosapic.c
>>> @@ -677,7 +677,7 @@ static int iosapic_set_affinity_irq(struct irq_data *d,
>>> if (dest_cpu < 0)
>>> return -1;
>>> - cpumask_copy(irq_data_get_affinity_mask(d),
>>> cpumask_of(dest_cpu));
>>> + irq_data_update_affinity(d, cpumask_of(dest_cpu));
>>> vi->txn_addr = txn_affinity_addr(d->irq, dest_cpu);
>>> spin_lock_irqsave(&iosapic_lock, flags);
>>> diff --git a/drivers/sh/intc/chip.c b/drivers/sh/intc/chip.c
>>> index 358df7510186..828d81e02b37 100644
>>> --- a/drivers/sh/intc/chip.c
>>> +++ b/drivers/sh/intc/chip.c
>>> @@ -72,7 +72,7 @@ static int intc_set_affinity(struct irq_data *data,
>>> if (!cpumask_intersects(cpumask, cpu_online_mask))
>>> return -1;
>>> - cpumask_copy(irq_data_get_affinity_mask(data), cpumask);
>>> + irq_data_update_affinity(data, cpumask);
>>> return IRQ_SET_MASK_OK_NOCOPY;
>>> }
>>> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
>>> index 46d9295d9a6e..5e8321f43cbd 100644
>>> --- a/drivers/xen/events/events_base.c
>>> +++ b/drivers/xen/events/events_base.c
>>> @@ -528,9 +528,10 @@ static void bind_evtchn_to_cpu(evtchn_port_t evtchn, unsigned int cpu,
>>> BUG_ON(irq == -1);
>>> if (IS_ENABLED(CONFIG_SMP) && force_affinity) {
>>> - cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
>>> - cpumask_copy(irq_get_effective_affinity_mask(irq),
>>> - cpumask_of(cpu));
>>> + struct irq_data *data = irq_get_irq_data(irq);
>>> +
>>> + irq_data_update_affinity(data, cpumask_of(cpu));
>>> + irq_data_update_effective_affinity(data, cpumask_of(cpu));
>>> }
>>
>>
>> Nit: commit description says about reusing irq_data_update_affinity()
>> only, but here we also reuse irq_data_update_effective_affinity(), so
>> I would mention that in the description.
>>
>> Reviewed-by: Oleksandr Tyshchenko <[email protected]> # Xen bits
> b4 shouts because of your email address:
>
> NOTE: some trailers ignored due to from/email mismatches:
> ! Trailer: Reviewed-by: Oleksandr Tyshchenko <[email protected]> # Xen bits
> Msg From: Oleksandr <[email protected]>

sorry for the inconvenience


>
> I've used the tag anyway,


thank you


> but you may want to fix your setup in the
> future.

yes, will do


>
> Thanks,
>
> M.
>
--
Regards,

Oleksandr Tyshchenko