Re: [PATCH] Fix kernel lockup in RTL-8169 gigabit ethernet driver

[Francois Romieu]

Andrew Morton <akpm@xxxxxxxx> :
[...]
> The logic is faulty, or at least very odd.
> 
> 	tx_left = tp->cur_tx - dirty_tx;
> 
> 	while (tx_left > 0) {
> 		int entry = dirty_tx % NUM_TX_DESC;
> 
> 		if (!(le32_to_cpu(tp->TxDescArray[entry].status) & OWNbit)) {
> 			...
> 		}
> 	}
> 
> Why is that `if' test there at all?  If it ever returns false, the box

It checks that the network card has returned the buffer to the host computer.
The code will loop in the irq handler until every buffer submitted
for Tx is processed
- it is gross;
- it is not robust;
- I assume that's why the driver sucks cpu like hell on high Tx traffic;
- ... but it is not faulty. 

> locks up.  A BUG_ON(le32_to_cpu(tp->TxDescArray[entry].status) & OWNbit)
> might make more sense.

In Linus's current tree, I doubt it: assume the host has submitted a few
packets for Tx and the network card issue an interrupt for the first one 
whereas the second descriptor still belongs to the network card -> boom.

--
Ueimor
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/