[vortex-bug] tbusy and tx_full

[Bogdan Costescu]

On Wed, 17 May 2000, Donald Becker wrote:

> > vortex_interrupt (which might not be OK because it's async WRT
> > start_xmit).
> That's the point of the tx_full flag in many of the drivers.  See
>     ftp://scyld.com/pub/network/pci-skeleton.c
> You create a race condition if you clear dev->tbusy when the transmit path
> thinks that it is locked against re-entry.

I agree, but why do you clear tbusy in vortex_interrupt? If tbusy is for
locking only, it should be set at entry and cleared at exit by the routine
which wants to be protected. You have there ...

>   vp->tx_full  &&  vp->cur_tx - dirty_tx < TX_QUEUE_LEN
> Means the transmit queue was full, but is now accepting more entries.

I understand your logic, but why do you want to do it so complicated, with
tbusy based on tx_full and indexes and then tx_full based on indexes ?
Wouldn't be simpler like this:

start_xmit				interrupt

lock
cur_tx++				dirty_tx++
unlock

And always when wanting to check for full ring, you get the actual values
of cur_tx and dirty_tx; this in only needed once in start_xmit (to deny
queueing when ring full) and once in tx_timeout. In interrupt you only
free the transmitted DPDs (if any) and update dirty_tx; you don't care
about the lock or cur_tx as you are sure that you only free queued DPDs.
Then, in start_xmit you only test for ring full and if negative, you add
the packet and update cur_tx; this makes sure that you don't have cur_tx -
dirty_tx > TX_RING_SIZE. This also removes some other tests for cur_tx
and dirty_tx. That's all and IMHO it does not have any races (if ++ is
atomic).
I might have missed something or maybe there is a reason for having the
driver the way it is; in this case, please correct me.

> You get much better system performance if you put some hysteresis in the
> check e.g.
>   vp->tx_full  &&  vp->cur_tx - dirty_tx < TX_QUEUE_LEN - 4
> This allows four packets at a time to be queued.

I assume that you're talking here about the interrupt. But I don't
understand your logic here: if you have a full ring and you only free one
DPD in the current interrupt, by having this hysteresis you don't clear
tx_full and tbusy. Then, you cannot queue another packet if start_xmit is
called; you have to wait to free 4 DPDs before being able to queue another
packet. This will give you ability to queue in hops; why do you say this
gives better system performance ?

> The vortex driver operates differently than many because it has evolved
> around the unusual 3Com design.  The 3Com Vortex design was one of the
> earliest PCI bus masters, and suffers from several quirks.

But that's no reason why the driver should remain cumbersome now that
it supports so many generations of cards...

Sincerely,

Bogdan Costescu

IWR - Interdisziplinaeres Zentrum fuer Wissenschaftliches Rechnen
Universitaet Heidelberg, INF 368, D-69120 Heidelberg, GERMANY
Telephone: +49 6221 54 8869, Telefax: +49 6221 54 8868
E-mail: Bogdan.Costescu@IWR.Uni-Heidelberg.De