FIX: 0.99L and timeouts

[Bogdan Costescu]

On Fri, 21 Apr 2000, Andrew Morton wrote:

> > IMHO, the problem is that if other CPU takes the interrupt, it computes
> > entry and prev_entry based on vp->cur_tx which are _before_ the spinlock.
> 
> Why is this a problem?  The ISR doesn't change the value of cur_tx. 
> cur_tx is only altered within the spinlock, where the current CPU has
> complete control.

This discussion is still for the case before moving DownUnstall near 
spin_unlock. vp->cur_tx++ is after the DownUnstall, so the card can
generate the interrupt which can be processed by another CPU which may use
the value of vp->cur_tx before being incremented and so it points to
a soon-to-be-freed buffer. Then the spinlock is exited by the first CPU
and the second CPU enters it with wrong pointers.

> Oh dear.  Perhaps set 'debug=1'?
> 
> Also suggest you put a big printk() in vortex_rx() - it should never be
> called, and we're _technically_ still in voliation of the specs:
> 
> Page 122, bit [4]:
> 
> "This bit is automatically acknowledged by the upload
> engine as it uploads packets. Drivers should disable this
> interrupt and mask this bit when reading IntStatus."
> 
> We don't mask it - we still test it, although we are disabling it in the
> interrupt enable reg.
> 
> In fact, you could just remove the lines:
> 
>         if (status & RxComplete)
>             vortex_rx(dev);
>  
> from vortex_interrupt.
> 
> And finally, in vortex_interrupt:
> 
>         if (status & TxAvailable) {
>             if (vortex_debug > 5)
>                 printk(KERN_DEBUG " TX room bit was handled.\n");
>             /* There's room in the FIFO for a full-sized packet. */
>             outw(AckIntr | TxAvailable, ioaddr + EL3_CMD);
>             clear_bit(0, (void*)&dev->tbusy);
>             mark_bh(NET_BH);
>         }
> 
> Put a big printk in here.  The test should never be true.  I actually
> split the ISR into two for the 2.3 driver for these reasons, and cache
> footprint.  This eliminates the option of having full_bus_master_rx and
> not full_bus_master_tx (and vice versa), but this doesn't happen.

Thank you for the suggestions. Splitting the ISR was one thing that I
planned to do for (possible) performance improvement.
I forgot to mention in my previous message something that might be of some
importance: the flooding ping and some tests yesterday were done with your
driver (and DownUnstall moved) with TX_ & RX_RING_SIZE = 64. The locks
that I'm experiencing now are with TX_ & RX_RING_SIZE = 2 which might
trigger some marginal case...

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

-------------------------------------------------------------------
To unsubscribe send a message body containing "unsubscribe"
to linux-vortex-bug-request@beowulf.org