Re: [PATCH] Re: kernel BUG in eth_alloc_tx_desc_index at drivers/net/mv643xx_eth.c:1069!

["Thibaut VARENE" <T-Bone@parisc-linux.org>]

[-- Attachment #1: Type: text/plain, Size: 980 bytes --]

On 1/21/07, Thibaut VARENE <T-Bone@parisc-linux.org> wrote:
> On 1/11/07, Jarek Poplawski <jarkao2@o2.pl> 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/

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix_mv643xx_race.patch --]
[-- Type: text/x-patch; name="fix_mv643xx_race.patch", Size: 768 bytes --]

--- 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;
 }