netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: remove LLTX in atl2 driver
@ 2008-09-25  0:35 Kevin Hao
  2008-09-25 20:30 ` Chris Snook
  2008-09-26  1:24 ` Jay Cliburn
  0 siblings, 2 replies; 9+ messages in thread
From: Kevin Hao @ 2008-09-25  0:35 UTC (permalink / raw)
  To: Jeff Garzik, Jay Cliburn, Chris Snook, netdev, linux-kernel

When NETIF_F_LLTX is set, the atlx driver will use a private lock.
But in recent kernels this implementation seems redundant and
can cause problems where AF_PACKET sees things twice. Since
NETIF_F_LLTX is marked as deprecated and shouldn't be used in
new driver, this patch removes NETIF_F_LLTX and adds a mmiowb
before sending packet.

Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
---
 drivers/net/atlx/atl2.c |   24 +-----------------------
 drivers/net/atlx/atl2.h |    1 -
 2 files changed, 1 insertions(+), 24 deletions(-)

diff --git a/drivers/net/atlx/atl2.c b/drivers/net/atlx/atl2.c
index b2995ac..3df4ee1 100644
--- a/drivers/net/atlx/atl2.c
+++ b/drivers/net/atlx/atl2.c
@@ -116,7 +116,6 @@ static int __devinit atl2_sw_init(struct atl2_adapter *adapter)
 	hw->max_frame_size = adapter->netdev->mtu;
 
 	spin_lock_init(&adapter->stats_lock);
-	spin_lock_init(&adapter->tx_lock);
 
 	set_bit(__ATL2_DOWN, &adapter->flags);
 
@@ -749,11 +748,7 @@ static void atl2_down(struct atl2_adapter *adapter)
 	 * reschedule our watchdog timer */
 	set_bit(__ATL2_DOWN, &adapter->flags);
 
-#ifdef NETIF_F_LLTX
-	netif_stop_queue(netdev);
-#else
 	netif_tx_disable(netdev);
-#endif
 
 	/* reset MAC to disable all RX/TX */
 	atl2_reset_hw(&adapter->hw);
@@ -829,7 +824,6 @@ static inline int TxdFreeBytes(struct atl2_adapter *adapter)
 static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 {
 	struct atl2_adapter *adapter = netdev_priv(netdev);
-	unsigned long flags;
 	struct tx_pkt_header *txph;
 	u32 offset, copy_len;
 	int txs_unused;
@@ -845,16 +839,6 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 		return NETDEV_TX_OK;
 	}
 
-#ifdef NETIF_F_LLTX
-	local_irq_save(flags);
-	if (!spin_trylock(&adapter->tx_lock)) {
-		/* Collision - tell upper layer to requeue */
-		local_irq_restore(flags);
-		return NETDEV_TX_LOCKED;
-	}
-#else
-	spin_lock_irqsave(&adapter->tx_lock, flags);
-#endif
 	txs_unused = TxsFreeUnit(adapter);
 	txbuf_unused = TxdFreeBytes(adapter);
 
@@ -862,7 +846,6 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 		txs_unused < 1) {
 		/* not enough resources */
 		netif_stop_queue(netdev);
-		spin_unlock_irqrestore(&adapter->tx_lock, flags);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -908,8 +891,7 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 	ATL2_WRITE_REGW(&adapter->hw, REG_MB_TXD_WR_IDX,
 		(adapter->txd_write_ptr >> 2));
 
-	spin_unlock_irqrestore(&adapter->tx_lock, flags);
-
+	mmiowb();
 	netdev->trans_start = jiffies;
 	dev_kfree_skb_any(skb);
 	return NETDEV_TX_OK;
@@ -1447,10 +1429,6 @@ static int __devinit atl2_probe(struct pci_dev *pdev,
 	netdev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
 #endif
 
-#ifdef NETIF_F_LLTX
-	netdev->features |= NETIF_F_LLTX;
-#endif
-
 	/* Init PHY as early as possible due to power saving issue  */
 	atl2_phy_init(&adapter->hw);
 
diff --git a/drivers/net/atlx/atl2.h b/drivers/net/atlx/atl2.h
index 6e1f28f..09974df 100644
--- a/drivers/net/atlx/atl2.h
+++ b/drivers/net/atlx/atl2.h
@@ -462,7 +462,6 @@ struct atl2_adapter {
 	u16 link_duplex;
 
 	spinlock_t stats_lock;
-	spinlock_t tx_lock;
 
 	struct work_struct reset_task;
 	struct work_struct link_chg_task;
-- 
1.5.6.2.220.g44701


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] net: remove LLTX in atl2 driver
  2008-09-25  0:35 [PATCH] net: remove LLTX in atl2 driver Kevin Hao
@ 2008-09-25 20:30 ` Chris Snook
  2008-09-25 20:55   ` Jeff Garzik
  2008-09-26  1:24 ` Jay Cliburn
  1 sibling, 1 reply; 9+ messages in thread
From: Chris Snook @ 2008-09-25 20:30 UTC (permalink / raw)
  To: Kevin Hao; +Cc: Jeff Garzik, Jay Cliburn, netdev, linux-kernel

Kevin Hao wrote:
> When NETIF_F_LLTX is set, the atlx driver will use a private lock.
> But in recent kernels this implementation seems redundant and
> can cause problems where AF_PACKET sees things twice. Since
> NETIF_F_LLTX is marked as deprecated and shouldn't be used in
> new driver, this patch removes NETIF_F_LLTX and adds a mmiowb
> before sending packet.
> 
> Signed-off-by: Kevin Hao <kexin.hao@windriver.com>

Can you explain a bit more concretely the problem this solves, and your testing? 
  Ultimately we'll want to merge this code with the atl1 code, so we need to be 
confident that we can and should make the same change there.

-- Chris

> ---
>  drivers/net/atlx/atl2.c |   24 +-----------------------
>  drivers/net/atlx/atl2.h |    1 -
>  2 files changed, 1 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/atlx/atl2.c b/drivers/net/atlx/atl2.c
> index b2995ac..3df4ee1 100644
> --- a/drivers/net/atlx/atl2.c
> +++ b/drivers/net/atlx/atl2.c
> @@ -116,7 +116,6 @@ static int __devinit atl2_sw_init(struct atl2_adapter *adapter)
>  	hw->max_frame_size = adapter->netdev->mtu;
>  
>  	spin_lock_init(&adapter->stats_lock);
> -	spin_lock_init(&adapter->tx_lock);
>  
>  	set_bit(__ATL2_DOWN, &adapter->flags);
>  
> @@ -749,11 +748,7 @@ static void atl2_down(struct atl2_adapter *adapter)
>  	 * reschedule our watchdog timer */
>  	set_bit(__ATL2_DOWN, &adapter->flags);
>  
> -#ifdef NETIF_F_LLTX
> -	netif_stop_queue(netdev);
> -#else
>  	netif_tx_disable(netdev);
> -#endif
>  
>  	/* reset MAC to disable all RX/TX */
>  	atl2_reset_hw(&adapter->hw);
> @@ -829,7 +824,6 @@ static inline int TxdFreeBytes(struct atl2_adapter *adapter)
>  static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
>  {
>  	struct atl2_adapter *adapter = netdev_priv(netdev);
> -	unsigned long flags;
>  	struct tx_pkt_header *txph;
>  	u32 offset, copy_len;
>  	int txs_unused;
> @@ -845,16 +839,6 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
>  		return NETDEV_TX_OK;
>  	}
>  
> -#ifdef NETIF_F_LLTX
> -	local_irq_save(flags);
> -	if (!spin_trylock(&adapter->tx_lock)) {
> -		/* Collision - tell upper layer to requeue */
> -		local_irq_restore(flags);
> -		return NETDEV_TX_LOCKED;
> -	}
> -#else
> -	spin_lock_irqsave(&adapter->tx_lock, flags);
> -#endif
>  	txs_unused = TxsFreeUnit(adapter);
>  	txbuf_unused = TxdFreeBytes(adapter);
>  
> @@ -862,7 +846,6 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
>  		txs_unused < 1) {
>  		/* not enough resources */
>  		netif_stop_queue(netdev);
> -		spin_unlock_irqrestore(&adapter->tx_lock, flags);
>  		return NETDEV_TX_BUSY;
>  	}
>  
> @@ -908,8 +891,7 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
>  	ATL2_WRITE_REGW(&adapter->hw, REG_MB_TXD_WR_IDX,
>  		(adapter->txd_write_ptr >> 2));
>  
> -	spin_unlock_irqrestore(&adapter->tx_lock, flags);
> -
> +	mmiowb();
>  	netdev->trans_start = jiffies;
>  	dev_kfree_skb_any(skb);
>  	return NETDEV_TX_OK;
> @@ -1447,10 +1429,6 @@ static int __devinit atl2_probe(struct pci_dev *pdev,
>  	netdev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
>  #endif
>  
> -#ifdef NETIF_F_LLTX
> -	netdev->features |= NETIF_F_LLTX;
> -#endif
> -
>  	/* Init PHY as early as possible due to power saving issue  */
>  	atl2_phy_init(&adapter->hw);
>  
> diff --git a/drivers/net/atlx/atl2.h b/drivers/net/atlx/atl2.h
> index 6e1f28f..09974df 100644
> --- a/drivers/net/atlx/atl2.h
> +++ b/drivers/net/atlx/atl2.h
> @@ -462,7 +462,6 @@ struct atl2_adapter {
>  	u16 link_duplex;
>  
>  	spinlock_t stats_lock;
> -	spinlock_t tx_lock;
>  
>  	struct work_struct reset_task;
>  	struct work_struct link_chg_task;


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] net: remove LLTX in atl2 driver
  2008-09-25 20:30 ` Chris Snook
@ 2008-09-25 20:55   ` Jeff Garzik
  2008-09-25 22:34     ` Chris Snook
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Garzik @ 2008-09-25 20:55 UTC (permalink / raw)
  To: Chris Snook; +Cc: Kevin Hao, Jeff Garzik, Jay Cliburn, netdev, linux-kernel

Chris Snook wrote:
> Kevin Hao wrote:
>> When NETIF_F_LLTX is set, the atlx driver will use a private lock.
>> But in recent kernels this implementation seems redundant and
>> can cause problems where AF_PACKET sees things twice. Since
>> NETIF_F_LLTX is marked as deprecated and shouldn't be used in
>> new driver, this patch removes NETIF_F_LLTX and adds a mmiowb
>> before sending packet.
>>
>> Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
> 
> Can you explain a bit more concretely the problem this solves, and your 
> testing?  Ultimately we'll want to merge this code with the atl1 code, 
> so we need to be confident that we can and should make the same change 
> there.
> 
> -- Chris
> 
>> ---
>>  drivers/net/atlx/atl2.c |   24 +-----------------------
>>  drivers/net/atlx/atl2.h |    1 -
>>  2 files changed, 1 insertions(+), 24 deletions(-)

Not directly addressing your question, but LLTX is indeed deprecated and 
in general we want to move away from it.

	Jeff




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] net: remove LLTX in atl2 driver
  2008-09-25 20:55   ` Jeff Garzik
@ 2008-09-25 22:34     ` Chris Snook
  2008-09-25 23:58       ` Jay Cliburn
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Snook @ 2008-09-25 22:34 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Kevin Hao, Jeff Garzik, Jay Cliburn, netdev, linux-kernel

Jeff Garzik wrote:
> Chris Snook wrote:
>> Kevin Hao wrote:
>>> When NETIF_F_LLTX is set, the atlx driver will use a private lock.
>>> But in recent kernels this implementation seems redundant and
>>> can cause problems where AF_PACKET sees things twice. Since
>>> NETIF_F_LLTX is marked as deprecated and shouldn't be used in
>>> new driver, this patch removes NETIF_F_LLTX and adds a mmiowb
>>> before sending packet.
>>>
>>> Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
>>
>> Can you explain a bit more concretely the problem this solves, and 
>> your testing?  Ultimately we'll want to merge this code with the atl1 
>> code, so we need to be confident that we can and should make the same 
>> change there.
>>
>> -- Chris
>>
>>> ---
>>>  drivers/net/atlx/atl2.c |   24 +-----------------------
>>>  drivers/net/atlx/atl2.h |    1 -
>>>  2 files changed, 1 insertions(+), 24 deletions(-)
> 
> Not directly addressing your question, but LLTX is indeed deprecated and 
> in general we want to move away from it.
> 
>     Jeff

I'm all in favor of removing legacy cruft.  I want to know a little more about 
it so we can remove it from the atl1 code as well.  Ultimately most of this will 
be shared code.

-- Chris

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] net: remove LLTX in atl2 driver
  2008-09-25 22:34     ` Chris Snook
@ 2008-09-25 23:58       ` Jay Cliburn
  2008-09-26  1:20         ` Kevin Hao
  0 siblings, 1 reply; 9+ messages in thread
From: Jay Cliburn @ 2008-09-25 23:58 UTC (permalink / raw)
  To: Chris Snook; +Cc: Jeff Garzik, Kevin Hao, Jeff Garzik, netdev, linux-kernel

On Thu, 25 Sep 2008 18:34:29 -0400
Chris Snook <csnook@redhat.com> wrote:

> 
> I'm all in favor of removing legacy cruft.  I want to know a little
> more about it so we can remove it from the atl1 code as well.
> Ultimately most of this will be shared code.

Yes, LLTX can (and should) be removed from the atl1 code.  I had an
exchange with Herbert Xu and David Miller on LKML a few weeks ago, and
they gave me a few tips on how to do it, but I haven't yet had the
time.  What takes an experienced netdev warrior about a half hour to
implement unfortunately takes me days of after-dayjob time as I track
through all the calls that that are affected and test the modified
driver.

Now back to atl2...  I'd like to hear from Kevin also whether he tested
the LLTX removal.  I'm building the driver with his patch as we speak,
and I'll test it myself this weekend, but Kevin I'd still like to know
if you tested the patch.

Thanks,
Jay

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] net: remove LLTX in atl2 driver
  2008-09-25 23:58       ` Jay Cliburn
@ 2008-09-26  1:20         ` Kevin Hao
  2008-09-26  1:24           ` Chris Snook
  0 siblings, 1 reply; 9+ messages in thread
From: Kevin Hao @ 2008-09-26  1:20 UTC (permalink / raw)
  To: Jay Cliburn; +Cc: Chris Snook, Jeff Garzik, Jeff Garzik, netdev, linux-kernel

On Thu, 2008-09-25 at 18:58 -0500, Jay Cliburn wrote:
> On Thu, 25 Sep 2008 18:34:29 -0400
> Chris Snook <csnook@redhat.com> wrote:
> 
> > 
> > I'm all in favor of removing legacy cruft.  I want to know a little
> > more about it so we can remove it from the atl1 code as well.
> > Ultimately most of this will be shared code.
> 
> Yes, LLTX can (and should) be removed from the atl1 code.  I had an
> exchange with Herbert Xu and David Miller on LKML a few weeks ago, and
> they gave me a few tips on how to do it, but I haven't yet had the
> time.  What takes an experienced netdev warrior about a half hour to
> implement unfortunately takes me days of after-dayjob time as I track
> through all the calls that that are affected and test the modified
> driver.
> 
> Now back to atl2...  I'd like to hear from Kevin also whether he tested
> the LLTX removal.  I'm building the driver with his patch as we speak,
> and I'll test it myself this weekend, but Kevin I'd still like to know
> if you tested the patch.

Yes, I have tested this driver on a Eee PC. It works well.

Thanks,
Kevin

> 
> Thanks,
> Jay

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] net: remove LLTX in atl2 driver
  2008-09-26  1:20         ` Kevin Hao
@ 2008-09-26  1:24           ` Chris Snook
  2008-09-26  2:20             ` Kevin Hao
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Snook @ 2008-09-26  1:24 UTC (permalink / raw)
  To: Kevin Hao; +Cc: Jay Cliburn, Jeff Garzik, Jeff Garzik, netdev, linux-kernel

Kevin Hao wrote:
> On Thu, 2008-09-25 at 18:58 -0500, Jay Cliburn wrote:
>> On Thu, 25 Sep 2008 18:34:29 -0400
>> Chris Snook <csnook@redhat.com> wrote:
>>
>>> I'm all in favor of removing legacy cruft.  I want to know a little
>>> more about it so we can remove it from the atl1 code as well.
>>> Ultimately most of this will be shared code.
>> Yes, LLTX can (and should) be removed from the atl1 code.  I had an
>> exchange with Herbert Xu and David Miller on LKML a few weeks ago, and
>> they gave me a few tips on how to do it, but I haven't yet had the
>> time.  What takes an experienced netdev warrior about a half hour to
>> implement unfortunately takes me days of after-dayjob time as I track
>> through all the calls that that are affected and test the modified
>> driver.
>>
>> Now back to atl2...  I'd like to hear from Kevin also whether he tested
>> the LLTX removal.  I'm building the driver with his patch as we speak,
>> and I'll test it myself this weekend, but Kevin I'd still like to know
>> if you tested the patch.
> 
> Yes, I have tested this driver on a Eee PC. It works well.

Thanks, please mention this with your patch submissions.

-- Chris

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] net: remove LLTX in atl2 driver
  2008-09-25  0:35 [PATCH] net: remove LLTX in atl2 driver Kevin Hao
  2008-09-25 20:30 ` Chris Snook
@ 2008-09-26  1:24 ` Jay Cliburn
  1 sibling, 0 replies; 9+ messages in thread
From: Jay Cliburn @ 2008-09-26  1:24 UTC (permalink / raw)
  To: Kevin Hao; +Cc: Jeff Garzik, Chris Snook, netdev, linux-kernel

On Thu, 25 Sep 2008 08:35:22 +0800
Kevin Hao <kexin.hao@windriver.com> wrote:

> When NETIF_F_LLTX is set, the atlx driver will use a private lock.
> But in recent kernels this implementation seems redundant and
> can cause problems where AF_PACKET sees things twice. Since
> NETIF_F_LLTX is marked as deprecated and shouldn't be used in
> new driver, this patch removes NETIF_F_LLTX and adds a mmiowb
> before sending packet.
> 
> Signed-off-by: Kevin Hao <kexin.hao@windriver.com>

Seems to work fine with large file (3.5 GiB) two-way scp and two-way
iperf runs.

[root@osprey ~]# ifconfig eth1
eth1      Link encap:Ethernet  HWaddr 00:13:74:00:5C:38  
          inet addr:192.168.1.33  Bcast:192.168.1.255  Mask:255.255.255.0
          inet6 addr: fe80::213:74ff:fe00:5c38/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1492  Metric:1
          RX packets:10054352 errors:0 dropped:0 overruns:0 frame:1
          TX packets:12364268 errors:0 dropped:0 overruns:0 carrier:6
          collisions:0 txqueuelen:1000 
          RX bytes:11990462653 (11.1 GiB)  TX bytes:12351847745 (11.5 GiB)
          Memory:dbfc0000-dc000000 

Acked-by: Jay Cliburn <jacliburn@bellsouth.net>

> ---
>  drivers/net/atlx/atl2.c |   24 +-----------------------
>  drivers/net/atlx/atl2.h |    1 -
>  2 files changed, 1 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/atlx/atl2.c b/drivers/net/atlx/atl2.c
> index b2995ac..3df4ee1 100644
> --- a/drivers/net/atlx/atl2.c
> +++ b/drivers/net/atlx/atl2.c
> @@ -116,7 +116,6 @@ static int __devinit atl2_sw_init(struct
> atl2_adapter *adapter) hw->max_frame_size = adapter->netdev->mtu;
>  
>  	spin_lock_init(&adapter->stats_lock);
> -	spin_lock_init(&adapter->tx_lock);
>  
>  	set_bit(__ATL2_DOWN, &adapter->flags);
>  
> @@ -749,11 +748,7 @@ static void atl2_down(struct atl2_adapter
> *adapter)
>  	 * reschedule our watchdog timer */
>  	set_bit(__ATL2_DOWN, &adapter->flags);
>  
> -#ifdef NETIF_F_LLTX
> -	netif_stop_queue(netdev);
> -#else
>  	netif_tx_disable(netdev);
> -#endif
>  
>  	/* reset MAC to disable all RX/TX */
>  	atl2_reset_hw(&adapter->hw);
> @@ -829,7 +824,6 @@ static inline int TxdFreeBytes(struct
> atl2_adapter *adapter) static int atl2_xmit_frame(struct sk_buff
> *skb, struct net_device *netdev) {
>  	struct atl2_adapter *adapter = netdev_priv(netdev);
> -	unsigned long flags;
>  	struct tx_pkt_header *txph;
>  	u32 offset, copy_len;
>  	int txs_unused;
> @@ -845,16 +839,6 @@ static int atl2_xmit_frame(struct sk_buff *skb,
> struct net_device *netdev) return NETDEV_TX_OK;
>  	}
>  
> -#ifdef NETIF_F_LLTX
> -	local_irq_save(flags);
> -	if (!spin_trylock(&adapter->tx_lock)) {
> -		/* Collision - tell upper layer to requeue */
> -		local_irq_restore(flags);
> -		return NETDEV_TX_LOCKED;
> -	}
> -#else
> -	spin_lock_irqsave(&adapter->tx_lock, flags);
> -#endif
>  	txs_unused = TxsFreeUnit(adapter);
>  	txbuf_unused = TxdFreeBytes(adapter);
>  
> @@ -862,7 +846,6 @@ static int atl2_xmit_frame(struct sk_buff *skb,
> struct net_device *netdev) txs_unused < 1) {
>  		/* not enough resources */
>  		netif_stop_queue(netdev);
> -		spin_unlock_irqrestore(&adapter->tx_lock, flags);
>  		return NETDEV_TX_BUSY;
>  	}
>  
> @@ -908,8 +891,7 @@ static int atl2_xmit_frame(struct sk_buff *skb,
> struct net_device *netdev) ATL2_WRITE_REGW(&adapter->hw,
> REG_MB_TXD_WR_IDX, (adapter->txd_write_ptr >> 2));
>  
> -	spin_unlock_irqrestore(&adapter->tx_lock, flags);
> -
> +	mmiowb();
>  	netdev->trans_start = jiffies;
>  	dev_kfree_skb_any(skb);
>  	return NETDEV_TX_OK;
> @@ -1447,10 +1429,6 @@ static int __devinit atl2_probe(struct pci_dev
> *pdev, netdev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
>  #endif
>  
> -#ifdef NETIF_F_LLTX
> -	netdev->features |= NETIF_F_LLTX;
> -#endif
> -
>  	/* Init PHY as early as possible due to power saving issue
> */ atl2_phy_init(&adapter->hw);
>  
> diff --git a/drivers/net/atlx/atl2.h b/drivers/net/atlx/atl2.h
> index 6e1f28f..09974df 100644
> --- a/drivers/net/atlx/atl2.h
> +++ b/drivers/net/atlx/atl2.h
> @@ -462,7 +462,6 @@ struct atl2_adapter {
>  	u16 link_duplex;
>  
>  	spinlock_t stats_lock;
> -	spinlock_t tx_lock;
>  
>  	struct work_struct reset_task;
>  	struct work_struct link_chg_task;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] net: remove LLTX in atl2 driver
  2008-09-26  1:24           ` Chris Snook
@ 2008-09-26  2:20             ` Kevin Hao
  0 siblings, 0 replies; 9+ messages in thread
From: Kevin Hao @ 2008-09-26  2:20 UTC (permalink / raw)
  To: Chris Snook; +Cc: Jay Cliburn, Jeff Garzik, Jeff Garzik, netdev, linux-kernel

When NETIF_F_LLTX is set, the atlx driver will use a private lock.
But in recent kernels this implementation seems redundant and
can cause problems where AF_PACKET sees things twice. Since
NETIF_F_LLTX is marked as deprecated and shouldn't be used in
new driver, this patch removes NETIF_F_LLTX and adds a mmiowb
before sending packet. I have tested this driver on a Eee PC.
It works well.

Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
Acked-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl2.c |   24 +-----------------------
 drivers/net/atlx/atl2.h |    1 -
 2 files changed, 1 insertions(+), 24 deletions(-)

diff --git a/drivers/net/atlx/atl2.c b/drivers/net/atlx/atl2.c
index b2995ac..3df4ee1 100644
--- a/drivers/net/atlx/atl2.c
+++ b/drivers/net/atlx/atl2.c
@@ -116,7 +116,6 @@ static int __devinit atl2_sw_init(struct atl2_adapter *adapter)
 	hw->max_frame_size = adapter->netdev->mtu;
 
 	spin_lock_init(&adapter->stats_lock);
-	spin_lock_init(&adapter->tx_lock);
 
 	set_bit(__ATL2_DOWN, &adapter->flags);
 
@@ -749,11 +748,7 @@ static void atl2_down(struct atl2_adapter *adapter)
 	 * reschedule our watchdog timer */
 	set_bit(__ATL2_DOWN, &adapter->flags);
 
-#ifdef NETIF_F_LLTX
-	netif_stop_queue(netdev);
-#else
 	netif_tx_disable(netdev);
-#endif
 
 	/* reset MAC to disable all RX/TX */
 	atl2_reset_hw(&adapter->hw);
@@ -829,7 +824,6 @@ static inline int TxdFreeBytes(struct atl2_adapter *adapter)
 static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 {
 	struct atl2_adapter *adapter = netdev_priv(netdev);
-	unsigned long flags;
 	struct tx_pkt_header *txph;
 	u32 offset, copy_len;
 	int txs_unused;
@@ -845,16 +839,6 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 		return NETDEV_TX_OK;
 	}
 
-#ifdef NETIF_F_LLTX
-	local_irq_save(flags);
-	if (!spin_trylock(&adapter->tx_lock)) {
-		/* Collision - tell upper layer to requeue */
-		local_irq_restore(flags);
-		return NETDEV_TX_LOCKED;
-	}
-#else
-	spin_lock_irqsave(&adapter->tx_lock, flags);
-#endif
 	txs_unused = TxsFreeUnit(adapter);
 	txbuf_unused = TxdFreeBytes(adapter);
 
@@ -862,7 +846,6 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 		txs_unused < 1) {
 		/* not enough resources */
 		netif_stop_queue(netdev);
-		spin_unlock_irqrestore(&adapter->tx_lock, flags);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -908,8 +891,7 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 	ATL2_WRITE_REGW(&adapter->hw, REG_MB_TXD_WR_IDX,
 		(adapter->txd_write_ptr >> 2));
 
-	spin_unlock_irqrestore(&adapter->tx_lock, flags);
-
+	mmiowb();
 	netdev->trans_start = jiffies;
 	dev_kfree_skb_any(skb);
 	return NETDEV_TX_OK;
@@ -1447,10 +1429,6 @@ static int __devinit atl2_probe(struct pci_dev *pdev,
 	netdev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
 #endif
 
-#ifdef NETIF_F_LLTX
-	netdev->features |= NETIF_F_LLTX;
-#endif
-
 	/* Init PHY as early as possible due to power saving issue  */
 	atl2_phy_init(&adapter->hw);
 
diff --git a/drivers/net/atlx/atl2.h b/drivers/net/atlx/atl2.h
index 6e1f28f..09974df 100644
--- a/drivers/net/atlx/atl2.h
+++ b/drivers/net/atlx/atl2.h
@@ -462,7 +462,6 @@ struct atl2_adapter {
 	u16 link_duplex;
 
 	spinlock_t stats_lock;
-	spinlock_t tx_lock;
 
 	struct work_struct reset_task;
 	struct work_struct link_chg_task;
-- 
1.5.6.2.220.g44701


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2008-09-26  2:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-25  0:35 [PATCH] net: remove LLTX in atl2 driver Kevin Hao
2008-09-25 20:30 ` Chris Snook
2008-09-25 20:55   ` Jeff Garzik
2008-09-25 22:34     ` Chris Snook
2008-09-25 23:58       ` Jay Cliburn
2008-09-26  1:20         ` Kevin Hao
2008-09-26  1:24           ` Chris Snook
2008-09-26  2:20             ` Kevin Hao
2008-09-26  1:24 ` Jay Cliburn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).