Herbert Xu wrote: > Yes it has the problem in two places. First of all the first > netif_stop_queue in tg3_start_xmit doesn't take the tx_lock at > all so it's obviously vulnerable.
Oh, that first one near the top of tg3_start_xmit()? That condition is not supposed to happen and is a bug condition. But we can certainly add a spinlock to make it more correct. > > The last netif_stop_queue is susceptible to a more subtle problem: > > CPU0 CPU1 > tg3_start_xmit > TX_BUFF_AVAIL <= threshold > tx_lock > tg3_tx > !netif_queue_stopped > netif_stop_queue > BUFF_AVAIL <= threshold > tx_cons = sw_idx > tx_unlock > > Because netif_queue_stopped is *not* a memory barrier, it can float > above the assignment to tx_cons. So even though netif_stop_queue *is* > a memory barrier on x86, the second BUFF_AVAIL test can still fail to > see the updated index. > > The fix here is obviously to add a memory barrier in tg3_tx. You are absolutely right and I missed it. We should add a wmb() after the tx_cons update in tg3_tx(). Thanks. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
