RE: [RFC][2.6.12.3] IRQ compression/sharing patch

Protasevich, Natalie
2005-08-11T13:16:50Z

> > The only problem is here:
> >
> > + if (i < NR_IRQS) {
> > + gsi_2_irq[gsi] = i;
> > + printk(KERN_INFO "GSI %d sharing vector 0x%02X and IRQ
> > %d\n",
> > + gsi, vector, i);
> > + return i;
> > + }
> > +
> > + i = next_irq++;
> >
> > That means for any IRQ < NR_IRQS you allow it to be
> identity mapped,
> > with all the gaps, and only for ones exceeding 224 you'll assign
> > consecutive next_irqs++, whereas you can do it for all PCI
> IRQs above
> > 15. So, the alternative clause should probably come down to just:
> > irq = next_irq++;
> > gsi_2_irq[gsi] = irq; - which means just
> removing the
> > one above...
> > (although we better test that :)...I will definitely test vector
> > sharing when manage to get on max configuration partition here.
> >
> > Regards,
> > --Natalie
>
> Blast. You're right, because i is the results of scanning
> irq_vector (AKA IO_APIC_VECTOR()) for a previous use of the
> vector. If i < NR_IRQS then it has found a previous use and
> would substitute the other IRQ number rather than allocate a new one.
>
> Of course, one side effect of calling assign_irq_vector(gsi)
> is that the vector number is stored at position gsi in
> irq_vector. So, for IRQs 0 to NR_IRQS - 1, it will always
> find itself.
>
> OK, how about not fighting that side effect and only
> reassigning IRQs that are >= NR_IRQS? Maybe something like this:

I am wondering where sharing_vectors gets set, it was not in the patch
originally.

> @@ -579,4 +589,57 @@ static inline int irq_trigger(int idx)
> return MPBIOS_trigger(idx);
> }
>
> +/*
> + * gsi_irq_sharing -- Name overload! "irq" can be either a
> legacy IRQ
> + * in the range 0-15, a linux IRQ in the range 0-223, or a GSI number
> + * from ACPI, which can easily reach 800.
> + *
> + * Compact the sparse GSI space into a available IRQs and reuse
> + * vectors if possible.
> + */
> +int gsi_irq_sharing(int gsi)
> +{
> + int i, tries, vector;
> +
> + BUG_ON(gsi >= NR_IRQ_VECTORS);
> +
> + /*
> + * Don't share legacy IRQs. Their trigger modes are
> usually edge
> + * and PCI is level. Mixed modes are trouble. Only
> big boxes are
> + * likely to overflow IRQs or to share vectors.
> + */
> + if (likely(gsi < NR_IRQS && !sharing_vectors) ||
> platform_legacy_irq(gsi)) {
> + gsi_2_irq[gsi] = gsi;
> + return gsi;
> + }
> +
> + if (gsi_2_irq[gsi] != 0xFF)
> + return gsi_2_irq[gsi];
> + /*
> + * Ran out of vectors or IRQ >= NR_IRQS. Sharing vectors
> + * means sharing IRQs, so scan irq_vectors for previous use
> + * of vector and return that IRQ.
> + */
> + tries = NR_IRQS;
> + try_again:
> + vector = assign_irq_vector(gsi);
> +
> + /* Find the first IRQ using vector. */
> + for (i = 0; i < NR_IRQS; i++)
> + if (IO_APIC_VECTOR(i) == vector)
> + break;
> +
> + if (i >= NR_IRQS || platform_legacy_irq(i)) {
> + if (--tries >= 0) {
> + IO_APIC_VECTOR(gsi) = 0;
> + goto try_again;
> + }
> + panic("gsi_irq_sharing: didn't find an IRQ
> using vector 0x%02X for GSI %d", vector, gsi);
> + }
> + printk("GSI %d assigned vector 0x%02X and IRQ %d\n",
> gsi, vector, i);
> + gsi_2_irq[gsi] = i;
> + return i;
> +}
> +
> static int pin_2_irq(int idx, int apic, int pin) {
> int irq, i;
>
>
> --
> James Cleverdon
> IBM LTC (xSeries Linux Solutions)
> {jamesclv(Unix, preferred), cleverdj(Notes)} at us dot ibm dot comm
>