From patchwork Fri May 4 14:45:54 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 10380817 X-Patchwork-Delegate: dledford@redhat.com 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 0830560353 for ; Fri, 4 May 2018 14:46:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E4427294B9 for ; Fri, 4 May 2018 14:46:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D7542294BA; Fri, 4 May 2018 14:46:00 +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 497B1294B8 for ; Fri, 4 May 2018 14:46:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751189AbeEDOp7 (ORCPT ); Fri, 4 May 2018 10:45:59 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:41028 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750707AbeEDOp7 (ORCPT ); Fri, 4 May 2018 10:45:59 -0400 Received: from localhost ([127.0.0.1] helo=bazinga.breakpoint.cc) by Galois.linutronix.de with esmtp (Exim 4.80) (envelope-from ) id 1fEby8-00060J-SV; Fri, 04 May 2018 16:45:57 +0200 From: Sebastian Andrzej Siewior To: linux-rdma@vger.kernel.org Cc: tglx@linutronix.de, Sebastian Andrzej Siewior , Doug Ledford , Jason Gunthorpe Subject: [PATCH] IB/ipoib: replace local_irq_disable() with proper locking Date: Fri, 4 May 2018 16:45:54 +0200 Message-Id: <20180504144555.5595-1-bigeasy@linutronix.de> X-Mailer: git-send-email 2.17.0 MIME-Version: 1.0 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 Commit 78bfe0b5b67f ("IPoIB: Take dev->xmit_lock around mc_list accesses") introduced xmit_lock lock in ipoib_mcast_restart_task() and commit 932ff279a43a ("[NET]: Add netif_tx_lock") preserved the locking order while dev->xmit_lock has been replaced with a helper. The netif_tx_lock should not be acquired with disabled interrupts because it is meant to be a BH disabling lock. The priv->lock is always acquired with interrupts disabled. The only place where netif_addr_lock() and priv->lock nest ist ipoib_mcast_restart_task(). By reversing the lock order and taking netif_addr lock with bottom halfs disabled it is possible to get rid of the local_irq_save() completely. This requires to take 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. Cc: Doug Ledford Cc: Jason Gunthorpe Signed-off-by: Sebastian Andrzej Siewior Reviewed-by: Erez Shitrit --- 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); } }