| Message ID | 20250121221519.392014-7-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | eth: fix calling napi_enable() in atomic context | expand |
On Tue, Jan 21, 2025 at 11:15 PM Jakub Kicinski <kuba@kernel.org> wrote: > > napi_enable() may sleep now, take netdev_lock() before rp->lock. > napi_enable() is hidden inside init_registers(). > > Note that this patch orders netdev_lock after rp->task_lock, > to avoid having to take the netdev_lock() around disable path. > > Fixes: 413f0271f396 ("net: protect NAPI enablement with netdev_lock()") > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Link: https://lore.kernel.org/dcfd56bc-de32-4b11-9e19-d8bd1543745d@stanley.mountain > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: kevinbrace@bracecomputerlab.com > CC: romieu@fr.zoreil.com > CC: kuniyu@amazon.com > --- > drivers/net/ethernet/via/via-rhine.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > Hmm. I think you forgot this chunk : diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c index 894911f3d5603c109cba7651b6b047b59ec85c9..4e59b5da063e1690fce0f210c33143a42745810 100644 --- a/drivers/net/ethernet/via/via-rhine.c +++ b/drivers/net/ethernet/via/via-rhine.c @@ -1568,7 +1568,7 @@ static void init_registers(struct net_device *dev) if (rp->quirks & rqMgmt) rhine_init_cam_filter(dev); - napi_enable(&rp->napi); + napi_enable_locked(&rp->napi); iowrite16(RHINE_EVENT & 0xffff, ioaddr + IntrEnable); > diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c > index 894911f3d560..f27157561082 100644 > --- a/drivers/net/ethernet/via/via-rhine.c > +++ b/drivers/net/ethernet/via/via-rhine.c > @@ -1696,7 +1696,10 @@ static int rhine_open(struct net_device *dev) > rhine_power_init(dev); > rhine_chip_reset(dev); > rhine_task_enable(rp); > + > + netdev_lock(dev); > init_registers(dev); > + netdev_unlock(dev); > > netif_dbg(rp, ifup, dev, "%s() Done - status %04x MII status: %04x\n", > __func__, ioread16(ioaddr + ChipCmd), > @@ -1727,6 +1730,8 @@ static void rhine_reset_task(struct work_struct *work) > > napi_disable(&rp->napi); > netif_tx_disable(dev); > + > + netdev_lock(dev); > spin_lock_bh(&rp->lock); > > /* clear all descriptors */ > @@ -1740,6 +1745,7 @@ static void rhine_reset_task(struct work_struct *work) > init_registers(dev); > > spin_unlock_bh(&rp->lock); > + netdev_unlock(dev); > > netif_trans_update(dev); /* prevent tx timeout */ > dev->stats.tx_errors++; > @@ -2541,9 +2547,12 @@ static int rhine_resume(struct device *device) > alloc_tbufs(dev); > rhine_reset_rbufs(rp); > rhine_task_enable(rp); > + > + netdev_lock(dev); > spin_lock_bh(&rp->lock); > init_registers(dev); > spin_unlock_bh(&rp->lock); > + netdev_unlock(dev); > > netif_device_attach(dev); > > -- > 2.48.1 >
diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c index 894911f3d560..f27157561082 100644 --- a/drivers/net/ethernet/via/via-rhine.c +++ b/drivers/net/ethernet/via/via-rhine.c @@ -1696,7 +1696,10 @@ static int rhine_open(struct net_device *dev) rhine_power_init(dev); rhine_chip_reset(dev); rhine_task_enable(rp); + + netdev_lock(dev); init_registers(dev); + netdev_unlock(dev); netif_dbg(rp, ifup, dev, "%s() Done - status %04x MII status: %04x\n", __func__, ioread16(ioaddr + ChipCmd), @@ -1727,6 +1730,8 @@ static void rhine_reset_task(struct work_struct *work) napi_disable(&rp->napi); netif_tx_disable(dev); + + netdev_lock(dev); spin_lock_bh(&rp->lock); /* clear all descriptors */ @@ -1740,6 +1745,7 @@ static void rhine_reset_task(struct work_struct *work) init_registers(dev); spin_unlock_bh(&rp->lock); + netdev_unlock(dev); netif_trans_update(dev); /* prevent tx timeout */ dev->stats.tx_errors++; @@ -2541,9 +2547,12 @@ static int rhine_resume(struct device *device) alloc_tbufs(dev); rhine_reset_rbufs(rp); rhine_task_enable(rp); + + netdev_lock(dev); spin_lock_bh(&rp->lock); init_registers(dev); spin_unlock_bh(&rp->lock); + netdev_unlock(dev); netif_device_attach(dev);
napi_enable() may sleep now, take netdev_lock() before rp->lock. napi_enable() is hidden inside init_registers(). Note that this patch orders netdev_lock after rp->task_lock, to avoid having to take the netdev_lock() around disable path. Fixes: 413f0271f396 ("net: protect NAPI enablement with netdev_lock()") Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Link: https://lore.kernel.org/dcfd56bc-de32-4b11-9e19-d8bd1543745d@stanley.mountain Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: kevinbrace@bracecomputerlab.com CC: romieu@fr.zoreil.com CC: kuniyu@amazon.com --- drivers/net/ethernet/via/via-rhine.c | 9 +++++++++ 1 file changed, 9 insertions(+)