Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock
From: Pavel Machek <hidden>
Date: 2016-12-07 20:55:47
Also in:
lkml
Hi!
The driver uses a private lock for synchronization between the xmit function and the xmit completion handler, but since the NETIF_F_LLTX flag is not set, the xmit function is also called with the xmit_lock held. On the other hand the xmit completion handler first takes the private lock and (in case that the tx queue has been stopped) the xmit_lock, leading to a reverse locking order and the potential danger of a deadlock. Fix this by removing the private lock completely and synchronizing the xmit function and completion handler solely by means of the xmit_lock. By doing this remove also the now unnecessary double check for a stopped tx queue. Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Does not seem to apply to net-next based on adc176c5472214971d77c1a61c83db9b01e9cdc7. Aha, that's the printk() changes, probably would apply to mainline.
quoted hunk ↗ jump to hunk
index caf069a..db46ec4 100644--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c@@ -1307,7 +1307,7 @@ static void stmmac_tx_clean(struct stmmac_priv *priv) unsigned int bytes_compl = 0, pkts_compl = 0; unsigned int entry = priv->dirty_tx; - spin_lock(&priv->tx_lock); + netif_tx_lock(priv->dev); priv->xstats.tx_clean++;
Should it use "netif_tx_lock_bh"? I could not reproduce the deadlock without this patch, nor can I detect anything wrong with this patch, so I guess that is: Tested-by: Pavel Machek <redacted> Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachments
- signature.asc [application/pgp-signature] 181 bytes