From: Hector Palacios <hector.palacios@digi•com>
To: Eric Dumazet <eric.dumazet@gmail•com>
Cc: "netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"davem@davemloft•net" <davem@davemloft•net>,
"shawn.guo@linaro•org" <shawn.guo@linaro•org>,
"jgq516@gmail•com" <jgq516@gmail•com>,
"rostedt@goodmis•org" <rostedt@goodmis•org>,
"tim.sander@hbm•com" <tim.sander@hbm•com>,
"u.kleine-koenig@pengutronix•de" <u.kleine-koenig@pengutronix•de>,
"tglx@linutronix•de" <tglx@linutronix•de>,
Zeng Zhaoming <b32542@freescale•com>,
Frank Li <Frank.Li@freescale•com>
Subject: Re: [PATCH] net/fec: infinite spin on sirq-net-tx on real-time
Date: Mon, 6 Feb 2012 12:03:55 +0100 [thread overview]
Message-ID: <4F2FB39B.9010205@digi.com> (raw)
In-Reply-To: <1328522191.2220.9.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On 02/06/2012 10:56 AM, Eric Dumazet wrote:
> Le lundi 06 février 2012 à 10:33 +0100, Hector Palacios a écrit :
>> If softirqd is a real time task, an inifinite spin is hit
>> if the FEC driver tries to send a packet before the autonegotiation
>> with the PHY has completed.
>>
>> This was seen when booting the platform with DHCP on. The driver
>> sends the DHCP request before the PHY has completed autonegotiation.
>> As a consequence, the driver's dev_hard_start_xmit returns NETDEV_TX_BUSY.
>> NETDEV_TX_BUSY is part of NET_TX_MASK thus the packet is requeued (the
>> skb->next = nskb) in dev_hard_start_xmit(). And the NETDEV_TX_BUSY is
>> passed back to sch_derect_xmit() which calls dev_requeue_skb() which
>> then calls __netif_schedule(q) which will call __netif_reschedule(q)
>> which will then do raise_softirq_irqoff(NET_TX_SOFTIRQ).
>>
>> Thus, as soon as ksoftirq exits this routine, it will restart the
>> process over again. As the fec driver never finished with its
>> negotiations, the process starts over again and we never move forward.
>>
>> Newsgroup reference: linux-rt-users http://www.spinics.net/lists/linux-rt-users/msg07551.html
>>
>> Signed-off-by: Zeng Zhaoming<b32542@freescale•com>
>> Signed-off-by: Frank Li<Frank.Li@freescale•com>
>> Signed-off-by: Hector Palacios<hector.palacios@digi•com>
>> ---
>> drivers/net/ethernet/freescale/fec.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
>> index c136230..3fa7d3b 100644
>> --- a/drivers/net/ethernet/freescale/fec.c
>> +++ b/drivers/net/ethernet/freescale/fec.c
>> @@ -284,6 +284,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>>
>> if (!fep->link) {
>> /* Link is down or autonegotiation is in progress. */
>> + netif_stop_queue(dev);
>
> This seems odd.
>
> IMHO, NETDEV_TX_BUSY should be avoided as much as possible.
>
> Its part of the old driver API.
>
> If queue was stopped, you would not have to test fep->link at all in
> fast path, and Qdisc would not have to requeue a packet eventually.
>
>> return NETDEV_TX_BUSY;
>> }
>>
>> @@ -530,6 +531,7 @@ fec_stop(struct net_device *ndev)
>> udelay(10);
>> writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
>> writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
>> + fep->link = 0;
>
>
> Why not call netif_stop_queue(dev) here ?
>
>>
>> /* We have to keep ENET enabled to have MII interrupt stay working */
>> if (id_entry->driver_data& FEC_QUIRK_ENET_MAC)
>> @@ -866,6 +868,7 @@ static void fec_enet_adjust_link(struct net_device *ndev)
>> if (phy_dev->link) {
>> if (fep->full_duplex != phy_dev->duplex) {
>> fec_restart(ndev, phy_dev->duplex);
>> + netif_wake_queue(dev);
>> status_change = 1;
>> }
>> }
>
>
I'm no network driver expert so I'll leave it up to others to comment. I just forward
ported a patch I came across in Freescale's BSP which solves the problem in mainline
and in RT.
--
Héctor Palacios
next prev parent reply other threads:[~2012-02-06 11:22 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-06 9:33 [PATCH] net/fec: infinite spin on sirq-net-tx on real-time Hector Palacios
2012-02-06 9:56 ` Eric Dumazet
2012-02-06 11:03 ` Hector Palacios [this message]
2012-02-06 12:55 ` Eric Dumazet
2012-02-06 13:43 ` [PATCH]Re: " Tim Sander
2012-02-06 13:44 ` [PATCH] fec: fix tx bounce handling Eric Dumazet
2012-02-06 16:09 ` Tim Sander
2012-02-06 16:25 ` Eric Dumazet
2012-02-06 13:25 ` [PATCH] net/fec: infinite spin on sirq-net-tx on real-time Steven Rostedt
2012-02-06 13:39 ` Eric Dumazet
2012-02-07 0:48 ` [PATCH] " Tim Sander
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F2FB39B.9010205@digi.com \
--to=hector.palacios@digi$(echo .)com \
--cc=Frank.Li@freescale$(echo .)com \
--cc=b32542@freescale$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=jgq516@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=rostedt@goodmis$(echo .)org \
--cc=shawn.guo@linaro$(echo .)org \
--cc=tglx@linutronix$(echo .)de \
--cc=tim.sander@hbm$(echo .)com \
--cc=u.kleine-koenig@pengutronix$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox