iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

xtensa: iss: clean up per-device locking in network driver

Browse Source 
Per-device locking in the ISS network driver is used to protect poll
timer and stats updates. Stat collection is not protected.
Remove per-device locking everywhere except the stats updates. Replace
ndo_get_stats callback with ndo_get_stats64 and use proper locking there
as well.
As a side effect this fixes possible deadlock between iss_net_close
and iss_net_timer.

Reported by: Duoming Zhou <duoming@zju.edu.cn>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
This commit is contained in:
Max Filippov 2022-04-08 01:56:53 -07:00
parent fd16501614
commit b7a861a6c3
1 changed files with 18 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
arch/xtensa/platforms/iss/network.c
View File

@ -65,7 +65,7 @@ struct iss_net_private {
struct net_device *dev; struct net_device *dev;
struct platform_device pdev; struct platform_device pdev;
struct timer_list tl; struct timer_list tl;
struct net_device_stats stats; struct rtnl_link_stats64 stats;
struct timer_list timer; struct timer_list timer;
unsigned int timer_val; unsigned int timer_val;
@ -281,7 +281,9 @@ static int iss_net_rx(struct net_device *dev)
skb = dev_alloc_skb(dev->mtu + 2 + ETH_HEADER_OTHER); skb = dev_alloc_skb(dev->mtu + 2 + ETH_HEADER_OTHER);
if (skb == NULL) { if (skb == NULL) {
spin_lock_bh(&lp->lock);
lp->stats.rx_dropped++; lp->stats.rx_dropped++;
spin_unlock_bh(&lp->lock);
return 0; return 0;
} }
@ -298,8 +300,10 @@ static int iss_net_rx(struct net_device *dev)
skb_trim(skb, pkt_len); skb_trim(skb, pkt_len);
skb->protocol = lp->tp.protocol(skb); skb->protocol = lp->tp.protocol(skb);
spin_lock_bh(&lp->lock);
lp->stats.rx_bytes += skb->len; lp->stats.rx_bytes += skb->len;
lp->stats.rx_packets++; lp->stats.rx_packets++;
spin_unlock_bh(&lp->lock);
netif_rx(skb); netif_rx(skb);
return pkt_len; return pkt_len;
} }
@ -314,13 +318,9 @@ static int iss_net_poll(struct iss_net_private *lp)
if (!netif_running(lp->dev)) if (!netif_running(lp->dev))
return 0; return 0;
spin_lock(&lp->lock);
while ((err = iss_net_rx(lp->dev)) > 0) while ((err = iss_net_rx(lp->dev)) > 0)
ret++; ret++;
spin_unlock(&lp->lock);
if (err < 0) { if (err < 0) {
pr_err("Device '%s' read returned %d, shutting it down\n", pr_err("Device '%s' read returned %d, shutting it down\n",
lp->dev->name, err); lp->dev->name, err);
@ -338,9 +338,7 @@ static void iss_net_timer(struct timer_list *t)
struct iss_net_private *lp = from_timer(lp, t, timer); struct iss_net_private *lp = from_timer(lp, t, timer);
iss_net_poll(lp); iss_net_poll(lp);
spin_lock(&lp->lock);
mod_timer(&lp->timer, jiffies + lp->timer_val); mod_timer(&lp->timer, jiffies + lp->timer_val);
spin_unlock(&lp->lock);
} }
@ -349,11 +347,9 @@ static int iss_net_open(struct net_device *dev)
struct iss_net_private *lp = netdev_priv(dev); struct iss_net_private *lp = netdev_priv(dev);
int err; int err;
spin_lock_bh(&lp->lock);
err = lp->tp.open(lp); err = lp->tp.open(lp);
if (err < 0) if (err < 0)
goto out; return err;
netif_start_queue(dev); netif_start_queue(dev);
@ -368,22 +364,17 @@ static int iss_net_open(struct net_device *dev)
lp->timer_val = ISS_NET_TIMER_VALUE; lp->timer_val = ISS_NET_TIMER_VALUE;
mod_timer(&lp->timer, jiffies + lp->timer_val); mod_timer(&lp->timer, jiffies + lp->timer_val);
out:
spin_unlock_bh(&lp->lock);
return err; return err;
} }
static int iss_net_close(struct net_device *dev) static int iss_net_close(struct net_device *dev)
{ {
struct iss_net_private *lp = netdev_priv(dev); struct iss_net_private *lp = netdev_priv(dev);
netif_stop_queue(dev); netif_stop_queue(dev);
spin_lock_bh(&lp->lock);
del_timer_sync(&lp->timer); del_timer_sync(&lp->timer);
lp->tp.close(lp); lp->tp.close(lp);
spin_unlock_bh(&lp->lock);
return 0; return 0;
} }
@ -393,13 +384,14 @@ static int iss_net_start_xmit(struct sk_buff *skb, struct net_device *dev)
int len; int len;
netif_stop_queue(dev); netif_stop_queue(dev);
spin_lock_bh(&lp->lock);
len = lp->tp.write(lp, &skb); len = lp->tp.write(lp, &skb);
if (len == skb->len) { if (len == skb->len) {
spin_lock_bh(&lp->lock);
lp->stats.tx_packets++; lp->stats.tx_packets++;
lp->stats.tx_bytes += skb->len; lp->stats.tx_bytes += skb->len;
spin_unlock_bh(&lp->lock);
netif_trans_update(dev); netif_trans_update(dev);
netif_start_queue(dev); netif_start_queue(dev);
@ -408,24 +400,29 @@ static int iss_net_start_xmit(struct sk_buff *skb, struct net_device *dev)
} else if (len == 0) { } else if (len == 0) {
netif_start_queue(dev); netif_start_queue(dev);
spin_lock_bh(&lp->lock);
lp->stats.tx_dropped++; lp->stats.tx_dropped++;
spin_unlock_bh(&lp->lock);
} else { } else {
netif_start_queue(dev); netif_start_queue(dev);
pr_err("%s: %s failed(%d)\n", dev->name, __func__, len); pr_err("%s: %s failed(%d)\n", dev->name, __func__, len);
} }
spin_unlock_bh(&lp->lock);
dev_kfree_skb(skb); dev_kfree_skb(skb);
return NETDEV_TX_OK; return NETDEV_TX_OK;
} }
static struct net_device_stats *iss_net_get_stats(struct net_device *dev) static void iss_net_get_stats64(struct net_device *dev,
struct rtnl_link_stats64 *stats)
{ {
struct iss_net_private *lp = netdev_priv(dev); struct iss_net_private *lp = netdev_priv(dev);
return &lp->stats;
spin_lock_bh(&lp->lock);
*stats = lp->stats;
spin_unlock_bh(&lp->lock);
} }
static void iss_net_set_multicast_list(struct net_device *dev) static void iss_net_set_multicast_list(struct net_device *dev)
@ -457,7 +454,7 @@ static int driver_registered;
static const struct net_device_ops iss_netdev_ops = { static const struct net_device_ops iss_netdev_ops = {
.ndo_open = iss_net_open, .ndo_open = iss_net_open,
.ndo_stop = iss_net_close, .ndo_stop = iss_net_close,
.ndo_get_stats = iss_net_get_stats, .ndo_get_stats64 = iss_net_get_stats64,
.ndo_start_xmit = iss_net_start_xmit, .ndo_start_xmit = iss_net_start_xmit,
.ndo_validate_addr = eth_validate_addr, .ndo_validate_addr = eth_validate_addr,
.ndo_change_mtu = iss_net_change_mtu, .ndo_change_mtu = iss_net_change_mtu,