netdev - Re: [PATCH 3/5] net: mvneta: do not schedule in mvneta_tx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1389548333.3720.73.camel@deadeye.wl.decadent.org.uk>
Date:	Sun, 12 Jan 2014 17:38:53 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Willy Tarreau <w@....eu>
Cc:	davem@...emloft.net, netdev@...r.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Gregory CLEMENT <gregory.clement@...e-electrons.com>
Subject: Re: [PATCH 3/5] net: mvneta: do not schedule in mvneta_tx_timeout

[Putting another hat on]

On Sun, 2014-01-12 at 17:55 +0100, Willy Tarreau wrote:
> Hi Ben,
> 
> On Sun, Jan 12, 2014 at 04:49:51PM +0000, Ben Hutchings wrote:
> (...)
> > > So for now, let's simply ignore these timeouts generally caused by bugs
> > > only.
> > 
> > No, don't ignore them.  Schedule a work item to reset the device.  (And
> > remember to cancel it when stopping the device.)
> 
> OK I can try to do that. Could you recommend me one driver which does this
> successfully so that I can see exactly what needs to be taken care of ?

sfc does it, though the reset logic there is more complicated than you
would need.

I think this will DTRT, but it's compile-tested only.  I have been given
an OpenBlocks AX3 but haven't set it up yet.

Ben.

---
mvneta: Defer restart from TX watchdog handler to work item

A restart requires sleeping, but the watchdog handler runs in atomic
context.

The work item can race with down-ing of the interface, so take the RTNL
lock and do nothing if the interface is already down.

Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index d5f0d72..a91dcc2 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -234,6 +234,7 @@ struct mvneta_port {
 	struct mvneta_tx_queue *txqs;
 	struct timer_list tx_done_timer;
 	struct net_device *dev;
+	struct work_struct restart_work;
 
 	u32 cause_rx_tx;
 	struct napi_struct napi;
@@ -2231,8 +2232,22 @@ static void mvneta_tx_timeout(struct net_device *dev)
 	struct mvneta_port *pp = netdev_priv(dev);
 
 	netdev_info(dev, "tx timeout\n");
-	mvneta_stop_dev(pp);
-	mvneta_start_dev(pp);
+
+	/* defer mvneta_restart(), as we're in atomic context here */
+	schedule_work(&pp->restart_work);
+}
+
+static void mvneta_restart(struct work_struct *work)
+{
+	struct mvneta_port *pp =
+		container_of(work, struct mvneta_port, restart_work);
+
+	rtnl_lock();
+	if (netif_running(pp->dev)) {
+		mvneta_stop_dev(pp);
+		mvneta_start_dev(pp);
+	}
+	rtnl_unlock();
 }
 
 /* Return positive if MTU is valid */
@@ -2792,6 +2807,8 @@ static int mvneta_probe(struct platform_device *pdev)
 
 	pp = netdev_priv(dev);
 
+	INIT_WORK(&pp->restart_work, mvneta_restart);
+
 	u64_stats_init(&pp->tx_stats.syncp);
 	u64_stats_init(&pp->rx_stats.syncp);
 
@@ -2890,6 +2907,7 @@ static int mvneta_remove(struct platform_device *pdev)
 	struct mvneta_port *pp = netdev_priv(dev);
 
 	unregister_netdev(dev);
+	cancel_work_sync(&pp->restart_work);
 	mvneta_deinit(pp);
 	clk_disable_unprepare(pp->clk);
 	iounmap(pp->base);

-- 
Ben Hutchings
Quantity is no substitute for quality, but it's the only one we've got.

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)