Messages in this thread |  | | | Date | Tue, 24 Mar 2009 13:36:44 -0700 | | From | Yinghai Lu <> | | Subject | Re: [RFC] Correct behaviour of irq affinity? |
| |
Rusty Russell wrote: > On Tuesday 24 March 2009 17:51:43 Yinghai Lu wrote: >> On Mon, Mar 23, 2009 at 10:49 PM, Rusty Russell <rusty@rustcorp.com.au> wrote: >>> The effect of setting desc->affinity (ie. from userspace via sysfs) has varied >>> over time. In 2.6.27, the 32-bit code anded the value with cpu_online_map, >>> and both 32 and 64-bit did that anding whenever a cpu was unplugged. >>> >>> 2.6.29 consolidated this into one routine (and fixed hotplug) but introduced >>> another variation: anding the affinity with cfg->domain. Is this right, or >>> should we just set it to what the user said? Or as now, indicate that we're >>> restricting it. >>> >>> If we should change it, here's what the patch looks like against x86 tip >>> (cpu_mask_to_apicid_and already takes cpu_online_mask into account): >>> >>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c >>> index 86827d8..30906cd 100644 >>> --- a/arch/x86/kernel/apic/io_apic.c >>> +++ b/arch/x86/kernel/apic/io_apic.c >>> @@ -592,10 +592,10 @@ set_desc_affinity(struct irq_desc *desc, const struct cpumask *mask) >>> if (assign_irq_vector(irq, cfg, mask)) >>> return BAD_APICID; >>> >>> - cpumask_and(desc->affinity, cfg->domain, mask); >>> + cpumask_copy(desc->affinity, mask); >>> set_extra_move_desc(desc, mask); >>> >>> - return apic->cpu_mask_to_apicid_and(desc->affinity, cpu_online_mask); >>> + return apic->cpu_mask_to_apicid_and(desc->affinity, cfg->domain); >>> } >>> >>> static void >>> >> cfg->domain for logical flat: will be ALL_CPUS >> for phys flat (aka bigsmp on 32bit) will be one cpu set mask. >> >> so desc->affinity: for logical will be not changed, but >> set_desc_affinity() return will be changed. ( not add with >> cpu_online_mask anymore) > > No, internally cpu_mask_to_apicid_and() does and with cpu_online_mask > already, eg in include/asm/bigsmp/apic.h: > > static inline unsigned int cpu_mask_to_apicid_and(const struct cpumask *cpumask, > const struct cpumask *andmask) > { > int cpu; > > /* > * We're using fixed IRQ delivery, can only return one phys APIC ID. > * May as well be the first. > */ > for_each_cpu_and(cpu, cpumask, andmask) > if (cpumask_test_cpu(cpu, cpu_online_mask)) > break; > if (cpu < nr_cpu_ids) > return cpu_to_logical_apicid(cpu); > > return BAD_APICID; > } > >> when mask is 0x0f >> for phys flat, desc->affinity will be changed to 0x0f from >> 0x01/0x02/0x04/08, return set_desc_affinity is not changed. >> so /proc/irq/xx/smp_affinity will be changed. and it does reflect that >> actually affinity. >> >> so this patch looks not right. > > Only change should be that smp_affinity will reflect actual affinity, not > affinity user set.
ok. how about static unsigned int flat_cpu_mask_to_apicid_and(const struct cpumask *cpumask, const struct cpumask *andmask) { unsigned long mask1 = cpumask_bits(cpumask)[0] & APIC_ALL_CPUS; unsigned long mask2 = cpumask_bits(andmask)[0] & APIC_ALL_CPUS; return mask1 & mask2; } change it use default_cpu_mask_to_apicid_and ?
YH
|  |