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 --- Signed-off-by: Jarek Poplawski <[EMAIL PROTECTED]> --- diff -Nurp linux-2.6.20-rc4-/drivers/net/mv643xx_eth.c linux-2.6.20-rc4/drivers/net/mv643xx_eth.c --- linux-2.6.20-rc4-/drivers/net/mv643xx_eth.c 2006-12-18 08:57:52.000000000 +0100 +++ linux-2.6.20-rc4/drivers/net/mv643xx_eth.c 2007-01-11 08:55:34.000000000 +0100 @@ -312,8 +312,8 @@ int mv643xx_eth_free_tx_descs(struct net 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; @@ -348,8 +348,10 @@ int mv643xx_eth_free_tx_descs(struct net dev_kfree_skb_irq(skb);
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.
released = 1; + spin_lock_irqsave(&mp->lock, flags); } + spin_unlock_irqrestore(&mp->lock, flags); return released; }
Ugh, this is really unclean... Taking a lock "for nothing" like that has a perf cost. HTH T-Bone -- Thibaut VARENE http://www.parisc-linux.org/~varenet/ - 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