diff options
author | David S. Miller <davem@davemloft.net> | 2020-01-06 13:54:55 -0800 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2020-01-06 13:54:55 -0800 |
commit | 50d31037e430791a517d74631a28dc5e218242d2 (patch) | |
tree | 740867ad738275f4cbf4068f50e4aea0b3b572bd /drivers | |
parent | 17aa23ee72342efdc3b214f582399dd22478887f (diff) | |
parent | 4ac0ac847f05bc49efb6ea9e8770f1dfec31f540 (diff) |
Merge branch 'ethtool-allow-nesting-of-begin-and-complete-callbacks'
Michal Kubecek says:
====================
ethtool: allow nesting of begin() and complete() callbacks
The ethtool ioctl interface used to guarantee that ethtool_ops callbacks
were always called in a block between calls to ->begin() and ->complete()
(if these are defined) and that this whole block was executed with RTNL
lock held:
rtnl_lock();
ops->begin();
/* other ethtool_ops calls */
ops->complete();
rtnl_unlock();
This prevented any nesting or crossing of the begin-complete blocks.
However, this is no longer guaranteed even for ioctl interface as at least
ethtool_phys_id() releases RTNL lock while waiting for a timer. With the
introduction of netlink ethtool interface, the begin-complete pairs are
naturally nested e.g. when a request triggers a netlink notification.
Fortunately, only minority of networking drivers implements begin() and
complete() callbacks and most of those that do, fall into three groups:
- wrappers for pm_runtime_get_sync() and pm_runtime_put()
- wrappers for clk_prepare_enable() and clk_disable_unprepare()
- begin() checks netif_running() (fails if false), no complete()
First two have their own refcounting, third is safe w.r.t. nesting of the
blocks.
Only three in-tree networking drivers need an update to deal with nesting
of begin() and complete() calls: via-velocity and epic100 perform resume
and suspend on their own and wil6210 completely serializes the calls using
its own mutex (which would lead to a deadlock if a request request
triggered a netlink notification). The series addresses these problems.
changes between v1 and v2:
- fix inverted condition in epic100 ethtool_begin() (thanks to Andrew
Lunn)
====================
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/net/ethernet/smsc/epic100.c | 7 | ||||
-rw-r--r-- | drivers/net/ethernet/via/via-velocity.c | 14 | ||||
-rw-r--r-- | drivers/net/ethernet/via/via-velocity.h | 1 | ||||
-rw-r--r-- | drivers/net/wireless/ath/wil6210/ethtool.c | 43 |
4 files changed, 32 insertions, 33 deletions
diff --git a/drivers/net/ethernet/smsc/epic100.c b/drivers/net/ethernet/smsc/epic100.c index 912760e8514c..61ddee0c2a2e 100644 --- a/drivers/net/ethernet/smsc/epic100.c +++ b/drivers/net/ethernet/smsc/epic100.c @@ -280,6 +280,7 @@ struct epic_private { signed char phys[4]; /* MII device addresses. */ u16 advertising; /* NWay media advertisement */ int mii_phy_cnt; + u32 ethtool_ops_nesting; struct mii_if_info mii; unsigned int tx_full:1; /* The Tx queue is full. */ unsigned int default_port:4; /* Last dev->if_port value. */ @@ -1435,8 +1436,10 @@ static int ethtool_begin(struct net_device *dev) struct epic_private *ep = netdev_priv(dev); void __iomem *ioaddr = ep->ioaddr; + if (ep->ethtool_ops_nesting == U32_MAX) + return -EBUSY; /* power-up, if interface is down */ - if (!netif_running(dev)) { + if (!ep->ethtool_ops_nesting++ && !netif_running(dev)) { ew32(GENCTL, 0x0200); ew32(NVCTL, (er32(NVCTL) & ~0x003c) | 0x4800); } @@ -1449,7 +1452,7 @@ static void ethtool_complete(struct net_device *dev) void __iomem *ioaddr = ep->ioaddr; /* power-down, if interface is down */ - if (!netif_running(dev)) { + if (!--ep->ethtool_ops_nesting && !netif_running(dev)) { ew32(GENCTL, 0x0008); ew32(NVCTL, (er32(NVCTL) & ~0x483c) | 0x0000); } diff --git a/drivers/net/ethernet/via/via-velocity.c b/drivers/net/ethernet/via/via-velocity.c index 346e44115c4e..4b556b74541a 100644 --- a/drivers/net/ethernet/via/via-velocity.c +++ b/drivers/net/ethernet/via/via-velocity.c @@ -3257,12 +3257,16 @@ static struct platform_driver velocity_platform_driver = { * @dev: network device * * Called before an ethtool operation. We need to make sure the - * chip is out of D3 state before we poke at it. + * chip is out of D3 state before we poke at it. In case of ethtool + * ops nesting, only wake the device up in the outermost block. */ static int velocity_ethtool_up(struct net_device *dev) { struct velocity_info *vptr = netdev_priv(dev); - if (!netif_running(dev)) + + if (vptr->ethtool_ops_nesting == U32_MAX) + return -EBUSY; + if (!vptr->ethtool_ops_nesting++ && !netif_running(dev)) velocity_set_power_state(vptr, PCI_D0); return 0; } @@ -3272,12 +3276,14 @@ static int velocity_ethtool_up(struct net_device *dev) * @dev: network device * * Called after an ethtool operation. Restore the chip back to D3 - * state if it isn't running. + * state if it isn't running. In case of ethtool ops nesting, only + * put the device to sleep in the outermost block. */ static void velocity_ethtool_down(struct net_device *dev) { struct velocity_info *vptr = netdev_priv(dev); - if (!netif_running(dev)) + + if (!--vptr->ethtool_ops_nesting && !netif_running(dev)) velocity_set_power_state(vptr, PCI_D3hot); } diff --git a/drivers/net/ethernet/via/via-velocity.h b/drivers/net/ethernet/via/via-velocity.h index cdfe7809e3c1..f196e71d2c04 100644 --- a/drivers/net/ethernet/via/via-velocity.h +++ b/drivers/net/ethernet/via/via-velocity.h @@ -1483,6 +1483,7 @@ struct velocity_info { struct velocity_context context; u32 ticks; + u32 ethtool_ops_nesting; u8 rev_id; diff --git a/drivers/net/wireless/ath/wil6210/ethtool.c b/drivers/net/wireless/ath/wil6210/ethtool.c index 912c4eaf017b..fef10886ca4a 100644 --- a/drivers/net/wireless/ath/wil6210/ethtool.c +++ b/drivers/net/wireless/ath/wil6210/ethtool.c @@ -11,26 +11,6 @@ #include "wil6210.h" -static int wil_ethtoolops_begin(struct net_device *ndev) -{ - struct wil6210_priv *wil = ndev_to_wil(ndev); - - mutex_lock(&wil->mutex); - - wil_dbg_misc(wil, "ethtoolops_begin\n"); - - return 0; -} - -static void wil_ethtoolops_complete(struct net_device *ndev) -{ - struct wil6210_priv *wil = ndev_to_wil(ndev); - - wil_dbg_misc(wil, "ethtoolops_complete\n"); - - mutex_unlock(&wil->mutex); -} - static int wil_ethtoolops_get_coalesce(struct net_device *ndev, struct ethtool_coalesce *cp) { @@ -39,11 +19,12 @@ static int wil_ethtoolops_get_coalesce(struct net_device *ndev, u32 rx_itr_en, rx_itr_val = 0; int ret; + mutex_lock(&wil->mutex); wil_dbg_misc(wil, "ethtoolops_get_coalesce\n"); ret = wil_pm_runtime_get(wil); if (ret < 0) - return ret; + goto out; tx_itr_en = wil_r(wil, RGF_DMA_ITR_TX_CNT_CTL); if (tx_itr_en & BIT_DMA_ITR_TX_CNT_CTL_EN) @@ -57,7 +38,11 @@ static int wil_ethtoolops_get_coalesce(struct net_device *ndev, cp->tx_coalesce_usecs = tx_itr_val; cp->rx_coalesce_usecs = rx_itr_val; - return 0; + ret = 0; + +out: + mutex_unlock(&wil->mutex); + return ret; } static int wil_ethtoolops_set_coalesce(struct net_device *ndev, @@ -67,12 +52,14 @@ static int wil_ethtoolops_set_coalesce(struct net_device *ndev, struct wireless_dev *wdev = ndev->ieee80211_ptr; int ret; + mutex_lock(&wil->mutex); wil_dbg_misc(wil, "ethtoolops_set_coalesce: rx %d usec, tx %d usec\n", cp->rx_coalesce_usecs, cp->tx_coalesce_usecs); if (wdev->iftype == NL80211_IFTYPE_MONITOR) { wil_dbg_misc(wil, "No IRQ coalescing in monitor mode\n"); - return -EINVAL; + ret = -EINVAL; + goto out; } /* only @rx_coalesce_usecs and @tx_coalesce_usecs supported, @@ -88,24 +75,26 @@ static int wil_ethtoolops_set_coalesce(struct net_device *ndev, ret = wil_pm_runtime_get(wil); if (ret < 0) - return ret; + goto out; wil->txrx_ops.configure_interrupt_moderation(wil); wil_pm_runtime_put(wil); + ret = 0; - return 0; +out: + mutex_unlock(&wil->mutex); + return ret; out_bad: wil_dbg_misc(wil, "Unsupported coalescing params. Raw command:\n"); print_hex_dump_debug("DBG[MISC] coal ", DUMP_PREFIX_OFFSET, 16, 4, cp, sizeof(*cp), false); + mutex_unlock(&wil->mutex); return -EINVAL; } static const struct ethtool_ops wil_ethtool_ops = { - .begin = wil_ethtoolops_begin, - .complete = wil_ethtoolops_complete, .get_drvinfo = cfg80211_get_drvinfo, .get_coalesce = wil_ethtoolops_get_coalesce, .set_coalesce = wil_ethtoolops_set_coalesce, |