Message ID | 1375340470-2126-1-git-send-email-kda@linux-powerpc.org |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Thursday, August 01, 2013 at 3:01 PM, Denis Kirjanov wrote: > @@ -1613,9 +1613,12 @@ sis900_start_xmit(struct sk_buff *skb, struct net_device *net_dev) > unsigned int count_dirty_tx; > > /* Don't transmit data before the complete of auto-negotiation */ > - if(!sis_priv->autong_complete){ > - netif_stop_queue(net_dev); > - return NETDEV_TX_BUSY; > + if (unlikely(!sis_priv->autong_complete)) { > + if (netif_msg_tx_err(sis_priv) && net_ratelimit()) > + netdev_dbg(net_dev, "Auto negotiation in progress\n"); > + net_dev->stats.tx_dropped++; > + dev_kfree_skb(skb); > + return NETDEV_TX_OK; > } I always insist on that tx queue timeout is caused by un-correctly phy link handler flow. The link up handler flow: Auto-negotiation done-> netif_carrier_on() The issue cannot be processed in xmit callback, which must be handled in link adjust process. Thanks, Andy -- 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: Duan Fugang-B38611 <B38611@freescale.com> Date: Thu, 1 Aug 2013 15:50:51 +0000 > On Thursday, August 01, 2013 at 3:01 PM, Denis Kirjanov wrote: > >> @@ -1613,9 +1613,12 @@ sis900_start_xmit(struct sk_buff *skb, struct net_device *net_dev) >> unsigned int count_dirty_tx; >> >> /* Don't transmit data before the complete of auto-negotiation */ >> - if(!sis_priv->autong_complete){ >> - netif_stop_queue(net_dev); >> - return NETDEV_TX_BUSY; >> + if (unlikely(!sis_priv->autong_complete)) { >> + if (netif_msg_tx_err(sis_priv) && net_ratelimit()) >> + netdev_dbg(net_dev, "Auto negotiation in progress\n"); >> + net_dev->stats.tx_dropped++; >> + dev_kfree_skb(skb); >> + return NETDEV_TX_OK; >> } > > I always insist on that tx queue timeout is caused by un-correctly phy link handler flow. > The link up handler flow: Auto-negotiation done-> netif_carrier_on() > > The issue cannot be processed in xmit callback, which must be handled in link adjust process. I completely agree, if sis_priv->autonet_complete is false this driver _MUST_ keep netif_carrier_off() until it becomes true. This patch is not the correct fix for this problem, please fix it correctly. -- 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
diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c index eb4aea3..f1fe02b 100644 --- a/drivers/net/ethernet/sis/sis900.c +++ b/drivers/net/ethernet/sis/sis900.c @@ -1613,9 +1613,12 @@ sis900_start_xmit(struct sk_buff *skb, struct net_device *net_dev) unsigned int count_dirty_tx; /* Don't transmit data before the complete of auto-negotiation */ - if(!sis_priv->autong_complete){ - netif_stop_queue(net_dev); - return NETDEV_TX_BUSY; + if (unlikely(!sis_priv->autong_complete)) { + if (netif_msg_tx_err(sis_priv) && net_ratelimit()) + netdev_dbg(net_dev, "Auto negotiation in progress\n"); + net_dev->stats.tx_dropped++; + dev_kfree_skb(skb); + return NETDEV_TX_OK; } spin_lock_irqsave(&sis_priv->lock, flags);
[ 198.720048] ------------[ cut here ]------------ [ 198.720108] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:255 dev_watchdog+0x229/0x240() [ 198.720118] NETDEV WATCHDOG: eth0 (sis900): transmit queue 0 timed out [ 198.720125] Modules linked in: bridge stp llc dmfe sundance 3c59x sis900 mii [ 198.720159] CPU: 0 PID: 0 Comm: swapper Not tainted 3.11.0-rc3+ #12 [ 198.720167] Hardware name: System Manufacturer System Name/TUSI-M, BIOS ASUS TUSI-M ACPI BIOS Revision 1013 Beta 001 12/14/2001 [ 198.720175] 000000ff c13fa6b9 c169ddcc c12208d6 c169ddf8 c1031e4d c1664a84 c169de24 [ 198.720197] 00000000 c165f5ea 000000ff c13fa6b9 00000001 000000ff c1664a84 c169de10 [ 198.720217] c1031f13 00000009 c169de08 c1664a84 c169de24 c169de50 c13fa6b9 c165f5ea [ 198.720240] Call Trace: [ 198.720257] [<c13fa6b9>] ? dev_watchdog+0x229/0x240 [ 198.720274] [<c12208d6>] dump_stack+0x16/0x20 [ 198.720306] [<c1031e4d>] warn_slowpath_common+0x7d/0xa0 [ 198.720318] [<c13fa6b9>] ? dev_watchdog+0x229/0x240 [ 198.720330] [<c1031f13>] warn_slowpath_fmt+0x33/0x40 [ 198.720342] [<c13fa6b9>] dev_watchdog+0x229/0x240 [ 198.720357] [<c103f158>] call_timer_fn+0x78/0x150 [ 198.720369] [<c103f0e0>] ? internal_add_timer+0x40/0x40 [ 198.720381] [<c13fa490>] ? dev_init_scheduler+0xa0/0xa0 [ 198.720392] [<c103f33f>] run_timer_softirq+0x10f/0x200 [ 198.720412] [<c103954f>] ? __do_softirq+0x6f/0x210 [ 198.720424] [<c13fa490>] ? dev_init_scheduler+0xa0/0xa0 [ 198.720435] [<c1039598>] __do_softirq+0xb8/0x210 [ 198.720467] [<c14b54d2>] ? _raw_spin_unlock+0x22/0x30 [ 198.720484] [<c1003245>] ? handle_irq+0x25/0xd0 [ 198.720496] [<c1039c0c>] irq_exit+0x9c/0xb0 [ 198.720508] [<c14bc9d7>] do_IRQ+0x47/0x94 [ 198.720534] [<c1056078>] ? hrtimer_start+0x28/0x30 [ 198.720564] [<c14bc8b1>] common_interrupt+0x31/0x38 [ 198.720589] [<c1008692>] ? default_idle+0x22/0xa0 [ 198.720600] [<c10083c7>] arch_cpu_idle+0x17/0x30 [ 198.720631] [<c106d23d>] cpu_startup_entry+0xcd/0x180 [ 198.720643] [<c14ae30a>] rest_init+0xaa/0xb0 [ 198.720654] [<c14ae260>] ? reciprocal_value+0x50/0x50 [ 198.720668] [<c17044e0>] ? repair_env_string+0x60/0x60 [ 198.720679] [<c1704bda>] start_kernel+0x29a/0x350 [ 198.720690] [<c17044e0>] ? repair_env_string+0x60/0x60 [ 198.720721] [<c1704269>] i386_start_kernel+0x39/0xa0 [ 198.720729] ---[ end trace 81e0a6266f5c73a8 ]--- [ 198.720740] eth0: Transmit timeout, status 00000204 00000000 We are trying to initiate a frame transmission even if we are not ready to do so due auto negotiation progress: timer routine checks the link status and if it's up calls netif_carrier_on() allowing upper layer to start the tx queue Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org> --- v1->v2: use netdev_dbg() instead of printk() --- drivers/net/ethernet/sis/sis900.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)