Re: [PATCH v5 11/17] net: sgi: ioc3-eth: no need to stop queue set_multicast_list
From: Jakub Kicinski <hidden>
Date: 2019-08-20 00:05:05
Also in:
linux-doc, linux-mips, linux-rtc, linux-serial, lkml, netdev
On Mon, 19 Aug 2019 18:31:34 +0200, Thomas Bogendoerfer wrote:
quoted hunk ↗ jump to hunk
netif_stop_queue()/netif_wake_qeue() aren't needed for changing multicast filters. Use spinlocks instead for proper protection of private struct. Signed-off-by: Thomas Bogendoerfer <redacted> --- drivers/net/ethernet/sgi/ioc3-eth.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c index d862f28887f9..7f85a3bfef14 100644 --- a/drivers/net/ethernet/sgi/ioc3-eth.c +++ b/drivers/net/ethernet/sgi/ioc3-eth.c@@ -1542,8 +1542,7 @@ static void ioc3_set_multicast_list(struct net_device *dev) struct netdev_hw_addr *ha; u64 ehar = 0; - netif_stop_queue(dev); /* Lock out others. */ - + spin_lock_irq(&ip->ioc3_lock);
What does this lock protect? 🤔 No question that stopping TX queues makes little sense, but this function is only called from ndo_set_rx_mode(), so with rtnl_lock held. I thought it may protect ip->emcr, but that one is accessed with no locking from the ioc3_timer() -> ioc3_setup_duplex() path..
quoted hunk ↗ jump to hunk
if (dev->flags & IFF_PROMISC) { /* Set promiscuous. */ ip->emcr |= EMCR_PROMISC; writel(ip->emcr, ®s->emcr);@@ -1572,7 +1571,7 @@ static void ioc3_set_multicast_list(struct net_device *dev) writel(ip->ehar_l, ®s->ehar_l); } - netif_wake_queue(dev); /* Let us get going again. */ + spin_unlock_irq(&ip->ioc3_lock); } module_pci_driver(ioc3_driver);