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

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

[Yinghai Lu]

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 <yinghai@kernel.org>

---
 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;

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

[Yinghai Lu]

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 <yinghai@kernel.org>

--
 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);
 }
 

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

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> 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 <yinghai@kernel.org>
> 
> --
>  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

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

[Yinghai Lu]

On Wed, Apr 8, 2009 at 5:54 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Yinghai Lu <yinghai@kernel.org> 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 <yinghai@kernel.org>
>>
>> --
>>  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

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

[Ingo Molnar]

* Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> On Wed, Apr 8, 2009 at 5:54 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Yinghai Lu <yinghai@kernel.org> 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 <yinghai@kernel.org>
> >>
> >> --
> >>  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