On 1/21/07, Thibaut VARENE <[EMAIL PROTECTED]> wrote:
On 1/11/07, Jarek Poplawski <[EMAIL PROTECTED]> wrote:
>
> PS: alas I didn't even check compiling - I had no time to
> find all compile dependencies of this driver
> ---
Hmm, I think this is guaranteed not to work. In between those lines
the lock is released, while data in the mp structure is still being
accessed. It seems that this bit of code is indeed not race-safe
though, I'm gonna try to figure something.

This was indeed the right spot. The attached raw hack seems to fix the
bug (I couldn't crash the box so far).  I haven't checked that the
same "situation" happens elsewhere in the code, I leave that as an
exercise for the maintainers (or until I experience another kind of
crash :)

The patch is a bit ugly (printk with irq disabled will not show, etc)
but at least it does work. I'm sure somebody will figure something

HTH

T-Bone

--
Thibaut VARENE
http://www.parisc-linux.org/~varenet/
--- linux-2.6.19.orig/drivers/net/mv643xx_eth.c	2007-01-21 13:56:04.450689123 +0100
+++ linux-2.6.19/drivers/net/mv643xx_eth.c	2007-01-21 13:39:58.228404763 +0100
@@ -312,8 +312,8 @@
 	int count;
 	int released = 0;
 
+	spin_lock_irqsave(&mp->lock, flags);
 	while (mp->tx_desc_count > 0) {
-		spin_lock_irqsave(&mp->lock, flags);
 		tx_index = mp->tx_used_desc_q;
 		desc = &mp->p_tx_desc_area[tx_index];
 		cmd_sts = desc->cmd_sts;
@@ -332,8 +332,6 @@
 		if (skb)
 			mp->tx_skb[tx_index] = NULL;
 
-		spin_unlock_irqrestore(&mp->lock, flags);
-
 		if (cmd_sts & ETH_ERROR_SUMMARY) {
 			printk("%s: Error in TX\n", dev->name);
 			mp->stats.tx_errors++;
@@ -349,6 +347,7 @@
 
 		released = 1;
 	}
+	spin_unlock_irqrestore(&mp->lock, flags);
 
 	return released;
 }

Reply via email to