From patchwork Fri May 19 04:44:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selvin Xavier X-Patchwork-Id: 9735833 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 51B46601A1 for ; Fri, 19 May 2017 04:45:10 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3EF08288AA for ; Fri, 19 May 2017 04:45:10 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 33092288CA; Fri, 19 May 2017 04:45:10 +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.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, 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 6AC4A288AA for ; Fri, 19 May 2017 04:45:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750741AbdESEpI (ORCPT ); Fri, 19 May 2017 00:45:08 -0400 Received: from mail-wm0-f43.google.com ([74.125.82.43]:36157 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750703AbdESEpH (ORCPT ); Fri, 19 May 2017 00:45:07 -0400 Received: by mail-wm0-f43.google.com with SMTP id 70so67791441wmq.1 for ; Thu, 18 May 2017 21:45:07 -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=O60P1CVETlqR67VvD+zTH9SeTqFxif1lsj/jPXxdGL4=; b=aV9OgmZEcdBKJA2NMmvnDpraWB1EQlCU1xAOL56ZIB1izeUQS11DnqJjQqusQKoZGS zt8PqqYN1tV8ZrsdN5f7F00kzr3YEh7JrLMI+1WEfJdaK06L+WlpPIy5bHa9HCP/SqgO YsNWVqaADypUk42XJGn7eiyUT0w9iV5N/W7lc= 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=O60P1CVETlqR67VvD+zTH9SeTqFxif1lsj/jPXxdGL4=; b=YGqhC6Ncu/j+t26zqGnf4BKVHW653DLLBY3uWpqYoCtZew8M0dYwj8/tTWyQN2Vu6z Ed9JS20uF1jFMa589UBf7zyx7GTc+ZXRkHyRaR3DOGDl3wHBzPdar4YHLRkKfeJ5k95C nr4TCuBD4bjLEx2KOGUP5k244yRukTC/JbrIyLlr2PEB6otHdAz6VOmtOXBV2pulQ3qv QhuErQNLZj95rFGp68huRZanZ5Xp82hLoE/lVJ5Llt/pvt4mQl7jHrT80dJFNkVWInbq fZCaXuGc6IJAM2I+XhHfQMmtZ84TlFcntptyb1jX0irfUB7mrGpo9IE5l5EW4V/GfkfI u65g== X-Gm-Message-State: AODbwcA3v4JO5UGjd7AHGJ0rWPQLaladZAzsyTYi2I7/BUelov2xElXc bzMRgt4YiuSoTVaS X-Received: by 10.28.9.204 with SMTP id 195mr5321702wmj.97.1495169106121; Thu, 18 May 2017 21:45:06 -0700 (PDT) Received: from dhcp-10-192-206-197.iig.avagotech.net ([192.19.239.250]) by smtp.gmail.com with ESMTPSA id p107sm1202508wrb.64.2017.05.18.21.45.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 May 2017 21:45:05 -0700 (PDT) From: Selvin Xavier To: dledford@redhat.com Cc: linux-rdma@vger.kernel.org, Somnath Kotur , Sriharsha Basavapatna , Selvin Xavier Subject: [PATCH V3 for-next 03/15] RDMA/bnxt_re: Fix race between netdev register and unregister events Date: Thu, 18 May 2017 21:44:22 -0700 Message-Id: <1495169074-12641-4-git-send-email-selvin.xavier@broadcom.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1495169074-12641-1-git-send-email-selvin.xavier@broadcom.com> References: <1495169074-12641-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 ebf7be8..fad04b2 100644 --- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h +++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h @@ -80,11 +80,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; @@ -127,6 +129,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 ae428e6..a89f095 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 5d35540..99a54fd 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 @@ -926,6 +928,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); @@ -1152,6 +1158,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) { @@ -1169,6 +1191,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); @@ -1186,6 +1212,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; @@ -1244,10 +1271,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);