Re: [PATCH] cpumask_t

From: William Lee Irwin III (wli@holomorphy.com)
Date: Sun Jan 19 2003 - 17:18:52 EST


On Sun, Jan 19, 2003 at 01:35:24PM -0800, William Lee Irwin III wrote:
> -static void set_ioapic_affinity (unsigned int irq, unsigned long mask)
> +#if NR_CPUS <= 32
> +static void set_ioapic_affinity (unsigned int irq, cpumask_t mask)

This is just broken anyway. And fixing it isn't as obvious as just
using some mask -> APIC destination converter, as it's not possible
to form all masks in all APIC modes. Even better, we set up our RTE's
with physical DESTMOD, which not only finds these "masks" to be garbage,
but can only address things parked on the same APIC bus as the IO-APIC;
its implicit "any IO-APIC can interrupt any cpu" assumption is broken.

There are various very good reasons to keep it that way, too, for one,
interrupting cpus on remote nodes would be dog slow, and the software
interrupt number resolution techniques seem to require locality
restrictions, as I don't believe the box has anything resembling MSI
or whatever allows IA64 boxen to get away with 1:1 vector <-> irq maps.

I should probably find some way to make it play fair with xAPIC-based
stuff and find something better to #ifdef on (or some other way out of
actually being forced to deal with this) if I push this any further.

On Sun, Jan 19, 2003 at 01:35:24PM -0800, William Lee Irwin III wrote:
> @@ -1478,8 +1497,9 @@ static void __init setup_ioapic_ids_from
> - if (check_apicid_used(phys_id_present_map,
> - mp_ioapics[apic].mpc_apicid)) {
> +
> + /* MAJOR BRAINDAMAGE */
> + if (phys_id_present_map & (1UL << mp_ioapics[apic].mpc_apicid)) {
> printk(KERN_ERR "BIOS bug, IO-APIC#%d ID %d is already used!...\n",

It's currently broken on multi-APIC-bus boxen anyway, and there is a
replacement (not included here), at least for NUMA-Q. NFI about others;
multi-APIC bus configs predating xAPIC's etc. with 8-bit physid's are
probably rare outside of Sequent's boxen.

On Sun, Jan 19, 2003 at 01:35:24PM -0800, William Lee Irwin III wrote:
> - if (!(new_value & cpu_online_map))
> + cpus_and(tmp, new_value, cpu_online_map);
> + if (!any_online_cpu(tmp))
> return -EINVAL;

Woops; I better fix that.

Hmm, there are probably other stupidities about. It could probably
be cleaner/tested/etc. too. The IO-APIC and irq balancing stuff is
a disaster area with or without the patch; I'm not sure how much I
can really be expected to clean up down there. The TLB flushing stuff
wanting various tidbits of atomicity is a wee bit worrying as well.
I think I can get away with just shoving a barrier or two in there.
And the shift operations on bitmaps could probably made faster, but
AFAICT they aren't on any performance-critical paths.

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



This archive was generated by hypermail 2b29 : Thu Jan 23 2003 - 22:00:22 EST