Re: [PATCH 2.6.18 1/3] tg3: Remove unnecessary tx_lock

2006-06-17 Thread David Miller 
From: Michael Chan [EMAIL PROTECTED]
Date: Mon, 05 Jun 2006 12:47:23 -0700

 Remove tx_lock where it is unnecessary. tg3 runs lockless and so it
 requires interrupts to be disabled and sync'ed, netif_queue and NAPI
 poll to be stopped before the device can be reconfigured. After
 stopping everything, it is no longer necessary to get the tx_lock.
 
 Signed-off-by: Michael Chan [EMAIL PROTECTED]

Applied, 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

Re: [PATCH 2.6.18 1/3] tg3: Remove unnecessary tx_lock

2006-06-06 Thread Herbert Xu 
On Mon, Jun 05, 2006 at 10:10:56PM -0700, Michael Chan wrote:

 In places where we don't call tg3_netif_stop() before tg3_full_lock(),
 we are typically doing one of the following:
 
 - changing tg3_flags or tg3_flags2
 - reprogramming MAC address
 - reprogramming interrupt coalescing values
 - reprogramming the PHY
 - setting the rx mode

OK, thanks for checking.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
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

[PATCH 2.6.18 1/3] tg3: Remove unnecessary tx_lock

2006-06-05 Thread Michael Chan 
Remove tx_lock where it is unnecessary. tg3 runs lockless and so it
requires interrupts to be disabled and sync'ed, netif_queue and NAPI
poll to be stopped before the device can be reconfigured. After
stopping everything, it is no longer necessary to get the tx_lock.

Signed-off-by: Michael Chan [EMAIL PROTECTED]


diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index f2382aa..4cda2af 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -2984,9 +2984,7 @@ static void tg3_tx_recover(struct tg3 *t
   and include system chipset information.\n, tp-dev-name);
 
spin_lock(tp-lock);
-   spin_lock(tp-tx_lock);
tp-tg3_flags |= TG3_FLAG_TX_RECOVERY_PENDING;
-   spin_unlock(tp-tx_lock);
spin_unlock(tp-lock);
 }
 
@@ -3429,12 +3427,10 @@ static inline void tg3_full_lock(struct 
if (irq_sync)
tg3_irq_quiesce(tp);
spin_lock_bh(tp-lock);
-   spin_lock(tp-tx_lock);
 }
 
 static inline void tg3_full_unlock(struct tg3 *tp)
 {
-   spin_unlock(tp-tx_lock);
spin_unlock_bh(tp-lock);
 }
 


-
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

Re: [PATCH 2.6.18 1/3] tg3: Remove unnecessary tx_lock

2006-06-05 Thread Herbert Xu 
On Mon, Jun 05, 2006 at 12:47:23PM -0700, Michael Chan wrote:
 Remove tx_lock where it is unnecessary. tg3 runs lockless and so it
 requires interrupts to be disabled and sync'ed, netif_queue and NAPI
 poll to be stopped before the device can be reconfigured. After
 stopping everything, it is no longer necessary to get the tx_lock.

The paths where full lock is preceded by netif_tx_disable are obviously
safe (well, once you take off the LLTX flag anyway).  However, there are
paths that don't do netif_tx_disable (e.g., tg3_set_rx_mode), are they
safe as well?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
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

Re: [PATCH 2.6.18 1/3] tg3: Remove unnecessary tx_lock

2006-06-05 Thread Michael Chan 
Herbert Xu wrote:

 The paths where full lock is preceded by netif_tx_disable are 
 obviously
 safe (well, once you take off the LLTX flag anyway).  
 However, there are
 paths that don't do netif_tx_disable (e.g., tg3_set_rx_mode), are they
 safe as well?
 

In places where we don't call tg3_netif_stop() before tg3_full_lock(),
we are typically doing one of the following:

- changing tg3_flags or tg3_flags2
- reprogramming MAC address
- reprogramming interrupt coalescing values
- reprogramming the PHY
- setting the rx mode

The TX code does not do any of these so it is safe.

tg3_set_rx_mode() is actually called with xmit_lock held through
dev-set_multicast_list(). Or it is called after tg3_netif_stop() in
the VLAN registration calls (in my patch #3).

-
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