From patchwork Wed Jun 21 17:18:12 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selvin Xavier X-Patchwork-Id: 9802347 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 9CBAB60329 for ; Wed, 21 Jun 2017 17:19:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 64BCF2853C for ; Wed, 21 Jun 2017 17:19:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5918A28610; Wed, 21 Jun 2017 17:19:20 +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=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID 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 B0C272853C for ; Wed, 21 Jun 2017 17:19:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751974AbdFURTS (ORCPT ); Wed, 21 Jun 2017 13:19:18 -0400 Received: from mail-wr0-f180.google.com ([209.85.128.180]:34709 "EHLO mail-wr0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751961AbdFURTS (ORCPT ); Wed, 21 Jun 2017 13:19:18 -0400 Received: by mail-wr0-f180.google.com with SMTP id 77so143697014wrb.1 for ; Wed, 21 Jun 2017 10:19:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=U/z+qPqZuR2BBu25Av3aFTtTxJ89vFg+X9dnXAHHOSk=; b=OWO8Dkc/nEhQ6HGM26wbRVpT2nVIf61pAczxFgNNP4r7jzFgGJZvRCkCOOdAS5swY0 PHhzuMZbWuyE9AUe+EsTCPJV6a4fAzpYDcGzoiBb0kuyqpUUL/a/H0rhlA7poMW/rUmd +0Jg64A/vSywe4XqAzngn8Sz/nG+WrsFfYo5M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=U/z+qPqZuR2BBu25Av3aFTtTxJ89vFg+X9dnXAHHOSk=; b=sHbJmayjIJZypG+xjWtXEqkCP9D1iva1Xg7m7VTFqcxv69c4OD95YW5xpMn9qnva9V RivCAFKvVPr++0YMmFrdNmzeNy+rhl7PJ/ML8+/2FvZzq1IrbEBs7foxf6ylGCYmlxZ2 KlZv8F6juZJxUYgG7/LdM0/ZMzjZOfAM+OvKrkbm3KHU4/YvP2ET6jQjGJu7sk/Mnd2i K5zuNSvzQl/Oj1p1Q5r286oqWvdSgFKUALCkBlx3AuNO6SkaKp9SFjFuGEWVgBWVadDZ hgZ64FmMhfCwqqSnHcY4E/R/erqGgF+v1UCPsakzA/HeAlL2k+ejE6qDsaG5ZTm1JlFM zPcw== X-Gm-Message-State: AKS2vOwuuukAzE6n50EV2IHA0XX5eovK2PQUUSuaNs588yhISNTksdZR jF/O7g+BwbX3bsyqzfs= X-Received: by 10.223.175.200 with SMTP id y8mr21415194wrd.163.1498065556395; Wed, 21 Jun 2017 10:19:16 -0700 (PDT) Received: from dhcp-10-192-206-197.iig.avagotech.net ([192.19.239.250]) by smtp.gmail.com with ESMTPSA id n64sm7446955wmd.24.2017.06.21.10.19.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 21 Jun 2017 10:19:15 -0700 (PDT) From: Selvin Xavier To: dledford@redhat.com Cc: linux-rdma@vger.kernel.org, Somnath Kotur , Sriharsha Basavapatna , Selvin Xavier Subject: [PATCH for-next 01/13] RDMA/bnxt_re: Fix race between netdev register and unregister events Date: Wed, 21 Jun 2017 10:18:12 -0700 Message-Id: <1498065504-27902-2-git-send-email-selvin.xavier@broadcom.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1498065504-27902-1-git-send-email-selvin.xavier@broadcom.com> References: <1498065504-27902-1-git-send-email-selvin.xavier@broadcom.com> 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 From: Somnath Kotur Upon receipt of the NETDEV_REGISTER event from the netdev notifier chain, the IB stack registration is spawned off to a workqueue since that also requires an rtnl lock. There could be 2 kinds of races between the NETDEV_REGISTER and the NETDEV_UNREGISTER event handling. a)The NETDEV_UNREGISTER event is received in rapid succession after the NETDEV_REGISTER event even before the work queue got a chance to run b)The NETDEV_UNREGISTER event is received while the workqueue that handles registration with the IB stack is still in progress. Handle both the races with a bit flag that is set as part of the NETDEV_REGISTER event just before the work item is queued and cleared in the workqueue after the registration with the IB stack is complete. Use a barrier to ensure the bit is cleared only after the IB stack registration code is completed. Removes the link speed query from the query_port as it causes deadlock while trying to acquire rtnl_lock. Now querying the speed from the nedev event itself. Also, added a code to wait for resources(like CQ) to be freed by the ULPs, during driver unload Signed-off-by: Somnath Kotur Signed-off-by: Sriharsha Basavapatna Signed-off-by: Selvin Xavier --- drivers/infiniband/hw/bnxt_re/bnxt_re.h | 13 +++++++---- drivers/infiniband/hw/bnxt_re/ib_verbs.c | 21 +++-------------- drivers/infiniband/hw/bnxt_re/main.c | 39 ++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h index 0877283..12950ec 100644 --- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h +++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h @@ -84,11 +84,13 @@ struct bnxt_re_dev { struct ib_device ibdev; struct list_head list; unsigned long flags; -#define BNXT_RE_FLAG_NETDEV_REGISTERED 0 -#define BNXT_RE_FLAG_IBDEV_REGISTERED 1 -#define BNXT_RE_FLAG_GOT_MSIX 2 -#define BNXT_RE_FLAG_RCFW_CHANNEL_EN 8 -#define BNXT_RE_FLAG_QOS_WORK_REG 16 +#define BNXT_RE_FLAG_NETDEV_REGISTERED 0 +#define BNXT_RE_FLAG_IBDEV_REGISTERED 1 +#define BNXT_RE_FLAG_GOT_MSIX 2 +#define BNXT_RE_FLAG_RCFW_CHANNEL_EN 4 +#define BNXT_RE_FLAG_QOS_WORK_REG 5 +#define BNXT_RE_FLAG_NETDEV_REG_IN_PROG 6 + struct net_device *netdev; unsigned int version, major, minor; struct bnxt_en_dev *en_dev; @@ -131,6 +133,7 @@ struct bnxt_re_dev { struct bnxt_re_qp *qp1_sqp; struct bnxt_re_ah *sqp_ah; struct bnxt_re_sqp_entries sqp_tbl[1024]; + u32 espeed; }; #define to_bnxt_re_dev(ptr, member) \ diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c index c7bd683..fa388f2 100644 --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c @@ -223,20 +223,8 @@ int bnxt_re_modify_device(struct ib_device *ibdev, return 0; } -static void __to_ib_speed_width(struct net_device *netdev, u8 *speed, u8 *width) +static void __to_ib_speed_width(u32 espeed, u8 *speed, u8 *width) { - struct ethtool_link_ksettings lksettings; - u32 espeed; - - if (netdev->ethtool_ops && netdev->ethtool_ops->get_link_ksettings) { - memset(&lksettings, 0, sizeof(lksettings)); - rtnl_lock(); - netdev->ethtool_ops->get_link_ksettings(netdev, &lksettings); - rtnl_unlock(); - espeed = lksettings.base.speed; - } else { - espeed = SPEED_UNKNOWN; - } switch (espeed) { case SPEED_1000: *speed = IB_SPEED_SDR; @@ -303,12 +291,9 @@ int bnxt_re_query_port(struct ib_device *ibdev, u8 port_num, port_attr->sm_sl = 0; port_attr->subnet_timeout = 0; port_attr->init_type_reply = 0; - /* call the underlying netdev's ethtool hooks to query speed settings - * for which we acquire rtnl_lock _only_ if it's registered with - * IB stack to avoid race in the NETDEV_UNREG path - */ + if (test_bit(BNXT_RE_FLAG_IBDEV_REGISTERED, &rdev->flags)) - __to_ib_speed_width(rdev->netdev, &port_attr->active_speed, + __to_ib_speed_width(rdev->espeed, &port_attr->active_speed, &port_attr->active_width); return 0; } diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c index 1fce5e7..91b5208 100644 --- a/drivers/infiniband/hw/bnxt_re/main.c +++ b/drivers/infiniband/hw/bnxt_re/main.c @@ -48,6 +48,8 @@ #include #include #include +#include +#include #include #include @@ -922,6 +924,10 @@ static void bnxt_re_ib_unreg(struct bnxt_re_dev *rdev, bool lock_wait) if (test_and_clear_bit(BNXT_RE_FLAG_QOS_WORK_REG, &rdev->flags)) cancel_delayed_work(&rdev->worker); + /* Wait for ULPs to release references */ + while (atomic_read(&rdev->cq_count)) + usleep_range(500, 1000); + bnxt_re_cleanup_res(rdev); bnxt_re_free_res(rdev, lock_wait); @@ -1148,6 +1154,22 @@ static void bnxt_re_remove_one(struct bnxt_re_dev *rdev) pci_dev_put(rdev->en_dev->pdev); } +static void bnxt_re_get_link_speed(struct bnxt_re_dev *rdev) +{ + struct ethtool_link_ksettings lksettings; + struct net_device *netdev = rdev->netdev; + + if (netdev->ethtool_ops && netdev->ethtool_ops->get_link_ksettings) { + memset(&lksettings, 0, sizeof(lksettings)); + if (rtnl_trylock()) { + netdev->ethtool_ops->get_link_ksettings(netdev, + &lksettings); + rdev->espeed = lksettings.base.speed; + rtnl_unlock(); + } + } +} + /* Handle all deferred netevents tasks */ static void bnxt_re_task(struct work_struct *work) { @@ -1165,6 +1187,10 @@ static void bnxt_re_task(struct work_struct *work) switch (re_work->event) { case NETDEV_REGISTER: rc = bnxt_re_ib_reg(rdev); + /* Use memory barrier to sync the rdev->flags */ + smp_mb(); + clear_bit(BNXT_RE_FLAG_NETDEV_REG_IN_PROG, + &rdev->flags); if (rc) dev_err(rdev_to_dev(rdev), "Failed to register with IB: %#x", rc); @@ -1182,6 +1208,7 @@ static void bnxt_re_task(struct work_struct *work) else if (netif_carrier_ok(rdev->netdev)) bnxt_re_dispatch_event(&rdev->ibdev, NULL, 1, IB_EVENT_PORT_ACTIVE); + bnxt_re_get_link_speed(rdev); break; default: break; @@ -1240,10 +1267,22 @@ static int bnxt_re_netdev_event(struct notifier_block *notifier, break; } bnxt_re_init_one(rdev); + /* + * Query the link speed at the time of registration. + * Already in rtnl_lock context + */ + bnxt_re_get_link_speed(rdev); + + set_bit(BNXT_RE_FLAG_NETDEV_REG_IN_PROG, &rdev->flags); sch_work = true; break; case NETDEV_UNREGISTER: + /* netdev notifier will call NETDEV_UNREGISTER again later since + * we are still holding the reference to the netdev + */ + if (test_bit(BNXT_RE_FLAG_NETDEV_REG_IN_PROG, &rdev->flags)) + goto exit; bnxt_re_ib_unreg(rdev, false); bnxt_re_remove_one(rdev); bnxt_re_dev_unreg(rdev);