Hi Claudi,

Claudi Jr wrote:
I have tested the original FEC driver with IRQF_DISABLED and it works fine.

***** NOTE *****
When I refer to "fec tests" I mean to perform several attacks to the board
by downloading and uploading files thru BOA web server, simultaneously.
****************

However, I'd like to propose to replace the while() statement in fec_enet_interrupt() for a simple if() because, since we are disabling INT's, a highly loaded network would make the ISR to keep running for a possibly not desired long period of time (think in, not real time, but maybe in audio applications?? - don't know). Actually I

Do you really mean "applications" in the user/kernel mode sense?

If the network engine is still busy (that is it is flagging
another interrupt), then exiting the the fec interrupt routine
will result in immediate raising of another CPU interrupt,
and re-entry back into the fec interrupt handler. So a while loop
is merely saving the interrupt exit/entry overhead here.

A higher hardware priority interrupt event may get in first
(though strictly speaking we don't rely on any specific hardware
priority encoding in uClinux).

If we were not using IRQF_DISABLED then another interrupt could
interrupt on top of the fec interrupt. This, I think, is possible,
cleaning up and separating the fec interrupt events into separate
handlers. But if the fec engine is still raising interrupt events
then we will keep entering its interrupt handler if we don't have
the while loop.


don't know whether setting
IRQF_DISABLED and a single if() statement would also affect the overall performance. Disabling nested
interrupts may lead to a poor system performance for specific applications.
My last FEC driver is a little bit different from the first I dumped in the mailing list: as you said in the last email, here, besides of acknowledging the interrupts after attending them, here
I attend only to 1 interrupt at a time.

Since fec_enet_interrupt is quite short, I show the whole modified function below:

static irqreturn_t
fec_enet_interrupt(int irq, void * dev_id)
{
    struct    net_device *dev = dev_id;
    volatile fec_t    *fecp;
    uint    int_events;
    int handled = 0;
fecp = (volatile fec_t*)dev->base_addr;

    /* Get the interrupt events that caused us to be here.
    */
    /* Not a while() statement, but an if() one */
if ((int_events = (fecp->fec_ievent & (FEC_ENET_RXF|FEC_ENET_TXF|FEC_ENET_MII))) != 0) { /* Handle receive event in its own function.
        */
        if (int_events & FEC_ENET_RXF) {
            handled = 1;
            fec_enet_rx(dev);
            fecp->fec_ievent = FEC_ENET_RXF;
            return IRQ_RETVAL(handled);            /* Return now */
        }

        /* Transmit OK, or non-fatal error. Update the buffer
           descriptors. FEC handles all errors, we just discover
           them as part of the transmit process.
        */
        if (int_events & FEC_ENET_TXF) {
            handled = 1;
            fec_enet_tx(dev);
            fecp->fec_ievent = FEC_ENET_TXF;
            return IRQ_RETVAL(handled);            /* Return now */
        }

        if (int_events & FEC_ENET_MII) {
            handled = 1;
            fec_enet_mii(dev);
            fecp->fec_ievent = FEC_ENET_MII;
            return IRQ_RETVAL(handled);            /* Return now */
        }
}
return IRQ_RETVAL(handled);
}


This version of fec_enet_interrupt() works fine to me, but I must say that performance in network
communications is notably worsen.

On a loaded network you are going to be doing a lot of interrupt
entry/exit with this version of the handler. The while loop will save
a lot of extra cycles in this loaded scenario.


Thus, I would like to know if you agree to make a patch that requests ints with IRQF_DISABLED but attending the interrupt with an if() statement instead of a while(). It's just only to balance the overall system performance. The function would stay as follows (again, IRQF_DISABLED should be
set when calling request_irq):


static irqreturn_t
fec_enet_interrupt(int irq, void * dev_id)
{
    struct    net_device *dev = dev_id;
    volatile fec_t    *fecp;
    uint    int_events;
    int handled = 0;

    fecp = (volatile fec_t*)dev->base_addr;

    /* Get the interrupt events that caused us to be here.
    */
    if ((int_events = fecp->fec_ievent) != 0) {
        fecp->fec_ievent = int_events;

        /* Handle receive event in its own function.
        */
        if (int_events & FEC_ENET_RXF) {
            handled = 1;
            fec_enet_rx(dev);
        }

        /* Transmit OK, or non-fatal error. Update the buffer
           descriptors. FEC handles all errors, we just discover
           them as part of the transmit process.
        */
        if (int_events & FEC_ENET_TXF) {
            handled = 1;
            fec_enet_tx(dev);
        }

        if (int_events & FEC_ENET_MII) {
            handled = 1;
            fec_enet_mii(dev);
        }

    }
    return IRQ_RETVAL(handled);
}

I have also tested this code and it performs the same as the version with the while() statement. However this way may be better for specific applications that require
quick interrupt processing.

What performance tests did you carry out?
I'd like to see real numbers on total throughput under
heavy load to see see how much better/worse this is.

Regards
Greg


------------------------------------------------------------------------
Greg Ungerer  --  Chief Software Dude       EMAIL:     [EMAIL PROTECTED]
Secure Computing Corporation                PHONE:       +61 7 3435 2888
825 Stanley St,                             FAX:         +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia         WEB: http://www.SnapGear.com
_______________________________________________
uClinux-dev mailing list
[email protected]
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by [email protected]
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev

Reply via email to