From patchwork Wed Jul 5 21:17:52 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Ledford X-Patchwork-Id: 9827071 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 2B88760237 for ; Wed, 5 Jul 2017 21:18:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 13DF928504 for ; Wed, 5 Jul 2017 21:18:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 06BFE28512; Wed, 5 Jul 2017 21:18:14 +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.9 required=2.0 tests=BAYES_00,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 1947928504 for ; Wed, 5 Jul 2017 21:18:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751929AbdGEVSM (ORCPT ); Wed, 5 Jul 2017 17:18:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33560 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751914AbdGEVSL (ORCPT ); Wed, 5 Jul 2017 17:18:11 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ED35C267C4; Wed, 5 Jul 2017 21:18:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com ED35C267C4 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=dledford@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com ED35C267C4 Received: from linux-ws.xsintricity.com.com (ovpn-120-61.rdu2.redhat.com [10.10.120.61]) by smtp.corp.redhat.com (Postfix) with ESMTP id 32B2E60BE3; Wed, 5 Jul 2017 21:18:10 +0000 (UTC) From: Doug Ledford To: linux-rdma@vger.kernel.org Cc: Niranjana Vishwanathapura , leon@kernel.org, Dennis Dalessandro , Doug Ledford Subject: [PATCH for-rc v1] IB/core, opa_vnic, hfi1, mlx5: Properly free rdma_netdev Date: Wed, 5 Jul 2017 17:17:52 -0400 Message-Id: <8e959601996dc645f4ed7004482a1667c27deb39.1499289360.git.dledford@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 05 Jul 2017 21:18:11 +0000 (UTC) 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: Niranjana Vishwanathapura IPOIB is calling free_rdma_netdev even though alloc_rdma_netdev has returned -EOPNOTSUPP. Move free_rdma_netdev from ib_device structure to rdma_netdev structure thus ensuring proper cleanup function is called for the rdma net device. Fix the following trace: ib0: Failed to modify QP to ERROR state BUG: unable to handle kernel paging request at 0000000000001d20 IP: hfi1_vnic_free_rn+0x26/0xb0 [hfi1] Call Trace: ipoib_remove_one+0xbe/0x160 [ib_ipoib] ib_unregister_device+0xd0/0x170 [ib_core] rvt_unregister_device+0x29/0x90 [rdmavt] hfi1_unregister_ib_device+0x1a/0x100 [hfi1] remove_one+0x4b/0x220 [hfi1] pci_device_remove+0x39/0xc0 device_release_driver_internal+0x141/0x200 driver_detach+0x3f/0x80 bus_remove_driver+0x55/0xd0 driver_unregister+0x2c/0x50 pci_unregister_driver+0x2a/0xa0 hfi1_mod_cleanup+0x10/0xf65 [hfi1] SyS_delete_module+0x171/0x250 do_syscall_64+0x67/0x150 entry_SYSCALL64_slow_path+0x25/0x25 Reviewed-by: Dennis Dalessandro Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Dennis Dalessandro Signed-off-by: Doug Ledford Reviewed-by: Leon Romanovsky --- v1 - I fixed this up to resolve Leon's comment. Leon, please make sure you are happy with the change to the mlx5 code. drivers/infiniband/hw/hfi1/verbs.c | 1 - drivers/infiniband/hw/hfi1/vnic.h | 1 - drivers/infiniband/hw/hfi1/vnic_main.c | 19 ++++++++-------- drivers/infiniband/hw/mlx5/main.c | 27 ++++++++++++++--------- drivers/infiniband/ulp/ipoib/ipoib_main.c | 8 +++---- drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c | 8 +++---- include/rdma/ib_verbs.h | 6 +++-- 7 files changed, 39 insertions(+), 31 deletions(-) diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c index 90e7b77d68e8..2d19f9bb434d 100644 --- a/drivers/infiniband/hw/hfi1/verbs.c +++ b/drivers/infiniband/hw/hfi1/verbs.c @@ -1779,7 +1779,6 @@ int hfi1_register_ib_device(struct hfi1_devdata *dd) ibdev->alloc_hw_stats = alloc_hw_stats; ibdev->get_hw_stats = get_hw_stats; ibdev->alloc_rdma_netdev = hfi1_vnic_alloc_rn; - ibdev->free_rdma_netdev = hfi1_vnic_free_rn; /* keep process mad in the driver */ ibdev->process_mad = hfi1_process_mad; diff --git a/drivers/infiniband/hw/hfi1/vnic.h b/drivers/infiniband/hw/hfi1/vnic.h index e2c455299b53..4a621cde4abb 100644 --- a/drivers/infiniband/hw/hfi1/vnic.h +++ b/drivers/infiniband/hw/hfi1/vnic.h @@ -176,7 +176,6 @@ struct net_device *hfi1_vnic_alloc_rn(struct ib_device *device, const char *name, unsigned char name_assign_type, void (*setup)(struct net_device *)); -void hfi1_vnic_free_rn(struct net_device *netdev); int hfi1_vnic_send_dma(struct hfi1_devdata *dd, u8 q_idx, struct hfi1_vnic_vport_info *vinfo, struct sk_buff *skb, u64 pbc, u8 plen); diff --git a/drivers/infiniband/hw/hfi1/vnic_main.c b/drivers/infiniband/hw/hfi1/vnic_main.c index b601c2929f8f..339f0cdd56d6 100644 --- a/drivers/infiniband/hw/hfi1/vnic_main.c +++ b/drivers/infiniband/hw/hfi1/vnic_main.c @@ -833,6 +833,15 @@ static const struct net_device_ops hfi1_netdev_ops = { .ndo_get_stats64 = hfi1_vnic_get_stats64, }; +static void hfi1_vnic_free_rn(struct net_device *netdev) +{ + struct hfi1_vnic_vport_info *vinfo = opa_vnic_dev_priv(netdev); + + hfi1_vnic_deinit(vinfo); + mutex_destroy(&vinfo->lock); + free_netdev(netdev); +} + struct net_device *hfi1_vnic_alloc_rn(struct ib_device *device, u8 port_num, enum rdma_netdev_t type, @@ -864,6 +873,7 @@ struct net_device *hfi1_vnic_alloc_rn(struct ib_device *device, vinfo->num_tx_q = dd->chip_sdma_engines; vinfo->num_rx_q = HFI1_NUM_VNIC_CTXT; vinfo->netdev = netdev; + rn->free_rdma_netdev = hfi1_vnic_free_rn; rn->set_id = hfi1_vnic_set_vesw_id; netdev->features = NETIF_F_HIGHDMA | NETIF_F_SG; @@ -892,12 +902,3 @@ struct net_device *hfi1_vnic_alloc_rn(struct ib_device *device, free_netdev(netdev); return ERR_PTR(rc); } - -void hfi1_vnic_free_rn(struct net_device *netdev) -{ - struct hfi1_vnic_vport_info *vinfo = opa_vnic_dev_priv(netdev); - - hfi1_vnic_deinit(vinfo); - mutex_destroy(&vinfo->lock); - free_netdev(netdev); -} diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c index 9ecc089d4529..afa5f6e88e1d 100644 --- a/drivers/infiniband/hw/mlx5/main.c +++ b/drivers/infiniband/hw/mlx5/main.c @@ -3542,6 +3542,11 @@ static int mlx5_ib_get_hw_stats(struct ib_device *ibdev, return num_counters; } +static void mlx5_ib_free_rdma_netdev(struct net_device *netdev) +{ + return mlx5_rdma_netdev_free(netdev); +} + static struct net_device* mlx5_ib_alloc_rdma_netdev(struct ib_device *hca, u8 port_num, @@ -3550,16 +3555,19 @@ mlx5_ib_alloc_rdma_netdev(struct ib_device *hca, unsigned char name_assign_type, void (*setup)(struct net_device *)) { + struct net_device *netdev; + struct rdma_netdev *rn; + if (type != RDMA_NETDEV_IPOIB) return ERR_PTR(-EOPNOTSUPP); - return mlx5_rdma_netdev_alloc(to_mdev(hca)->mdev, hca, - name, setup); -} - -static void mlx5_ib_free_rdma_netdev(struct net_device *netdev) -{ - return mlx5_rdma_netdev_free(netdev); + netdev = mlx5_rdma_netdev_alloc(to_mdev(hca)->mdev, hca, + name, setup); + if (likely(!IS_ERR_OR_NULL(netdev))) { + rn = netdev_priv(netdev); + rn->free_rdma_netdev = mlx5_ib_free_rdma_netdev; + } + return netdev; } static void *mlx5_ib_add(struct mlx5_core_dev *mdev) @@ -3692,10 +3700,9 @@ static void *mlx5_ib_add(struct mlx5_core_dev *mdev) dev->ib_dev.check_mr_status = mlx5_ib_check_mr_status; dev->ib_dev.get_port_immutable = mlx5_port_immutable; dev->ib_dev.get_dev_fw_str = get_dev_fw_str; - if (MLX5_CAP_GEN(mdev, ipoib_enhanced_offloads)) { + if (MLX5_CAP_GEN(mdev, ipoib_enhanced_offloads)) dev->ib_dev.alloc_rdma_netdev = mlx5_ib_alloc_rdma_netdev; - dev->ib_dev.free_rdma_netdev = mlx5_ib_free_rdma_netdev; - } + if (mlx5_core_is_pf(mdev)) { dev->ib_dev.get_vf_config = mlx5_ib_get_vf_config; dev->ib_dev.set_vf_link_state = mlx5_ib_set_vf_link_state; diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 1015a63de6ae..9ec0dbea3b6b 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -1893,6 +1893,7 @@ static struct net_device rn->send = ipoib_send; rn->attach_mcast = ipoib_mcast_attach; rn->detach_mcast = ipoib_mcast_detach; + rn->free_rdma_netdev = free_netdev; rn->hca = hca; dev->netdev_ops = &ipoib_netdev_default_pf; @@ -2288,6 +2289,8 @@ static void ipoib_remove_one(struct ib_device *device, void *client_data) return; list_for_each_entry_safe(priv, tmp, dev_list, list) { + struct rdma_netdev *rn = netdev_priv(priv->dev); + ib_unregister_event_handler(&priv->event_handler); flush_workqueue(ipoib_workqueue); @@ -2304,10 +2307,7 @@ static void ipoib_remove_one(struct ib_device *device, void *client_data) flush_workqueue(priv->wq); unregister_netdev(priv->dev); - if (device->free_rdma_netdev) - device->free_rdma_netdev(priv->dev); - else - free_netdev(priv->dev); + rn->free_rdma_netdev(priv->dev); list_for_each_entry_safe(cpriv, tcpriv, &priv->child_intfs, list) kfree(cpriv); diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c index 78d9007bc2f6..1a89c6033358 100644 --- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c +++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c @@ -323,13 +323,13 @@ struct opa_vnic_adapter *opa_vnic_add_netdev(struct ib_device *ibdev, else if (IS_ERR(netdev)) return ERR_CAST(netdev); + rn = netdev_priv(netdev); adapter = kzalloc(sizeof(*adapter), GFP_KERNEL); if (!adapter) { rc = -ENOMEM; goto adapter_err; } - rn = netdev_priv(netdev); rn->clnt_priv = adapter; rn->hca = ibdev; rn->port_num = port_num; @@ -366,7 +366,7 @@ struct opa_vnic_adapter *opa_vnic_add_netdev(struct ib_device *ibdev, mutex_destroy(&adapter->mactbl_lock); kfree(adapter); adapter_err: - ibdev->free_rdma_netdev(netdev); + rn->free_rdma_netdev(netdev); return ERR_PTR(rc); } @@ -375,7 +375,7 @@ struct opa_vnic_adapter *opa_vnic_add_netdev(struct ib_device *ibdev, void opa_vnic_rem_netdev(struct opa_vnic_adapter *adapter) { struct net_device *netdev = adapter->netdev; - struct ib_device *ibdev = adapter->ibdev; + struct rdma_netdev *rn = netdev_priv(netdev); v_info("removing\n"); unregister_netdev(netdev); @@ -383,5 +383,5 @@ void opa_vnic_rem_netdev(struct opa_vnic_adapter *adapter) mutex_destroy(&adapter->lock); mutex_destroy(&adapter->mactbl_lock); kfree(adapter); - ibdev->free_rdma_netdev(netdev); + rn->free_rdma_netdev(netdev); } diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index ba8314ec5768..71313d5ca1c8 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -1927,6 +1927,9 @@ struct rdma_netdev { struct ib_device *hca; u8 port_num; + /* cleanup function must be specified */ + void (*free_rdma_netdev)(struct net_device *netdev); + /* control functions */ void (*set_id)(struct net_device *netdev, int id); /* send packet */ @@ -2194,7 +2197,7 @@ struct ib_device { struct ib_udata *udata); int (*destroy_rwq_ind_table)(struct ib_rwq_ind_table *wq_ind_table); /** - * rdma netdev operations + * rdma netdev operation * * Driver implementing alloc_rdma_netdev must return -EOPNOTSUPP if it * doesn't support the specified rdma netdev type. @@ -2206,7 +2209,6 @@ struct ib_device { const char *name, unsigned char name_assign_type, void (*setup)(struct net_device *)); - void (*free_rdma_netdev)(struct net_device *netdev); struct module *owner; struct device dev;