pch_gbe: oops with vlan (new)

Folks,

I am looking for help in debugging a pch_gbe driver oops/abort.

Kernel: version 2.6.32-220.el6.i686 (RHEL6.2)
Driver: pch_gbe version 0.91-NAPI  (source tarball we used is at
https://sendfile.kontron.com/message/24tdUi6MXklnUtBLnOsumq until May
16)
NIC: 0b:00.1 Ethernet controller [0200]: Intel Corporation Platform
Controller Hub EG20T Gigabit Ethernet Controller [8086:8802] (rev 02)

Configuration, with VLAN:
 eth0 (not started)
 eth0.100 = 192.168.100.1
 eth0.200 = 192.168.200.1
 eth0.6  = 192.168.6.1

When starting the VLAN configuration, then doing a ping test for >= 5
minutes, I get a kernel oop/abort message as shown below.  This does not
happen without configuring VLAN.
Where should I look for possible causes for a transmit queue timeout
like this?  

I have contacted the OKI/LAPIS driver authors, but no response so far.
I thought that this group might be able to comment from similar
experiences.

Andy

May 11 11:06:09 kontron kernel: ------------[ cut here ]------------
May 11 11:06:09 kontron kernel: WARNING: at net/sched/sch_generic.c:261
dev_watchdog+0x1ec/0x200() (Not tainted)
May 11 11:06:09 kontron kernel: Hardware name: N/A
May 11 11:06:09 kontron kernel: NETDEV WATCHDOG: eth0 (pch_gbe):
transmit queue 0 timed out
May 11 11:06:09 kontron kernel: Modules linked in: fuse ip6table_filter
ip6_tables ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4
nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT xt_CHECKSUM
iptable_mangle iptable_filter ip_tables tun bridge autofs4 sunrpc
cpufreq_ondemand acpi_cpufreq mperf 8021q garp stp llc ipv6 ext3 jbd
uinput ppdev parport_pc parport sg microcode pch_gbe(U) mii serio_raw
snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq
snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc ext4
mbcache jbd2 sd_mod crc_t10dif ahci sdhci_pci sdhci mmc_core video
output dm_mirror dm_region_hash dm_log dm_mod [last unloaded:
scsi_wait_scan]
May 11 11:06:09 kontron kernel: Pid: 0, comm: swapper Not tainted
2.6.32-220.el6.i686 #1
May 11 11:06:09 kontron kernel: Call Trace:
May 11 11:06:09 kontron kernel: [<c0454c81>] ?
warn_slowpath_common+0x81/0xc0
May 11 11:06:09 kontron kernel: [<c07a16bc>] ? dev_watchdog+0x1ec/0x200
May 11 11:06:09 kontron kernel: [<c07a16bc>] ? dev_watchdog+0x1ec/0x200
May 11 11:06:09 kontron kernel: [<c0454d53>] ?
warn_slowpath_fmt+0x33/0x40
May 11 11:06:09 kontron kernel: [<c07a16bc>] ? dev_watchdog+0x1ec/0x200
May 11 11:06:09 kontron kernel: [<c0471bfa>] ? insert_work+0x5a/0xb0
May 11 11:06:09 kontron kernel: [<c04656f9>] ?
run_timer_softirq+0x139/0x2c0
May 11 11:06:09 kontron kernel: [<c0831315>] ?
apic_timer_interrupt+0x31/0x38
May 11 11:06:09 kontron kernel: [<c07a14d0>] ? dev_watchdog+0x0/0x200
May 11 11:06:09 kontron kernel: [<c045be4a>] ? __do_softirq+0x8a/0x1a0
May 11 11:06:09 kontron kernel: [<c045bf9d>] ? do_softirq+0x3d/0x50
May 11 11:06:09 kontron kernel: [<c045c0f5>] ? irq_exit+0x65/0x70
May 11 11:06:09 kontron kernel: [<c0428473>] ?
smp_apic_timer_interrupt+0x53/0x90
May 11 11:06:09 kontron kernel: [<c0831315>] ?
apic_timer_interrupt+0x31/0x38
May 11 11:06:09 kontron kernel: [<c045007b>] ?
throttle_cfs_rq+0x6b/0x130
May 11 11:06:09 kontron kernel: [<c064735f>] ? intel_idle+0xaf/0x140
May 11 11:06:09 kontron kernel: [<c075c282>] ?
cpuidle_idle_call+0x72/0x100
May 11 11:06:09 kontron kernel: [<c0408964>] ? cpu_idle+0x94/0xd0
May 11 11:06:09 kontron kernel: [<c082a645>] ?
start_secondary+0x20d/0x252
May 11 11:06:09 kontron kernel: ---[ end trace 3672ff56500ae344 ]---
May 11 11:06:09 kontron NetworkManager[1608]: <info> (eth0): carrier now
OFF (device state 3)
May 11 11:06:09 kontron NetworkManager[1608]: <info> (eth0): device
state change: 3 -> 2 (reason 40)
May 11 11:06:09 kontron NetworkManager[1608]: <info> (eth0):
deactivating device (reason: 40).

On Fri, 2012-05-11 at 13:48 -0700, Andy Cress wrote:

Folks,

I am looking for help in debugging a pch_gbe driver oops/abort.

Kernel: version 2.6.32-220.el6.i686 (RHEL6.2)
Driver: pch_gbe version 0.91-NAPI  (source tarball we used is at
https://sendfile.kontron.com/message/24tdUi6MXklnUtBLnOsumq until May
16)
NIC: 0b:00.1 Ethernet controller [0200]: Intel Corporation Platform
Controller Hub EG20T Gigabit Ethernet Controller [8086:8802] (rev 02)

Configuration, with VLAN:
 eth0 (not started)
 eth0.100 = 192.168.100.1
 eth0.200 = 192.168.200.1
 eth0.6  = 192.168.6.1

When starting the VLAN configuration, then doing a ping test for >= 5
minutes, I get a kernel oop/abort message as shown below.  This does not
happen without configuring VLAN.
Where should I look for possible causes for a transmit queue timeout
like this?  

I have contacted the OKI/LAPIS driver authors, but no response so far.
I thought that this group might be able to comment from similar
experiences.

Andy

typical sign of a buggy driver

A quick look in current Linus tree show a non existent synchronization
between ndo_start_xmit and TX completion.

tx completion uses a tx_queue_lock spinlock for nothing but false sense
of correctness.

# find drivers/net/ethernet/oki-semi/pch_gbe -name "*.[ch]"|xargs grep -4 -n tx_queue_lock
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-583-
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-584-/**
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-585- * struct pch_gbe_adapter - board specific private data structure
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-586- * @stats_lock:	Spinlock structure for status
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h:587: * @tx_queue_lock:	Spinlock structure for transmit
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-588- * @ethtool_lock:	Spinlock structure for ethtool
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-589- * @irq_sem:		Semaphore for interrupt
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-590- * @netdev:		Pointer of network device structure
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-591- * @pdev:		Pointer of pci device structure
--
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-608- */
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-609-
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-610-struct pch_gbe_adapter {
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-611-	spinlock_t stats_lock;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h:612:	spinlock_t tx_queue_lock;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-613-	spinlock_t ethtool_lock;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-614-	atomic_t irq_sem;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-615-	struct net_device *netdev;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h-616-	struct pci_dev *pdev;
--
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1641-		netif_wake_queue(adapter->netdev);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1642-		adapter->stats.tx_restart_count++;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1643-		pr_debug("Tx wake queue\n");
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1644-	}
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:1645:	spin_lock(&adapter->tx_queue_lock);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1646-	tx_ring->next_to_clean = i;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:1647:	spin_unlock(&adapter->tx_queue_lock);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1648-	pr_debug("next_to_clean : %d\n", tx_ring->next_to_clean);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1649-	return cleaned;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1650-}
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-1651-
--
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-2036-		pr_err("Unable to allocate memory for queues\n");
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-2037-		return -ENOMEM;
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-2038-	}
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-2039-	spin_lock_init(&adapter->hw.miim_lock);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:2040:	spin_lock_init(&adapter->tx_queue_lock);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-2041-	spin_lock_init(&adapter->stats_lock);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-2042-	spin_lock_init(&adapter->ethtool_lock);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-2043-	atomic_set(&adapter->irq_sem, 0);
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c-2044-	pch_gbe_irq_disable(adapter);

On Fri, 2012-05-11 at 23:13 +0200, Eric Dumazet wrote:

On Fri, 2012-05-11 at 13:48 -0700, Andy Cress wrote:

quoted

Folks,

I am looking for help in debugging a pch_gbe driver oops/abort.

Kernel: version 2.6.32-220.el6.i686 (RHEL6.2)
Driver: pch_gbe version 0.91-NAPI  (source tarball we used is at
https://sendfile.kontron.com/message/24tdUi6MXklnUtBLnOsumq until May
16)
NIC: 0b:00.1 Ethernet controller [0200]: Intel Corporation Platform
Controller Hub EG20T Gigabit Ethernet Controller [8086:8802] (rev 02)

Configuration, with VLAN:
 eth0 (not started)
 eth0.100 = 192.168.100.1
 eth0.200 = 192.168.200.1
 eth0.6  = 192.168.6.1

When starting the VLAN configuration, then doing a ping test for >= 5
minutes, I get a kernel oop/abort message as shown below.  This does not
happen without configuring VLAN.
Where should I look for possible causes for a transmit queue timeout
like this?  

I have contacted the OKI/LAPIS driver authors, but no response so far.
I thought that this group might be able to comment from similar
experiences.

Andy

typical sign of a buggy driver

A quick look in current Linus tree show a non existent synchronization
between ndo_start_xmit and TX completion.

tx completion uses a tx_queue_lock spinlock for nothing but false sense
of correctness.

Please try the following patch : (based on current net-next tree)

Also this driver has a strange RX path : It does a copy of incoming
frames on fixed size skbs, (2048+overhead -> kmalloc-4096 pool) instead
of using skb of the right size...



 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h      |    2 
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |   25 ++++------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 9f3dbc4..b07311e 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h

@@ -584,7 +584,6 @@ struct pch_gbe_hw_stats {
 /**
  * struct pch_gbe_adapter - board specific private data structure
  * @stats_lock:	Spinlock structure for status
- * @tx_queue_lock:	Spinlock structure for transmit
  * @ethtool_lock:	Spinlock structure for ethtool
  * @irq_sem:		Semaphore for interrupt
  * @netdev:		Pointer of network device structure

@@ -609,7 +608,6 @@ struct pch_gbe_hw_stats {
 
 struct pch_gbe_adapter {
 	spinlock_t stats_lock;
-	spinlock_t tx_queue_lock;
 	spinlock_t ethtool_lock;
 	atomic_t irq_sem;
 	struct net_device *netdev;

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 9dc7e50..3787c64 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c

@@ -645,14 +645,11 @@ static void pch_gbe_mac_set_pause_packet(struct pch_gbe_hw *hw)
  */
 static int pch_gbe_alloc_queues(struct pch_gbe_adapter *adapter)
 {
-	int size;
-
-	size = (int)sizeof(struct pch_gbe_tx_ring);
-	adapter->tx_ring = kzalloc(size, GFP_KERNEL);
+	adapter->tx_ring = kzalloc(sizeof(*adapter->tx_ring), GFP_KERNEL);
 	if (!adapter->tx_ring)
 		return -ENOMEM;
-	size = (int)sizeof(struct pch_gbe_rx_ring);
-	adapter->rx_ring = kzalloc(size, GFP_KERNEL);
+
+	adapter->rx_ring = kzalloc(sizeof(*adapter->rx_ring), GFP_KERNEL);
 	if (!adapter->rx_ring) {
 		kfree(adapter->tx_ring);
 		return -ENOMEM;

@@ -1169,7 +1166,6 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	struct sk_buff *tmp_skb;
 	unsigned int frame_ctrl;
 	unsigned int ring_num;
-	unsigned long flags;
 
 	/*-- Set frame control --*/
 	frame_ctrl = 0;

@@ -1216,14 +1212,14 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 			}
 		}
 	}
-	spin_lock_irqsave(&tx_ring->tx_lock, flags);
+
 	ring_num = tx_ring->next_to_use;
 	if (unlikely((ring_num + 1) == tx_ring->count))
 		tx_ring->next_to_use = 0;
 	else
 		tx_ring->next_to_use = ring_num + 1;
 
-	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
+
 	buffer_info = &tx_ring->buffer_info[ring_num];
 	tmp_skb = buffer_info->skb;
 

@@ -1525,7 +1521,7 @@ pch_gbe_alloc_rx_buffers_pool(struct pch_gbe_adapter *adapter,
 						&rx_ring->rx_buff_pool_logic,
 						GFP_KERNEL);
 	if (!rx_ring->rx_buff_pool) {
-		pr_err("Unable to allocate memory for the receive poll buffer\n");
+		pr_err("Unable to allocate memory for the receive pool buffer\n");
 		return -ENOMEM;
 	}
 	memset(rx_ring->rx_buff_pool, 0, size);

@@ -1644,15 +1640,17 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 	pr_debug("called pch_gbe_unmap_and_free_tx_resource() %d count\n",
 		 cleaned_count);
 	/* Recover from running out of Tx resources in xmit_frame */
+	spin_lock(&tx_ring->tx_lock);
 	if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev)))) {
 		netif_wake_queue(adapter->netdev);
 		adapter->stats.tx_restart_count++;
 		pr_debug("Tx wake queue\n");
 	}
-	spin_lock(&adapter->tx_queue_lock);
+
 	tx_ring->next_to_clean = i;
-	spin_unlock(&adapter->tx_queue_lock);
+
 	pr_debug("next_to_clean : %d\n", tx_ring->next_to_clean);
+	spin_unlock(&tx_ring->tx_lock);
 	return cleaned;
 }
 

@@ -2043,7 +2041,6 @@ static int pch_gbe_sw_init(struct pch_gbe_adapter *adapter)
 		return -ENOMEM;
 	}
 	spin_lock_init(&adapter->hw.miim_lock);
-	spin_lock_init(&adapter->tx_queue_lock);
 	spin_lock_init(&adapter->stats_lock);
 	spin_lock_init(&adapter->ethtool_lock);
 	atomic_set(&adapter->irq_sem, 0);

@@ -2148,10 +2145,10 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 			 tx_ring->next_to_use, tx_ring->next_to_clean);
 		return NETDEV_TX_BUSY;
 	}
-	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 
 	/* CRC,ITAG no support */
 	pch_gbe_tx_queue(adapter, tx_ring, skb);
+	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 	return NETDEV_TX_OK;
 }
 

Eric,

This patch resolved the driver oops (transmit queue timeout) with vlan.  It makes a lot of sense that the wrong spin_locks caused this.  
Thanks a lot.

Andy

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Eric Dumazet
Sent: Friday, May 11, 2012 6:11 PM
To: Andy Cress
Cc: netdev
Subject: Re: pch_gbe: oops with vlan (new)

On Fri, 2012-05-11 at 23:13 +0200, Eric Dumazet wrote:

On Fri, 2012-05-11 at 13:48 -0700, Andy Cress wrote:

quoted

Folks,

I am looking for help in debugging a pch_gbe driver oops/abort.

Kernel: version 2.6.32-220.el6.i686 (RHEL6.2)
Driver: pch_gbe version 0.91-NAPI  (source tarball we used is at
https://sendfile.kontron.com/message/24tdUi6MXklnUtBLnOsumq until May
16)
NIC: 0b:00.1 Ethernet controller [0200]: Intel Corporation Platform
Controller Hub EG20T Gigabit Ethernet Controller [8086:8802] (rev 02)

Configuration, with VLAN:
 eth0 (not started)
 eth0.100 = 192.168.100.1
 eth0.200 = 192.168.200.1
 eth0.6  = 192.168.6.1

When starting the VLAN configuration, then doing a ping test for >= 5
minutes, I get a kernel oop/abort message as shown below.  This does not
happen without configuring VLAN.
Where should I look for possible causes for a transmit queue timeout
like this?  

I have contacted the OKI/LAPIS driver authors, but no response so far.
I thought that this group might be able to comment from similar
experiences.

Andy

typical sign of a buggy driver

A quick look in current Linus tree show a non existent synchronization
between ndo_start_xmit and TX completion.

tx completion uses a tx_queue_lock spinlock for nothing but false sense
of correctness.

Please try the following patch : (based on current net-next tree)

Also this driver has a strange RX path : It does a copy of incoming
frames on fixed size skbs, (2048+overhead -> kmalloc-4096 pool) instead
of using skb of the right size...



 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h      |    2 
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |   25 ++++------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 9f3dbc4..b07311e 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h

@@ -584,7 +584,6 @@ struct pch_gbe_hw_stats {
 /**
  * struct pch_gbe_adapter - board specific private data structure
  * @stats_lock:	Spinlock structure for status
- * @tx_queue_lock:	Spinlock structure for transmit
  * @ethtool_lock:	Spinlock structure for ethtool
  * @irq_sem:		Semaphore for interrupt
  * @netdev:		Pointer of network device structure

@@ -609,7 +608,6 @@ struct pch_gbe_hw_stats {
 
 struct pch_gbe_adapter {
 	spinlock_t stats_lock;
-	spinlock_t tx_queue_lock;
 	spinlock_t ethtool_lock;
 	atomic_t irq_sem;
 	struct net_device *netdev;

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 9dc7e50..3787c64 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c

@@ -645,14 +645,11 @@ static void pch_gbe_mac_set_pause_packet(struct pch_gbe_hw *hw)
  */
 static int pch_gbe_alloc_queues(struct pch_gbe_adapter *adapter)
 {
-	int size;
-
-	size = (int)sizeof(struct pch_gbe_tx_ring);
-	adapter->tx_ring = kzalloc(size, GFP_KERNEL);
+	adapter->tx_ring = kzalloc(sizeof(*adapter->tx_ring), GFP_KERNEL);
 	if (!adapter->tx_ring)
 		return -ENOMEM;
-	size = (int)sizeof(struct pch_gbe_rx_ring);
-	adapter->rx_ring = kzalloc(size, GFP_KERNEL);
+
+	adapter->rx_ring = kzalloc(sizeof(*adapter->rx_ring), GFP_KERNEL);
 	if (!adapter->rx_ring) {
 		kfree(adapter->tx_ring);
 		return -ENOMEM;

@@ -1169,7 +1166,6 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	struct sk_buff *tmp_skb;
 	unsigned int frame_ctrl;
 	unsigned int ring_num;
-	unsigned long flags;
 
 	/*-- Set frame control --*/
 	frame_ctrl = 0;

@@ -1216,14 +1212,14 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 			}
 		}
 	}
-	spin_lock_irqsave(&tx_ring->tx_lock, flags);
+
 	ring_num = tx_ring->next_to_use;
 	if (unlikely((ring_num + 1) == tx_ring->count))
 		tx_ring->next_to_use = 0;
 	else
 		tx_ring->next_to_use = ring_num + 1;
 
-	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
+
 	buffer_info = &tx_ring->buffer_info[ring_num];
 	tmp_skb = buffer_info->skb;
 

@@ -1525,7 +1521,7 @@ pch_gbe_alloc_rx_buffers_pool(struct pch_gbe_adapter *adapter,
 						&rx_ring->rx_buff_pool_logic,
 						GFP_KERNEL);
 	if (!rx_ring->rx_buff_pool) {
-		pr_err("Unable to allocate memory for the receive poll buffer\n");
+		pr_err("Unable to allocate memory for the receive pool buffer\n");
 		return -ENOMEM;
 	}
 	memset(rx_ring->rx_buff_pool, 0, size);

@@ -1644,15 +1640,17 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 	pr_debug("called pch_gbe_unmap_and_free_tx_resource() %d count\n",
 		 cleaned_count);
 	/* Recover from running out of Tx resources in xmit_frame */
+	spin_lock(&tx_ring->tx_lock);
 	if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev)))) {
 		netif_wake_queue(adapter->netdev);
 		adapter->stats.tx_restart_count++;
 		pr_debug("Tx wake queue\n");
 	}
-	spin_lock(&adapter->tx_queue_lock);
+
 	tx_ring->next_to_clean = i;
-	spin_unlock(&adapter->tx_queue_lock);
+
 	pr_debug("next_to_clean : %d\n", tx_ring->next_to_clean);
+	spin_unlock(&tx_ring->tx_lock);
 	return cleaned;
 }
 

@@ -2043,7 +2041,6 @@ static int pch_gbe_sw_init(struct pch_gbe_adapter *adapter)
 		return -ENOMEM;
 	}
 	spin_lock_init(&adapter->hw.miim_lock);
-	spin_lock_init(&adapter->tx_queue_lock);
 	spin_lock_init(&adapter->stats_lock);
 	spin_lock_init(&adapter->ethtool_lock);
 	atomic_set(&adapter->irq_sem, 0);

@@ -2148,10 +2145,10 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 			 tx_ring->next_to_use, tx_ring->next_to_clean);
 		return NETDEV_TX_BUSY;
 	}
-	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 
 	/* CRC,ITAG no support */
 	pch_gbe_tx_queue(adapter, tx_ring, skb);
+	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 	return NETDEV_TX_OK;
 }
 


--

To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Eric Dumazet <edumazet@google.com>

Andy reported pch_gbe triggered "NETDEV WATCHDOG" errors.

May 11 11:06:09 kontron kernel: WARNING: at net/sched/sch_generic.c:261
dev_watchdog+0x1ec/0x200() (Not tainted)
May 11 11:06:09 kontron kernel: Hardware name: N/A
May 11 11:06:09 kontron kernel: NETDEV WATCHDOG: eth0 (pch_gbe):
transmit queue 0 timed out

It seems pch_gbe has a racy tx path (races with TX completion path)

Remove tx_queue_lock lock since it has no purpose, we must use tx_lock
instead.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Andy Cress <redacted>
Tested-by: Andy Cress <redacted>
---

patch rebased on net tree.

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h      |    2 
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |   25 ++++------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index dd14915..ba78174 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h

@@ -584,7 +584,6 @@ struct pch_gbe_hw_stats {
 /**
  * struct pch_gbe_adapter - board specific private data structure
  * @stats_lock:	Spinlock structure for status
- * @tx_queue_lock:	Spinlock structure for transmit
  * @ethtool_lock:	Spinlock structure for ethtool
  * @irq_sem:		Semaphore for interrupt
  * @netdev:		Pointer of network device structure

@@ -609,7 +608,6 @@ struct pch_gbe_hw_stats {
 
 struct pch_gbe_adapter {
 	spinlock_t stats_lock;
-	spinlock_t tx_queue_lock;
 	spinlock_t ethtool_lock;
 	atomic_t irq_sem;
 	struct net_device *netdev;

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 8035e5f..1e38d50 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c

@@ -640,14 +640,11 @@ static void pch_gbe_mac_set_pause_packet(struct pch_gbe_hw *hw)
  */
 static int pch_gbe_alloc_queues(struct pch_gbe_adapter *adapter)
 {
-	int size;
-
-	size = (int)sizeof(struct pch_gbe_tx_ring);
-	adapter->tx_ring = kzalloc(size, GFP_KERNEL);
+	adapter->tx_ring = kzalloc(sizeof(*adapter->tx_ring), GFP_KERNEL);
 	if (!adapter->tx_ring)
 		return -ENOMEM;
-	size = (int)sizeof(struct pch_gbe_rx_ring);
-	adapter->rx_ring = kzalloc(size, GFP_KERNEL);
+
+	adapter->rx_ring = kzalloc(sizeof(*adapter->rx_ring), GFP_KERNEL);
 	if (!adapter->rx_ring) {
 		kfree(adapter->tx_ring);
 		return -ENOMEM;

@@ -1162,7 +1159,6 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	struct sk_buff *tmp_skb;
 	unsigned int frame_ctrl;
 	unsigned int ring_num;
-	unsigned long flags;
 
 	/*-- Set frame control --*/
 	frame_ctrl = 0;

@@ -1211,14 +1207,14 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 			}
 		}
 	}
-	spin_lock_irqsave(&tx_ring->tx_lock, flags);
+
 	ring_num = tx_ring->next_to_use;
 	if (unlikely((ring_num + 1) == tx_ring->count))
 		tx_ring->next_to_use = 0;
 	else
 		tx_ring->next_to_use = ring_num + 1;
 
-	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
+
 	buffer_info = &tx_ring->buffer_info[ring_num];
 	tmp_skb = buffer_info->skb;
 

@@ -1518,7 +1514,7 @@ pch_gbe_alloc_rx_buffers_pool(struct pch_gbe_adapter *adapter,
 						&rx_ring->rx_buff_pool_logic,
 						GFP_KERNEL);
 	if (!rx_ring->rx_buff_pool) {
-		pr_err("Unable to allocate memory for the receive poll buffer\n");
+		pr_err("Unable to allocate memory for the receive pool buffer\n");
 		return -ENOMEM;
 	}
 	memset(rx_ring->rx_buff_pool, 0, size);

@@ -1637,15 +1633,17 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 	pr_debug("called pch_gbe_unmap_and_free_tx_resource() %d count\n",
 		 cleaned_count);
 	/* Recover from running out of Tx resources in xmit_frame */
+	spin_lock(&tx_ring->tx_lock);
 	if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev)))) {
 		netif_wake_queue(adapter->netdev);
 		adapter->stats.tx_restart_count++;
 		pr_debug("Tx wake queue\n");
 	}
-	spin_lock(&adapter->tx_queue_lock);
+
 	tx_ring->next_to_clean = i;
-	spin_unlock(&adapter->tx_queue_lock);
+
 	pr_debug("next_to_clean : %d\n", tx_ring->next_to_clean);
+	spin_unlock(&tx_ring->tx_lock);
 	return cleaned;
 }
 

@@ -2037,7 +2035,6 @@ static int pch_gbe_sw_init(struct pch_gbe_adapter *adapter)
 		return -ENOMEM;
 	}
 	spin_lock_init(&adapter->hw.miim_lock);
-	spin_lock_init(&adapter->tx_queue_lock);
 	spin_lock_init(&adapter->stats_lock);
 	spin_lock_init(&adapter->ethtool_lock);
 	atomic_set(&adapter->irq_sem, 0);

@@ -2142,10 +2139,10 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 			 tx_ring->next_to_use, tx_ring->next_to_clean);
 		return NETDEV_TX_BUSY;
 	}
-	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 
 	/* CRC,ITAG no support */
 	pch_gbe_tx_queue(adapter, tx_ring, skb);
+	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 	return NETDEV_TX_OK;
 }
 

From: Eric Dumazet <redacted>
Date: Mon, 14 May 2012 21:26:06 +0200

From: Eric Dumazet <edumazet@google.com>

Andy reported pch_gbe triggered "NETDEV WATCHDOG" errors.

May 11 11:06:09 kontron kernel: WARNING: at net/sched/sch_generic.c:261
dev_watchdog+0x1ec/0x200() (Not tainted)
May 11 11:06:09 kontron kernel: Hardware name: N/A
May 11 11:06:09 kontron kernel: NETDEV WATCHDOG: eth0 (pch_gbe):
transmit queue 0 timed out

It seems pch_gbe has a racy tx path (races with TX completion path)

Remove tx_queue_lock lock since it has no purpose, we must use tx_lock
instead.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Andy Cress <redacted>
Tested-by: Andy Cress <redacted>

Applied.