netdev
[Top] [All Lists]

Re: Queue and SMP locking discussion (was Re: 3c59x.c)

To: jamal <hadi@xxxxxxxxxx>
Subject: Re: Queue and SMP locking discussion (was Re: 3c59x.c)
From: Andrew Morton <andrewm@xxxxxxxxxx>
Date: Fri, 31 Mar 2000 02:27:50 +0000
Cc: Donald Becker <becker@xxxxxxxxx>, netdev@xxxxxxxxxxx, kuznet@xxxxxxxxxxxxx
References: <Pine.LNX.4.10.10003301359200.2499-100000@xxxxxxxxxxxxx> <Pine.GSO.4.20.0003301529070.14621-100000@xxxxxxxxxxxxxxxx>
Sender: owner-netdev@xxxxxxxxxxx
jamal wrote:
> 
> ...
> In my analysis i noted that the "tx timeout" problems under moderate
> network loads was _mostly_ because the tx thread was being starved.
> (i was blasting a lot of 64 byte packets at the tulip and eepro and
> trying to see where they start dying).
> One of the main reasons was that the rx thread interupt was consistently
> pre-empting the tx.
> Is the total paralelization you are proposing providing any protection
> against such issues? The suggested locks do fix this problem.

I believe that this tx starvation is due to the decision to schedule the
tx in the device ISR, for BH handling, rather than to actually dequeue
and send packets within the Tx ISR.  I can see why the bh scheduling is
simpler...

I like the loop-until-max_interrupt_work-exceeded architecture.  It's
_very_ efficient compared with interrupt per packet, and it kicks in
when the system is under stress.  But it's not being leveraged for
transmits.

BTW::::

The 3c59x driver's ISR does this:

while (stuff_to_do && (count++ < max_interrupt_work))
{
        if (the device has room for a tx packet)
                netif_wake_queue()
}

It appears to me that netif_wake_queue can be called multiple times
within this loop, at considerable expense, when the system is under Rx
stress.

Wouldn't it be better to have a local flag in the ISR which prevents
this?


bool done_wake = false;
while (count++ < max_interrupt_work)
{
        if (!done_wake && the device has room for a tx packet)
        {
                done_wake = true;
                netif_wake_queue();
        }
}



-- 
-akpm-

<Prev in Thread] Current Thread [Next in Thread>