Re: [RFC] Correct behaviour of irq affinity?

From: Rusty Russell
Date: Tue Mar 24 2009 - 08:52:40 EST


On Tuesday 24 March 2009 17:51:43 Yinghai Lu wrote:
> On Mon, Mar 23, 2009 at 10:49 PM, Rusty Russell <rusty@xxxxxxxxxxxxxxx> 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.

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/