2009-04-03 22:38:49

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 1/2] irq: correct CPUMASKS_OFFSTACK typo -v2


Impact: fix smp_affinity copying when moving irq_desc

CPUMASKS_OFFSTACK is not defined anywhere. it is a typo
and init_allocate_desc_masks called before it set affinity to all cpus...

split init_alloc_desc_masks() into all_desc_masks() and init_desc_masks()
so in the init_copy_desc_masks could use CPUMASK_OFFSTACK there.
aka copy path will not calling init_desc_masks anymore.

also could use that CPUMASK_OFFSTACK in alloc_desc_masks()

Signed-off-by: Yinghai Lu <[email protected]>

---
include/linux/irq.h | 27 ++++++++++++++++++---------
kernel/irq/handle.c | 9 ++++++---
kernel/irq/numa_migrate.c | 2 +-
3 files changed, 25 insertions(+), 13 deletions(-)

Index: linux-2.6/include/linux/irq.h
===================================================================
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -424,27 +424,25 @@ extern int set_irq_msi(unsigned int irq,

#ifdef CONFIG_SMP
/**
- * init_alloc_desc_masks - allocate cpumasks for irq_desc
+ * alloc_desc_masks - allocate cpumasks for irq_desc
* @desc: pointer to irq_desc struct
* @cpu: cpu which will be handling the cpumasks
* @boot: true if need bootmem
*
* Allocates affinity and pending_mask cpumask if required.
* Returns true if successful (or not required).
- * Side effect: affinity has all bits set, pending_mask has all bits clear.
*/
-static inline bool init_alloc_desc_masks(struct irq_desc *desc, int cpu,
+static inline bool alloc_desc_masks(struct irq_desc *desc, int cpu,
bool boot)
{
+#ifdef CONFIG_CPUMASK_OFFSTACK
int node;

if (boot) {
alloc_bootmem_cpumask_var(&desc->affinity);
- cpumask_setall(desc->affinity);

#ifdef CONFIG_GENERIC_PENDING_IRQ
alloc_bootmem_cpumask_var(&desc->pending_mask);
- cpumask_clear(desc->pending_mask);
#endif
return true;
}
@@ -453,18 +451,25 @@ static inline bool init_alloc_desc_masks

if (!alloc_cpumask_var_node(&desc->affinity, GFP_ATOMIC, node))
return false;
- cpumask_setall(desc->affinity);

#ifdef CONFIG_GENERIC_PENDING_IRQ
if (!alloc_cpumask_var_node(&desc->pending_mask, GFP_ATOMIC, node)) {
free_cpumask_var(desc->affinity);
return false;
}
- cpumask_clear(desc->pending_mask);
+#endif
#endif
return true;
}

+static inline void init_desc_masks(struct irq_desc *desc)
+{
+ cpumask_setall(desc->affinity);
+#ifdef CONFIG_GENERIC_PENDING_IRQ
+ cpumask_clear(desc->pending_mask);
+#endif
+}
+
/**
* init_copy_desc_masks - copy cpumasks for irq_desc
* @old_desc: pointer to old irq_desc struct
@@ -478,7 +483,7 @@ static inline bool init_alloc_desc_masks
static inline void init_copy_desc_masks(struct irq_desc *old_desc,
struct irq_desc *new_desc)
{
-#ifdef CONFIG_CPUMASKS_OFFSTACK
+#ifdef CONFIG_CPUMASK_OFFSTACK
cpumask_copy(new_desc->affinity, old_desc->affinity);

#ifdef CONFIG_GENERIC_PENDING_IRQ
@@ -499,12 +504,16 @@ static inline void free_desc_masks(struc

#else /* !CONFIG_SMP */

-static inline bool init_alloc_desc_masks(struct irq_desc *desc, int cpu,
+static inline bool alloc_desc_masks(struct irq_desc *desc, int cpu,
bool boot)
{
return true;
}

+static inline void init_desc_masks(struct irq_desc *desc)
+{
+}
+
static inline void init_copy_desc_masks(struct irq_desc *old_desc,
struct irq_desc *new_desc)
{
Index: linux-2.6/kernel/irq/handle.c
===================================================================
--- linux-2.6.orig/kernel/irq/handle.c
+++ linux-2.6/kernel/irq/handle.c
@@ -115,10 +115,11 @@ static void init_one_irq_desc(int irq, s
printk(KERN_ERR "can not alloc kstat_irqs\n");
BUG_ON(1);
}
- if (!init_alloc_desc_masks(desc, cpu, false)) {
+ if (!alloc_desc_masks(desc, cpu, false)) {
printk(KERN_ERR "can not alloc irq_desc cpumasks\n");
BUG_ON(1);
}
+ init_desc_masks(desc);
arch_init_chip_data(desc, cpu);
}

@@ -169,7 +170,8 @@ int __init early_irq_init(void)
desc[i].irq = i;
desc[i].kstat_irqs = kstat_irqs_legacy + i * nr_cpu_ids;
lockdep_set_class(&desc[i].lock, &irq_desc_lock_class);
- init_alloc_desc_masks(&desc[i], 0, true);
+ alloc_desc_masks(&desc[i], 0, true);
+ init_desc_masks(&desc[i]);
irq_desc_ptrs[i] = desc + i;
}

@@ -256,7 +258,8 @@ int __init early_irq_init(void)

for (i = 0; i < count; i++) {
desc[i].irq = i;
- init_alloc_desc_masks(&desc[i], 0, true);
+ alloc_desc_masks(&desc[i], 0, true);
+ init_desc_masks(&desc[i]);
desc[i].kstat_irqs = kstat_irqs_all[i];
}
return arch_early_irq_init();
Index: linux-2.6/kernel/irq/numa_migrate.c
===================================================================
--- linux-2.6.orig/kernel/irq/numa_migrate.c
+++ linux-2.6/kernel/irq/numa_migrate.c
@@ -37,7 +37,7 @@ static bool init_copy_one_irq_desc(int i
struct irq_desc *desc, int cpu)
{
memcpy(desc, old_desc, sizeof(struct irq_desc));
- if (!init_alloc_desc_masks(desc, cpu, false)) {
+ if (!alloc_desc_masks(desc, cpu, false)) {
printk(KERN_ERR "irq %d: can not get new irq_desc cpumask "
"for migration.\n", irq);
return false;


2009-04-03 22:40:09

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 2/2] irq: only update affinity in chip set_affinity()


Impact: keep affinity consistent

irq_set_affinity() and move_masked_irq() try to assign affinity
before calling chip set_affinity(). some archs are assigning again in set_affinity
again.

something like:
cpumask_cpy(desc->affinity, mask);
desc->chip->set_affinity(mask);

in the failing path, affinity should not be touched.

also set_extra_move_desc() ( called by set_affinity) will rely on the old
affinity to decide if need to move irq_desc to different node when logical
flat apic mode is used.

So try remove those assignment, and make some missed arch to assign affinity
in their set_affinity.

Signed-off-by: Yinghai Lu <[email protected]>

--
arch/alpha/kernel/sys_dp264.c | 6 ++++--
arch/alpha/kernel/sys_titan.c | 3 ++-
arch/arm/common/gic.c | 1 +
arch/cris/arch-v32/kernel/irq.c | 1 +
arch/ia64/kernel/iosapic.c | 3 +++
arch/ia64/sn/kernel/irq.c | 3 +++
arch/mips/cavium-octeon/octeon-irq.c | 6 ++++++
arch/mips/sibyte/bcm1480/irq.c | 2 ++
arch/mips/sibyte/sb1250/irq.c | 2 ++
arch/powerpc/platforms/pseries/xics.c | 5 +++++
arch/powerpc/sysdev/mpic.c | 2 ++
arch/sparc/kernel/irq_64.c | 7 +++++++
drivers/xen/events.c | 2 ++
kernel/irq/manage.c | 6 ++----
kernel/irq/migration.c | 8 +++-----
15 files changed, 45 insertions(+), 12 deletions(-)

Index: linux-2.6/arch/alpha/kernel/sys_dp264.c
===================================================================
--- linux-2.6.orig/arch/alpha/kernel/sys_dp264.c
+++ linux-2.6/arch/alpha/kernel/sys_dp264.c
@@ -178,7 +178,8 @@ cpu_set_irq_affinity(unsigned int irq, c

static void
dp264_set_affinity(unsigned int irq, const struct cpumask *affinity)
-{
+{
+ cpumask_copy(irq_desc[irq].affinity, affinity);
spin_lock(&dp264_irq_lock);
cpu_set_irq_affinity(irq, *affinity);
tsunami_update_irq_hw(cached_irq_mask);
@@ -187,7 +188,8 @@ dp264_set_affinity(unsigned int irq, con

static void
clipper_set_affinity(unsigned int irq, const struct cpumask *affinity)
-{
+{
+ cpumask_copy(irq_desc[irq].affinity, affinity);
spin_lock(&dp264_irq_lock);
cpu_set_irq_affinity(irq - 16, *affinity);
tsunami_update_irq_hw(cached_irq_mask);
Index: linux-2.6/arch/alpha/kernel/sys_titan.c
===================================================================
--- linux-2.6.orig/arch/alpha/kernel/sys_titan.c
+++ linux-2.6/arch/alpha/kernel/sys_titan.c
@@ -159,7 +159,8 @@ titan_cpu_set_irq_affinity(unsigned int

static void
titan_set_irq_affinity(unsigned int irq, const struct cpumask *affinity)
-{
+{
+ cpumask_copy(irq_desc[irq].affinity, affinity);
spin_lock(&titan_irq_lock);
titan_cpu_set_irq_affinity(irq - 16, *affinity);
titan_update_irq_hw(titan_cached_irq_mask);
Index: linux-2.6/arch/arm/common/gic.c
===================================================================
--- linux-2.6.orig/arch/arm/common/gic.c
+++ linux-2.6/arch/arm/common/gic.c
@@ -118,6 +118,7 @@ static void gic_set_cpu(unsigned int irq

spin_lock(&irq_controller_lock);
irq_desc[irq].cpu = cpu;
+ cpumask_copy(irq_desc[irq].affinity, mask_val);
val = readl(reg) & ~(0xff << shift);
val |= 1 << (cpu + shift);
writel(val, reg);
Index: linux-2.6/arch/cris/arch-v32/kernel/irq.c
===================================================================
--- linux-2.6.orig/arch/cris/arch-v32/kernel/irq.c
+++ linux-2.6/arch/cris/arch-v32/kernel/irq.c
@@ -330,6 +330,7 @@ void set_affinity_crisv32_irq(unsigned i
unsigned long flags;
spin_lock_irqsave(&irq_lock, flags);
irq_allocations[irq - FIRST_IRQ].mask = *dest;
+ cpumask_copy(irq_desc[j].affinity, dest);
spin_unlock_irqrestore(&irq_lock, flags);
}

Index: linux-2.6/arch/ia64/kernel/iosapic.c
===================================================================
--- linux-2.6.orig/arch/ia64/kernel/iosapic.c
+++ linux-2.6/arch/ia64/kernel/iosapic.c
@@ -338,6 +338,7 @@ iosapic_set_affinity(unsigned int irq, c
int redir = (irq & IA64_IRQ_REDIRECTED) ? 1 : 0;
struct iosapic_rte_info *rte;
struct iosapic *iosapic;
+ irq_desc_t *idesc = irq_desc + irq;

irq &= (~IA64_IRQ_REDIRECTED);

@@ -353,6 +354,8 @@ iosapic_set_affinity(unsigned int irq, c
if (!iosapic_intr_info[irq].count)
return; /* not an IOSAPIC interrupt */

+ cpumask_copy(desc->affinity, mask);
+
set_irq_affinity_info(irq, dest, redir);

/* dest contains both id and eid */
Index: linux-2.6/arch/ia64/sn/kernel/irq.c
===================================================================
--- linux-2.6.orig/arch/ia64/sn/kernel/irq.c
+++ linux-2.6/arch/ia64/sn/kernel/irq.c
@@ -232,6 +232,9 @@ static void sn_set_affinity_irq(unsigned
struct sn_irq_info *sn_irq_info, *sn_irq_info_safe;
nasid_t nasid;
int slice;
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ cpumask_copy(desc->affinity, mask);

nasid = cpuid_to_nasid(cpumask_first(mask));
slice = cpuid_to_slice(cpumask_first(mask));
Index: linux-2.6/arch/mips/cavium-octeon/octeon-irq.c
===================================================================
--- linux-2.6.orig/arch/mips/cavium-octeon/octeon-irq.c
+++ linux-2.6/arch/mips/cavium-octeon/octeon-irq.c
@@ -181,6 +181,9 @@ static void octeon_irq_ciu0_set_affinity
{
int cpu;
int bit = irq - OCTEON_IRQ_WORKQ0; /* Bit 0-63 of EN0 */
+ irq_desc_t *desc = irq_desc + irq;
+
+ cpumask_copy(desc->affinity, dest);

write_lock(&octeon_irq_ciu0_rwlock);
for_each_online_cpu(cpu) {
@@ -296,6 +299,9 @@ static void octeon_irq_ciu1_set_affinity
{
int cpu;
int bit = irq - OCTEON_IRQ_WDOG0; /* Bit 0-63 of EN1 */
+ irq_desc_t *desc = irq_desc + irq;
+
+ cpumask_copy(desc->affinity, dest);

write_lock(&octeon_irq_ciu1_rwlock);
for_each_online_cpu(cpu) {
Index: linux-2.6/arch/mips/sibyte/bcm1480/irq.c
===================================================================
--- linux-2.6.orig/arch/mips/sibyte/bcm1480/irq.c
+++ linux-2.6/arch/mips/sibyte/bcm1480/irq.c
@@ -123,6 +123,8 @@ static void bcm1480_set_affinity(unsigne
}
i = cpumask_first(mask);

+ cpumask_copy(desc->affinity, mask);
+
/* Convert logical CPU to physical CPU */
cpu = cpu_logical_map(i);

Index: linux-2.6/arch/mips/sibyte/sb1250/irq.c
===================================================================
--- linux-2.6.orig/arch/mips/sibyte/sb1250/irq.c
+++ linux-2.6/arch/mips/sibyte/sb1250/irq.c
@@ -117,6 +117,8 @@ static void sb1250_set_affinity(unsigned
return;
}

+ cpumask_copy(desc->affinity, mask);
+
/* Convert logical CPU to physical CPU */
cpu = cpu_logical_map(i);

Index: linux-2.6/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/pseries/xics.c
+++ linux-2.6/arch/powerpc/platforms/pseries/xics.c
@@ -339,6 +339,7 @@ static void xics_set_affinity(unsigned i
int status;
int xics_status[2];
int irq_server;
+ struct irq_desc *desc;

irq = (unsigned int)irq_map[virq].hwirq;
if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
@@ -374,6 +375,10 @@ static void xics_set_affinity(unsigned i
__func__, irq, status);
return;
}
+
+ desc = get_irq_desc(virq);
+
+ cpumask_copy(desc->affinity, cpumask);
}

static struct irq_chip xics_pic_direct = {
Index: linux-2.6/arch/powerpc/sysdev/mpic.c
===================================================================
--- linux-2.6.orig/arch/powerpc/sysdev/mpic.c
+++ linux-2.6/arch/powerpc/sysdev/mpic.c
@@ -811,7 +811,9 @@ void mpic_set_affinity(unsigned int irq,
{
struct mpic *mpic = mpic_from_irq(irq);
unsigned int src = mpic_irq_to_hw(irq);
+ struct irq_desc *desc = get_irq_desc(irq);

+ cpumask_copy(desc->affinity, cpumask);
if (mpic->flags & MPIC_SINGLE_DEST_CPU) {
int cpuid = irq_choose_cpu(irq);

Index: linux-2.6/arch/sparc/kernel/irq_64.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/irq_64.c
+++ linux-2.6/arch/sparc/kernel/irq_64.c
@@ -321,6 +321,9 @@ static void sun4u_irq_enable(unsigned in
static void sun4u_set_affinity(unsigned int virt_irq,
const struct cpumask *mask)
{
+ struct irq_desc *desc = irq_desc + virt_irq;
+
+ cpumask_copy(desc->affinity, mask);
sun4u_irq_enable(virt_irq);
}

@@ -382,8 +385,10 @@ static void sun4v_set_affinity(unsigned
{
unsigned int ino = virt_irq_table[virt_irq].dev_ino;
unsigned long cpuid = irq_choose_cpu(virt_irq);
+ struct irq_desc *desc = irq_desc + virt_irq;
int err;

+ cpumask_copy(desc->affinity, mask);
err = sun4v_intr_settarget(ino, cpuid);
if (err != HV_EOK)
printk(KERN_ERR "sun4v_intr_settarget(%x,%lu): "
@@ -449,8 +454,10 @@ static void sun4v_virt_set_affinity(unsi
const struct cpumask *mask)
{
unsigned long cpuid, dev_handle, dev_ino;
+ struct irq_desc *desc = irq_desc + virt_irq;
int err;

+ cpumask_copy(desc->affinity, mask);
cpuid = irq_choose_cpu(virt_irq);

dev_handle = virt_irq_table[virt_irq].dev_handle;
Index: linux-2.6/drivers/xen/events.c
===================================================================
--- linux-2.6.orig/drivers/xen/events.c
+++ linux-2.6/drivers/xen/events.c
@@ -713,6 +713,8 @@ static void rebind_irq_to_cpu(unsigned i
static void set_affinity_irq(unsigned irq, const struct cpumask *dest)
{
unsigned tcpu = cpumask_first(dest);
+
+ cpumask_copy(irq_to_desc(irq)->affinity, mask);
rebind_irq_to_cpu(irq, tcpu);
}

Index: linux-2.6/kernel/irq/manage.c
===================================================================
--- linux-2.6.orig/kernel/irq/manage.c
+++ linux-2.6/kernel/irq/manage.c
@@ -109,15 +109,13 @@ int irq_set_affinity(unsigned int irq, c
spin_lock_irqsave(&desc->lock, flags);

#ifdef CONFIG_GENERIC_PENDING_IRQ
- if (desc->status & IRQ_MOVE_PCNTXT || desc->status & IRQ_DISABLED) {
- cpumask_copy(desc->affinity, cpumask);
+ if (desc->status & IRQ_MOVE_PCNTXT || desc->status & IRQ_DISABLED)
desc->chip->set_affinity(irq, cpumask);
- } else {
+ else {
desc->status |= IRQ_MOVE_PENDING;
cpumask_copy(desc->pending_mask, cpumask);
}
#else
- cpumask_copy(desc->affinity, cpumask);
desc->chip->set_affinity(irq, cpumask);
#endif
irq_set_thread_affinity(desc, cpumask);
Index: linux-2.6/kernel/irq/migration.c
===================================================================
--- linux-2.6.orig/kernel/irq/migration.c
+++ linux-2.6/kernel/irq/migration.c
@@ -39,11 +39,9 @@ void move_masked_irq(int irq)
* masking the irqs.
*/
if (likely(cpumask_any_and(desc->pending_mask, cpu_online_mask)
- < nr_cpu_ids)) {
- cpumask_and(desc->affinity,
- desc->pending_mask, cpu_online_mask);
- desc->chip->set_affinity(irq, desc->affinity);
- }
+ < nr_cpu_ids))
+ desc->chip->set_affinity(irq, desc->pending_mask);
+
cpumask_clear(desc->pending_mask);
}

2009-04-08 12:55:45

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] irq: only update affinity in chip set_affinity()


* Yinghai Lu <[email protected]> wrote:

> Impact: keep affinity consistent
>
> irq_set_affinity() and move_masked_irq() try to assign affinity
> before calling chip set_affinity(). some archs are assigning again
> in set_affinity again.
>
> something like:
> cpumask_cpy(desc->affinity, mask);
> desc->chip->set_affinity(mask);
>
> in the failing path, affinity should not be touched.
>
> also set_extra_move_desc() ( called by set_affinity) will rely on
> the old affinity to decide if need to move irq_desc to different
> node when logical flat apic mode is used.
>
> So try remove those assignment, and make some missed arch to
> assign affinity in their set_affinity.
>
> Signed-off-by: Yinghai Lu <[email protected]>
>
> --
> arch/alpha/kernel/sys_dp264.c | 6 ++++--
> arch/alpha/kernel/sys_titan.c | 3 ++-
> arch/arm/common/gic.c | 1 +
> arch/cris/arch-v32/kernel/irq.c | 1 +
> arch/ia64/kernel/iosapic.c | 3 +++
> arch/ia64/sn/kernel/irq.c | 3 +++
> arch/mips/cavium-octeon/octeon-irq.c | 6 ++++++
> arch/mips/sibyte/bcm1480/irq.c | 2 ++
> arch/mips/sibyte/sb1250/irq.c | 2 ++
> arch/powerpc/platforms/pseries/xics.c | 5 +++++
> arch/powerpc/sysdev/mpic.c | 2 ++
> arch/sparc/kernel/irq_64.c | 7 +++++++
> drivers/xen/events.c | 2 ++
> kernel/irq/manage.c | 6 ++----
> kernel/irq/migration.c | 8 +++-----
> 15 files changed, 45 insertions(+), 12 deletions(-)

Hm, this spreads a lot of instances of identical lines:

cpumask_copy(irq_desc[irq].affinity, mask_val);

all around architectures. How is that an improvement?

Ingo

2009-04-08 15:54:41

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 2/2] irq: only update affinity in chip set_affinity()

On Wed, Apr 8, 2009 at 5:54 AM, Ingo Molnar <[email protected]> wrote:
>
> * Yinghai Lu <[email protected]> wrote:
>
>> Impact: keep affinity consistent
>>
>> irq_set_affinity() and move_masked_irq() try to assign affinity
>> before calling chip set_affinity(). some archs are assigning again
>> in set_affinity again.
>>
>> something like:
>> cpumask_cpy(desc->affinity, mask);
>> desc->chip->set_affinity(mask);
>>
>> in the failing path, affinity should not be touched.
>>
>> also set_extra_move_desc() ( called by set_affinity) will rely on
>> the old affinity to decide if need to move irq_desc to different
>> node when logical flat apic mode is used.
>>
>> So try remove those assignment, and make some missed arch to
>> assign affinity in their set_affinity.
>>
>> Signed-off-by: Yinghai Lu <[email protected]>
>>
>> --
>> ?arch/alpha/kernel/sys_dp264.c ? ? ? ? | ? ?6 ++++--
>> ?arch/alpha/kernel/sys_titan.c ? ? ? ? | ? ?3 ++-
>> ?arch/arm/common/gic.c ? ? ? ? ? ? ? ? | ? ?1 +
>> ?arch/cris/arch-v32/kernel/irq.c ? ? ? | ? ?1 +
>> ?arch/ia64/kernel/iosapic.c ? ? ? ? ? ?| ? ?3 +++
>> ?arch/ia64/sn/kernel/irq.c ? ? ? ? ? ? | ? ?3 +++
>> ?arch/mips/cavium-octeon/octeon-irq.c ?| ? ?6 ++++++
>> ?arch/mips/sibyte/bcm1480/irq.c ? ? ? ?| ? ?2 ++
>> ?arch/mips/sibyte/sb1250/irq.c ? ? ? ? | ? ?2 ++
>> ?arch/powerpc/platforms/pseries/xics.c | ? ?5 +++++
>> ?arch/powerpc/sysdev/mpic.c ? ? ? ? ? ?| ? ?2 ++
>> ?arch/sparc/kernel/irq_64.c ? ? ? ? ? ?| ? ?7 +++++++
>> ?drivers/xen/events.c ? ? ? ? ? ? ? ? ?| ? ?2 ++
>> ?kernel/irq/manage.c ? ? ? ? ? ? ? ? ? | ? ?6 ++----
>> ?kernel/irq/migration.c ? ? ? ? ? ? ? ?| ? ?8 +++-----
>> ?15 files changed, 45 insertions(+), 12 deletions(-)
>
> Hm, this spreads a lot of instances of identical lines:
>
> ? cpumask_copy(irq_desc[irq].affinity, mask_val);
>
> all around architectures. How is that an improvement?
>

in failing path in set_affinity, for example it can not get vector in
specified cpu,
then affinity should not be changed.

YH

2009-04-08 15:59:37

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] irq: only update affinity in chip set_affinity()


* Yinghai Lu <[email protected]> wrote:

> On Wed, Apr 8, 2009 at 5:54 AM, Ingo Molnar <[email protected]> wrote:
> >
> > * Yinghai Lu <[email protected]> wrote:
> >
> >> Impact: keep affinity consistent
> >>
> >> irq_set_affinity() and move_masked_irq() try to assign affinity
> >> before calling chip set_affinity(). some archs are assigning again
> >> in set_affinity again.
> >>
> >> something like:
> >> cpumask_cpy(desc->affinity, mask);
> >> desc->chip->set_affinity(mask);
> >>
> >> in the failing path, affinity should not be touched.
> >>
> >> also set_extra_move_desc() ( called by set_affinity) will rely on
> >> the old affinity to decide if need to move irq_desc to different
> >> node when logical flat apic mode is used.
> >>
> >> So try remove those assignment, and make some missed arch to
> >> assign affinity in their set_affinity.
> >>
> >> Signed-off-by: Yinghai Lu <[email protected]>
> >>
> >> --
> >> ?arch/alpha/kernel/sys_dp264.c ? ? ? ? | ? ?6 ++++--
> >> ?arch/alpha/kernel/sys_titan.c ? ? ? ? | ? ?3 ++-
> >> ?arch/arm/common/gic.c ? ? ? ? ? ? ? ? | ? ?1 +
> >> ?arch/cris/arch-v32/kernel/irq.c ? ? ? | ? ?1 +
> >> ?arch/ia64/kernel/iosapic.c ? ? ? ? ? ?| ? ?3 +++
> >> ?arch/ia64/sn/kernel/irq.c ? ? ? ? ? ? | ? ?3 +++
> >> ?arch/mips/cavium-octeon/octeon-irq.c ?| ? ?6 ++++++
> >> ?arch/mips/sibyte/bcm1480/irq.c ? ? ? ?| ? ?2 ++
> >> ?arch/mips/sibyte/sb1250/irq.c ? ? ? ? | ? ?2 ++
> >> ?arch/powerpc/platforms/pseries/xics.c | ? ?5 +++++
> >> ?arch/powerpc/sysdev/mpic.c ? ? ? ? ? ?| ? ?2 ++
> >> ?arch/sparc/kernel/irq_64.c ? ? ? ? ? ?| ? ?7 +++++++
> >> ?drivers/xen/events.c ? ? ? ? ? ? ? ? ?| ? ?2 ++
> >> ?kernel/irq/manage.c ? ? ? ? ? ? ? ? ? | ? ?6 ++----
> >> ?kernel/irq/migration.c ? ? ? ? ? ? ? ?| ? ?8 +++-----
> >> ?15 files changed, 45 insertions(+), 12 deletions(-)
> >
> > Hm, this spreads a lot of instances of identical lines:
> >
> > ? cpumask_copy(irq_desc[irq].affinity, mask_val);
> >
> > all around architectures. How is that an improvement?
> >
>
> in failing path in set_affinity, for example it can not get vector
> in specified cpu, then affinity should not be changed.

isnt the right solution then to propagate the failure code back to
the generic code?

Preferably via a new callback, and the patches only touching the
core code plus maybe x86, so that other architectures can be
converted/fixed more gradually.

Ingo