Subject: Re: poor mc* performance revisited
To: None <port-macppc@netbsd.org>
From: Tim Kelly <hockey@dialectronics.com>
List: port-macppc
Date: 11/30/2004 08:45:04
At 11:14 PM -0500 11/29/04, Tim Kelly wrote:
> In the case of
>mace, on my 7300 it has interrupts at 14, 2, and 3. However, as virq 1 goes
>to the second CPU, and virq 2 goes to the extra ethernet card I have
>installed, the irq 2 from mace does not get properly mapped in
>intr_establish.

Well, what do you know - establishing an interrupt for irq 2 is commented
out in if_mc.c. According to cvs, it's been like that since it was
committed the first time.

Here are two patches, one to uncomment irq 2, and one to comment out the
overloading of virqs. As far as I can tell, mapirq is only called within
the extintr.c file, so anything that this breaks should be revealing. If
the second patch doesn't apply, enter it manually. I have a bunch of other
stuff for MP that isn't relevant in this context, so I just cut and pasted
the particular piece of the patch. It's the first change to the file, so
I'm hoping that the line offsets will be ok.

I can't see where overloading the virq is a "Good Thing," as used here. I
suspect that the intention was to reuse the function to also do unmapping.
However, as can be seen at the bottom of the second patch,

        intrsources[v].is_hwirq = irq;
        virq[irq] = v;

some hardware information is associated with the virq intrsources. If the
virq was already assigned when a hardware irq matching it comes through,
the hardware irq never gets associated with a virq. Later, during
do_pending_int,

                irq = 31 - cntlzw(hwpend);
                is = &intrsources[irq];
                if (!have_openpic)
                        gc_enable_irq(is->is_hwirq);

gc_enable_irq gets the wrong hwirq if the real hardware irq is involved. I
haven't worked out the implications for MP yet, although it does appear
that the kernel lock involves access to a device, but it seems to me that
if the hwirq is associated with esp (external scsi) but the device is mace
(ethernet), there's plenty of room for timeouts, resends, and general poor
performance.

tim

Index: if_mc.c
===================================================================
RCS file: /cvsroot/src/sys/arch/macppc/dev/if_mc.c,v
retrieving revision 1.9
diff -d -u -r1.9 if_mc.c
--- if_mc.c     15 Jul 2003 02:43:29 -0000      1.9
+++ if_mc.c     30 Nov 2004 13:21:59 -0000
@@ -174,7 +174,7 @@
        dbdma_reset(sc->sc_txdma);

        /* install interrupt handlers */
-       /*intr_establish(ca->ca_intr[1], IST_LEVEL, IPL_NET, mc_dmaintr, sc);*/
+       intr_establish(ca->ca_intr[1], IST_LEVEL, IPL_NET, mc_dmaintr, sc);
        intr_establish(ca->ca_intr[2], IST_LEVEL, IPL_NET, mc_dmaintr, sc);
        intr_establish(ca->ca_intr[0], IST_LEVEL, IPL_NET, mcintr, sc);


Index: extintr.c
===================================================================
RCS file: /cvsroot/src/sys/arch/macppc/macppc/extintr.c,v
retrieving revision 1.44
diff -d -u -r1.44 extintr.c
--- extintr.c   20 Jun 2004 20:50:13 -0000      1.44
+++ extintr.c   30 Nov 2004 13:28:42 -0000
@@ -159,14 +159,18 @@

        if (irq < 0 || irq >= ICU_LEN)
                panic("invalid irq %d", irq);
+#if 0
        if (virq[irq])
                return virq[irq];
+#endif

        virq_max++;
        v = virq_max;
        if (v > HWIRQ_MAX)
                panic("virq overflow");

+       printf("mapping irq %x to virq %x\n", irq, v);
+
        intrsources[v].is_hwirq = irq;
        virq[irq] = v;