diff mbox

[net,2/2] 8139cp: reset BQL when ring tx ring cleared

Message ID 20130520095443.2d18b2cb@nehalam.linuxnetplumber.net
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Stephen Hemminger May 20, 2013, 4:54 p.m. UTC 
This patch cures transmit timeout's with DHCP observed
while running under KVM. When the transmit ring is cleaned out,
the Byte Queue Limit values need to be reset.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

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

Comments

David Miller May 20, 2013, 9:03 p.m. UTC | #1 
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 20 May 2013 09:54:43 -0700

> This patch cures transmit timeout's with DHCP observed
> while running under KVM. When the transmit ring is cleaned out,
> the Byte Queue Limit values need to be reset.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied.
--
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
Francois Romieu May 20, 2013, 9:37 p.m. UTC | #2 
Stephen Hemminger <stephen@networkplumber.org> :
> This patch cures transmit timeout's with DHCP observed
> while running under KVM. When the transmit ring is cleaned out,
> the Byte Queue Limit values need to be reset.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> --- a/drivers/net/ethernet/realtek/8139cp.c	2013-05-20 09:29:49.689900862 -0700
> +++ b/drivers/net/ethernet/realtek/8139cp.c	2013-05-20 09:29:50.857880563 -0700
> @@ -1141,6 +1141,7 @@ static void cp_clean_rings (struct cp_pr
>  			cp->dev->stats.tx_dropped++;
>  		}
>  	}
> +	netdev_reset_queue(cp->dev);
>  
>  	memset(cp->rx_ring, 0, sizeof(struct cp_desc) * CP_RX_RING_SIZE);
>  	memset(cp->tx_ring, 0, sizeof(struct cp_desc) * CP_TX_RING_SIZE);

It's a good catch but I believe that you are not telling us the whole
story. :o)

Where does cp_clean_rings get called ?

- error path of cp_refill_rx
  - cp_init_rings
    - cp_alloc_rings
      - cp_open
        - in cp_change_mtu, after cp_close, whence cp_stop_hw
    - cp_tx_timeout
      -> after cp_stop_hw
- cp_free_rings
  - error path of cp_open
    -> after cp_stop_hw
  - cp_close
    -> after cp_stop_hw
- cp_tx_timeout
  -> after cp_stop_hw

cp_stop_hw includes netdev_reset_queue.

You have imho exhibited a start_xmit after cp_stop_hw race - not sure if
it happens in cp_tx_timeout or cp_change_mtu. Reverting the analysis above,
I have not found a place where cp_stop_hw could be called without being
followed by a cp_clean_rings. The netdev_reset_queue in cp_stop_hw, now
useless, should thus be removed.

Does it make sense ?
Stephen Hemminger May 21, 2013, 12:27 a.m. UTC | #3 
On Mon, 20 May 2013 23:37:28 +0200
Francois Romieu <romieu@fr.zoreil.com> wrote:

> cp_stop_hw includes netdev_reset_queue.
> 
> You have imho exhibited a start_xmit after cp_stop_hw race - not sure if
> it happens in cp_tx_timeout or cp_change_mtu. Reverting the analysis above,
> I have not found a place where cp_stop_hw could be called without being
> followed by a cp_clean_rings. The netdev_reset_queue in cp_stop_hw, now
> useless, should thus be removed.
> 
> Does it make sense ?

Your right, you could probably remove it.

It doesn't solve the problem, still seeing transmit timeouts.
Looks like what happens with DHCP is something else.


 zebra[2210]: interface eth0 index 2 changed <UP,BROADCAST,RUNNING,MULTICAST>.

[   23.960059] ------------[ cut here ]------------
[   23.960107] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0x243/0x250()
[   23.960110] Hardware name: Bochs 
[   23.960114] Modules linked in: 9p 9pnet fscache ipv6 tun mperf cpufreq_stats cpufreq_ondemand freq_table cpufreq_userspace cpufreq_conservative virtio_uio(O) cpufreq_powersave igb_uio(O) uio cirrus ttm drm_kms_helper drm sysimgblt sysfillrect virtio_balloon i2c_piix4 i2c_core hid_generic syscopyarea psmouse evdev serio_raw microcode pcspkr intel_agp intel_gtt agpgart processor thermal_sys button vfat fat usb_storage ohci_hcd squashfs loop overlayfs raid10 raid456 async_pq async_xor xor async_memcpy async_raid6_recov usbhid hid raid6_pq async_tx raid1 raid0 multipath linear md_mod pata_acpi ata_generic virtio_blk virtio_net 8139cp ata_piix 8139too mii virtio_pci virtio_ring virtio floppy
[   23.960177] Pid: 0, comm: swapper/3 Tainted: G           O 3.9.3-1-amd64-vyatta #1
[   23.960179] Call Trace:
[   23.960181]  <IRQ>  [<ffffffff8105479b>] ? warn_slowpath_common+0x7b/0xc0
[   23.960212]  [<ffffffff81054895>] ? warn_slowpath_fmt+0x45/0x50
[   23.960228]  [<ffffffff814dfb45>] ? _raw_spin_lock+0x5/0x10
[   23.960232]  [<ffffffff8143dde3>] ? dev_watchdog+0x243/0x250
[   23.960245]  [<ffffffff810704c0>] ? __queue_work+0x2b0/0x2b0
[   23.960253]  [<ffffffff8143dba0>] ? __netdev_watchdog_up+0x70/0x70
[   23.960257]  [<ffffffff8143dba0>] ? __netdev_watchdog_up+0x70/0x70
[   23.960262]  [<ffffffff81064773>] ? call_timer_fn+0x43/0x130
[   23.960265]  [<ffffffff81064c27>] ? run_timer_softirq+0x237/0x280 
[   23.960269]  [<ffffffff8107b200>] ? update_rmtp+0x80/0x80
[   23.960272]  [<ffffffff814dfb45>] ? _raw_spin_lock+0x5/0x10
[   23.960276]  [<ffffffff8105ca41>] ? __do_softirq+0xe1/0x270
[   23.960280]  [<ffffffff8105ccd5>] ? irq_exit+0xb5/0xc0
[   23.960291]  [<ffffffff81039fd8>] ? smp_apic_timer_interrupt+0x68/0xa0
[   23.960302]  [<ffffffff814e151d>] ? apic_timer_interrupt+0x6d/0x80
[   23.960304]  <EOI>  [<ffffffff81042152>] ? native_safe_halt+0x2/0x10
[   23.960318]  [<ffffffff8101e1ef>] ? default_idle+0x3f/0xf0
[   23.960323]  [<ffffffff8101db48>] ? cpu_idle+0x88/0xd0
[   23.960330]  [<ffffffff814d547a>] ? start_secondary+0x211/0x216
[   23.960333] ---[ end trace a519f7f509aceb9a ]--- 
[   23.960407] 8139cp 0000:00:03.0 eth0: Transmit timeout, status  d   3b    4 80ff
[   35.960137] 8139cp 0000:00:03.0 eth0: Transmit timeout, status  d   3b    5 80ff
[   47.960108] 8139cp 0000:00:03.0 eth0: Transmit timeout, status  d   3b    5 80ff
[   59.960117] 8139cp 0000:00:03.0 eth0: Transmit timeout, status  d   3b    5 80ff
[   71.960115] 8139cp 0000:00:03.0 eth0: Transmit timeout, status  d   3b    5 80ff
[   83.960117] 8139cp 0000:00:03.0 eth0: Transmit timeout, status  d   3b    5 80ff
[   95.960131] 8139cp 0000:00:03.0 eth0: Transmit timeout, status  d   3b    5 80ff
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
~                                                                                                                                                    
"messages" 35 lines, 2958 characters written

--
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
David Woodhouse Sept. 14, 2015, 12:05 p.m. UTC | #4 
On Mon, 2013-05-20 at 17:27 -0700, Stephen Hemminger wrote:
> On Mon, 20 May 2013 23:37:28 +0200
> Francois Romieu <romieu@fr.zoreil.com> wrote:
> 
> > cp_stop_hw includes netdev_reset_queue.
> > 
> > You have imho exhibited a start_xmit after cp_stop_hw race - not sure if
> > it happens in cp_tx_timeout or cp_change_mtu. Reverting the analysis above,
> > I have not found a place where cp_stop_hw could be called without being
> > followed by a cp_clean_rings. The netdev_reset_queue in cp_stop_hw, now
> > useless, should thus be removed.
> > 
> > Does it make sense ?
> 
> Your right, you could probably remove it.
> 
> It doesn't solve the problem, still seeing transmit timeouts.
> Looks like what happens with DHCP is something else.

Did you ever work this out? I'm seeing something similar on the inward
-facing interface on my home router under high load — and it doesn't
automatically recover.



[308309.340644] ------------[ cut here ]------------
[308309.345379] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0x103/0x190()
[308309.352789] Hardware name: Geos
[308309.356020] NETDEV WATCHDOG: eth1 (8139cp): transmit queue 0 timed out
[308309.362733] Modules linked in: sch_fq_codel sch_teql gpio_keys_polled leds_gpio geodewdt solos_pci ledtrig_heartbeat gpio_cs5535 cs5535_clockevt 8139cp ip6t_REJECT ip6t_rt ip6t_hbh ip6t_mh ip6t_ipv6header ip6t_frag ip6t_eui64 ip6t_ah ip6table_raw ip6table_mangle ip6table_filter ip6_tables nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_irc nf_conntrack_irc nf_nat_ftp nf_conntrack_ftp xt_HL xt_hl xt_ecn ipt_ECN xt_CLASSIFY xt_time xt_tcpmss xt_statistic xt_mark xt_length xt_DSCP xt_dscp cs5535_mfgpt cs5535_mfd mfd_core ipt_MASQUERADE nf_nat xt_recent xt_helper xt_connmark xt_connbytes pptp l2tp_ppp pppoe xt_conntrack xt_CT iptable_raw xt_state nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack pppox pppoatm ipt_REJECT xt_TCPMSS xt_comment xt_multiport xt_mac xt_limit iptable_mangle iptable_filter ip_tables xt_tcpudp x_tables nsc_gpio ip_gre gre sit l2tp_netlink l2tp_core ppp_mppe tunnel4 tun ppp_async ppp_generic slhc br2684 atm crc_ccitt ipv6 input_polldev msr input_core sha1_generic geode_aes ecb arc4 aes_i586 ohci_hcd ehci_hcd usbcore usb_common
[308309.457239] Pid: 0, comm: swapper Not tainted 3.7.1 #1
[308309.462463] Call Trace:
[308309.465020]  [<c10272e7>] ? warn_slowpath_common+0x87/0xb0
[308309.470691]  [<c11e3d43>] ? dev_watchdog+0x103/0x190
[308309.475755]  [<c10273a3>] ? warn_slowpath_fmt+0x33/0x40
[308309.481159]  [<c11e3d43>] ? dev_watchdog+0x103/0x190
[308309.486244]  [<c11e3c40>] ? pfifo_fast_dequeue+0xd0/0xd0
[308309.491751]  [<c1030f3c>] ? call_timer_fn.isra.42+0x1c/0x80
[308309.497422]  [<c11d1394>] ? process_backlog+0x54/0xe0
[308309.502674]  [<c10310ca>] ? run_timer_softirq+0x12a/0x160
[308309.508169]  [<c11e3c40>] ? pfifo_fast_dequeue+0xd0/0xd0
[308309.513697]  [<c102cfcd>] ? __do_softirq+0x6d/0x110
[308309.518675]  [<c102cf60>] ? __tasklet_schedule+0x40/0x40
[308309.524178]  <IRQ>  [<c102d121>] ? irq_exit+0x31/0x60
[308309.529359]  [<c1003c7d>] ? do_IRQ+0x8d/0xb0
[308309.533723]  [<c1003c7d>] ? do_IRQ+0x8d/0xb0
[308309.538201]  [<c1254fe9>] ? common_interrupt+0x29/0x2e
[308309.543440]  [<c1050000>] ? rt_mutex_adjust_prio_chain+0x180/0x280
[308309.549829]  [<c10085c4>] ? default_idle+0x14/0x30
[308309.554719]  [<c1008e1f>] ? cpu_idle+0x2f/0x50
[308309.559259]  [<c131e878>] ? start_kernel+0x286/0x28b
[308309.564414]  [<c131e440>] ? repair_env_string+0x4d/0x4d
[308309.569729] ---[ end trace 2e18cc211cee6089 ]---
[308309.574551] 8139cp 0000:00:0b.0 eth1: Transmit timeout, status  c   2b    0 80ff
diff mbox

Patch

--- a/drivers/net/ethernet/realtek/8139cp.c	2013-05-20 09:29:49.689900862 -0700
+++ b/drivers/net/ethernet/realtek/8139cp.c	2013-05-20 09:29:50.857880563 -0700
@@ -1141,6 +1141,7 @@  static void cp_clean_rings (struct cp_pr
 			cp->dev->stats.tx_dropped++;
 		}
 	}
+	netdev_reset_queue(cp->dev);
 
 	memset(cp->rx_ring, 0, sizeof(struct cp_desc) * CP_RX_RING_SIZE);
 	memset(cp->tx_ring, 0, sizeof(struct cp_desc) * CP_TX_RING_SIZE);