From patchwork Wed May 16 19:47:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 10404771 X-Patchwork-Delegate: jgg@ziepe.ca Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B4B40601C8 for ; Wed, 16 May 2018 19:47:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9440E27F8C for ; Wed, 16 May 2018 19:47:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 88AB0286C6; Wed, 16 May 2018 19:47:40 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BC5DB27F8C for ; Wed, 16 May 2018 19:47:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750937AbeEPTrg convert rfc822-to-8bit (ORCPT ); Wed, 16 May 2018 15:47:36 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:42021 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750806AbeEPTrf (ORCPT ); Wed, 16 May 2018 15:47:35 -0400 Received: from bigeasy by Galois.linutronix.de with local (Exim 4.80) (envelope-from ) id 1fJ2Oa-0003aa-SM; Wed, 16 May 2018 21:47:32 +0200 Date: Wed, 16 May 2018 21:47:32 +0200 From: Sebastian Andrzej Siewior To: Jason Gunthorpe Cc: linux-rdma@vger.kernel.org, tglx@linutronix.de, Doug Ledford Subject: [PATCH v2] IB/ipoib: replace local_irq_disable() with proper locking Message-ID: <20180516194732.p4vk2lomdm6yisbq@linutronix.de> References: <20180504144555.5595-1-bigeasy@linutronix.de> <20180515230025.GA22822@ziepe.ca> <20180516131228.2svnt4yfttb53vyc@linutronix.de> <20180516170509.GE25661@ziepe.ca> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180516170509.GE25661@ziepe.ca> User-Agent: NeoMutt/20180323 Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In ipoib_mcast_restart_task() the netif_addr_lock() is invoked prior local_irq_save(). netif_addr_lock() should not be invoked in interrupt disabled section, only in BH disabled sections. The priv->lock is always acquired with disabled interrupts. The only place where netif_addr_lock() and priv->lock nest ist ipoib_mcast_restart_task(). Drop the local_irq_save() and acquire priv->lock with spin_lock_irq() inside the netif_addr locked section. It's safe to do so because the caller is either a worker function or __ipoib_ib_dev_flush() which are both calling with interrupts enabled (and since BH is enabled here, too so netif_addr_lock_bh() needs to be used). Cc: Doug Ledford Cc: Jason Gunthorpe Signed-off-by: Sebastian Andrzej Siewior --- On 2018-05-16 11:05:09 [-0600], Jason Gunthorpe wrote: > Send a v2 with a tighter commit message please here it is. > Jason v1…v2: tighter commit message drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index 9b3f47ae2016..6709328d90f8 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -886,7 +886,6 @@ void ipoib_mcast_restart_task(struct work_struct *work) struct netdev_hw_addr *ha; struct ipoib_mcast *mcast, *tmcast; LIST_HEAD(remove_list); - unsigned long flags; struct ib_sa_mcmember_rec rec; if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags)) @@ -898,9 +897,8 @@ void ipoib_mcast_restart_task(struct work_struct *work) ipoib_dbg_mcast(priv, "restarting multicast task\n"); - local_irq_save(flags); - netif_addr_lock(dev); - spin_lock(&priv->lock); + netif_addr_lock_bh(dev); + spin_lock_irq(&priv->lock); /* * Unfortunately, the networking core only gives us a list of all of @@ -978,9 +976,8 @@ void ipoib_mcast_restart_task(struct work_struct *work) } } - spin_unlock(&priv->lock); - netif_addr_unlock(dev); - local_irq_restore(flags); + spin_unlock_irq(&priv->lock); + netif_addr_unlock_bh(dev); ipoib_mcast_remove_list(&remove_list); @@ -988,9 +985,9 @@ void ipoib_mcast_restart_task(struct work_struct *work) * Double check that we are still up */ if (test_bit(IPOIB_FLAG_OPER_UP, &priv->flags)) { - spin_lock_irqsave(&priv->lock, flags); + spin_lock_irq(&priv->lock); __ipoib_mcast_schedule_join_thread(priv, NULL, 0); - spin_unlock_irqrestore(&priv->lock, flags); + spin_unlock_irq(&priv->lock); } }