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

James Cleverdon
2005-08-11T03:14:18Z

On Wednesday 10 August 2005 05:21 pm, Protasevich, Natalie wrote:
> > > int gsi_irq_sharing(int gsi)
> > > {
> > > int i, irq, vector;
> > >
> > > BUG_ON(gsi >= NR_IRQ_VECTORS);
> > >
> > > if (platform_legacy_irq(gsi)) {
> > > gsi_2_irq[gsi] = gsi;
> > > return gsi;
> > > }
> > >
> > > if (gsi_2_irq[gsi] != 0xFF)
> > > return (int)gsi_2_irq[gsi];
> > >
> > > vector = assign_irq_vector(gsi); // this part
> > > here==========
> >
> > I thought I had this case covered earlier, given that in both i386
> > and x86_64:
> >
> > #define platform_legacy_irq(irq) ((irq) < 16)
>
> Yes, you are absolutely correct, I don't need the (gsi<16) part, this
> takes care of PCI IRQs that happened to be <16.
>
> > In the deleted vector sharing code, I also check
> > platform_legacy_irq, to avoid inadvertently sharing vectors
> > already assigned to legacy IRQs.
> >
> > Am I missing your point here?
> >
> > > if (gsi < 16) {
> > > irq = gsi;
> > > gsi_2_irq[gsi] = irq;
> > > } else {
> > > irq = next_irq++;
> > > gsi_2_irq[gsi] = irq;
> > > }
> > > //====================
> >
> > I can't explain the gaps in the numbers with the original
> > version. I'll give your variant a try.
>
> 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:

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