From patchwork Wed May 10 10:45:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selvin Xavier X-Patchwork-Id: 9719653 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 B9317603F8 for ; Wed, 10 May 2017 10:46:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AEDE728576 for ; Wed, 10 May 2017 10:46:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A339328591; Wed, 10 May 2017 10:46:13 +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 1896128576 for ; Wed, 10 May 2017 10:46:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752580AbdEJKqH (ORCPT ); Wed, 10 May 2017 06:46:07 -0400 Received: from mail-qk0-f177.google.com ([209.85.220.177]:35759 "EHLO mail-qk0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752448AbdEJKqG (ORCPT ); Wed, 10 May 2017 06:46:06 -0400 Received: by mail-qk0-f177.google.com with SMTP id a72so24553114qkj.2 for ; Wed, 10 May 2017 03:46:06 -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=Ufnv3uvFJB1cP9LNMi80BwokoFOZpBb5rct74gz/mOk=; b=JtXqEVxCRMJKVibGHgwRHyx73uxsiXvvrV8CYLb9/Mmwe68sU0iL+qj7ToyzPATKY3 s9T8fl1lDgJ/ozp0hNubzXz8be/Kmq/m+fJY3MU2zgvzXXYs3l8tHvjJ+xxZrq2oZUqU dMtUp636qQT6H9aBV6BFaZ47RK7OBUCEnWiK0= 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=Ufnv3uvFJB1cP9LNMi80BwokoFOZpBb5rct74gz/mOk=; b=MYiiRaKk5/AssdL//wiVG4Gz98HvB90rxMocrp9iunNPwi8wA0Q6G3FKb83ellFlo7 tQOzeIJ0FVIs5XuZlNVYx78z0/wfdFZniGZqvKJ78QuAXRZBlYcqcxGQ9+AiaYDo7wEL PD6n6VNDij8M0F9VjOSzxTeO2Zm2NdF9c1MtLayNOuz+cG2mjEGPzBnupCcZ1SKgry14 AySKNqTyD2l9S3SsBEzBLa2wEaby9bg+p2qA1kvq9kr1g7ju85mQge5sTzY+RHx/GTRf c+D0ttsHQq4lPEf/IgHSLuoHZORcDHVU0yeFqAcUjmBZ2catA32YA3FAKE55wDop2Wfg zftA== X-Gm-Message-State: AODbwcBOuwbI+8zVKQwVH79qNAdcyISG7QvVN5LcC80NKAjXy1bti4q+ g5SLRUD+UvlkIJqR5wc= X-Received: by 10.55.130.3 with SMTP id e3mr4850184qkd.95.1494413165374; Wed, 10 May 2017 03:46:05 -0700 (PDT) Received: from dhcp-10-192-206-197.iig.avagotech.net ([192.19.239.250]) by smtp.gmail.com with ESMTPSA id k10sm1868531qke.3.2017.05.10.03.46.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 May 2017 03:46:04 -0700 (PDT) From: Selvin Xavier To: dledford@redhat.com, linux-rdma@vger.kernel.org Cc: Somnath Kotur , Sriharsha Basavapatna , Selvin Xavier Subject: [PATCH for-next 03/14] RDMA/bnxt_re: Fix race between netdev register and unregister events Date: Wed, 10 May 2017 03:45:28 -0700 Message-Id: <1494413139-11883-4-git-send-email-selvin.xavier@broadcom.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1494413139-11883-1-git-send-email-selvin.xavier@broadcom.com> References: <1494413139-11883-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 6385213..c717d5d 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);