From patchwork Mon Jul 10 15:05:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yishai Hadas X-Patchwork-Id: 9833223 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 5745A60393 for ; Mon, 10 Jul 2017 15:06:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4873026E3E for ; Mon, 10 Jul 2017 15:06:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3D1ED2857D; Mon, 10 Jul 2017 15:06:12 +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, UNPARSEABLE_RELAY 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 B968A28539 for ; Mon, 10 Jul 2017 15:06:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753565AbdGJPGK (ORCPT ); Mon, 10 Jul 2017 11:06:10 -0400 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:40399 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753646AbdGJPGJ (ORCPT ); Mon, 10 Jul 2017 11:06:09 -0400 Received: from Internal Mail-Server by MTLPINE1 (envelope-from yishaih@mellanox.com) with ESMTPS (AES256-SHA encrypted); 10 Jul 2017 18:06:03 +0300 Received: from vnc17.mtl.labs.mlnx (vnc17.mtl.labs.mlnx [10.7.2.17]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id v6AF5xwl015664; Mon, 10 Jul 2017 18:06:02 +0300 Received: from vnc17.mtl.labs.mlnx (vnc17.mtl.labs.mlnx [127.0.0.1]) by vnc17.mtl.labs.mlnx (8.13.8/8.13.8) with ESMTP id v6AF5xRL029114; Mon, 10 Jul 2017 18:05:59 +0300 Received: (from yishaih@localhost) by vnc17.mtl.labs.mlnx (8.13.8/8.13.8/Submit) id v6AF5xmO029113; Mon, 10 Jul 2017 18:05:59 +0300 From: Yishai Hadas To: dledford@redhat.com Cc: linux-rdma@vger.kernel.org, yishaih@mellanox.com, maorg@mellanox.com, majd@mellanox.com Subject: [PATCH V1 rdma-core 4/5] verbs: Avoid ibv_device memory leak Date: Mon, 10 Jul 2017 18:05:36 +0300 Message-Id: <1499699137-29037-5-git-send-email-yishaih@mellanox.com> X-Mailer: git-send-email 1.8.2.3 In-Reply-To: <1499699137-29037-1-git-send-email-yishaih@mellanox.com> References: <1499699137-29037-1-git-send-email-yishaih@mellanox.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: Maor Gottlieb Now, ibv_device list is refreshed each time that ibv_get_device_list is called, therefore we need to free devices from previous scan which aren't bound anymore, otherwise it could lead to memory leak of ibv_device structures. We can free the memory of ibv_device only if it isn't used anymore by the user. How we identify if device is still used ======================================== We add a reference count to the verbs device struct. This reference count is increased when: a. Set to one for having the device in the list until it should be deleted. b. User call to ibv_get_device_list. c. User call to ibv_open_device. The reference count is decreased when: a. User call to ibv_free_device_list. b. User call to ibv_close_device. c. Device is no longer exists in the sysfs. Device will be freed when the refcount is decreased to zero. For free the ibv_device struct, we add uninit_device callback function to verbs_device_ops. Signed-off-by: Maor Gottlieb Reviewed-by: Yishai Hadas --- libibverbs/device.c | 9 +++++++++ libibverbs/driver.h | 3 +++ libibverbs/ibverbs.h | 2 ++ libibverbs/init.c | 24 +++++++++++++++++++++++- 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/libibverbs/device.c b/libibverbs/device.c index 146e943..ac04bb3 100644 --- a/libibverbs/device.c +++ b/libibverbs/device.c @@ -99,6 +99,7 @@ struct ibv_device **__ibv_get_device_list(int *num) list_for_each(&device_list, device, entry) { l[i] = &device->device; + ibverbs_device_hold(l[i]); i++; } if (num) @@ -111,6 +112,10 @@ default_symver(__ibv_get_device_list, ibv_get_device_list); void __ibv_free_device_list(struct ibv_device **list) { + int i; + + for (i = 0; list[i]; i++) + ibverbs_device_put(list[i]); free(list); } default_symver(__ibv_free_device_list, ibv_free_device_list); @@ -260,6 +265,8 @@ struct ibv_context *__ibv_open_device(struct ibv_device *device) context->cmd_fd = cmd_fd; pthread_mutex_init(&context->mutex, NULL); + ibverbs_device_hold(device); + return context; verbs_err: @@ -278,6 +285,7 @@ int __ibv_close_device(struct ibv_context *context) int cq_fd = -1; struct verbs_context *context_ex; struct verbs_device *verbs_device = verbs_get_device(context->device); + struct ibv_device *device = context->device; context_ex = verbs_get_ctx(context); if (context_ex) { @@ -292,6 +300,7 @@ int __ibv_close_device(struct ibv_context *context) close(cmd_fd); if (abi_ver <= 2) close(cq_fd); + ibverbs_device_put(device); return 0; } diff --git a/libibverbs/driver.h b/libibverbs/driver.h index 2ed846e..298bd35 100644 --- a/libibverbs/driver.h +++ b/libibverbs/driver.h @@ -35,6 +35,7 @@ #ifndef INFINIBAND_DRIVER_H #define INFINIBAND_DRIVER_H +#include #include #include #include @@ -112,6 +113,7 @@ struct verbs_device_ops { struct ibv_context *ctx, int cmd_fd); void (*uninit_context)(struct verbs_device *device, struct ibv_context *ctx); + void (*uninit_device)(struct verbs_device *device); }; /* Must change the PRIVATE IBVERBS_PRIVATE_ symbol if this is changed */ @@ -120,6 +122,7 @@ struct verbs_device { const struct verbs_device_ops *ops; size_t sz; size_t size_of_context; + atomic_int refcount; struct list_node entry; struct ibv_sysfs_dev *sysfs; }; diff --git a/libibverbs/ibverbs.h b/libibverbs/ibverbs.h index 3e2fe8f..05fd2c8 100644 --- a/libibverbs/ibverbs.h +++ b/libibverbs/ibverbs.h @@ -61,6 +61,8 @@ extern int abi_ver; int ibverbs_get_device_list(struct list_head *list); int ibverbs_init(void); +void ibverbs_device_put(struct ibv_device *dev); +void ibverbs_device_hold(struct ibv_device *dev); struct verbs_ex_private { struct ibv_cq_ex *(*create_cq_ex)(struct ibv_context *context, diff --git a/libibverbs/init.c b/libibverbs/init.c index 5ad95ca..b3046b3 100644 --- a/libibverbs/init.c +++ b/libibverbs/init.c @@ -382,6 +382,7 @@ static struct verbs_device *try_driver(struct ibv_driver *driver, if (!vdev) return NULL; + atomic_init(&vdev->refcount, 1); dev = &vdev->device; assert(dev->_ops._dummy1 == NULL); assert(dev->_ops._dummy2 == NULL); @@ -512,8 +513,11 @@ int ibverbs_get_device_list(struct list_head *list) break; } } - if (!sysfs_dev) + + if (!sysfs_dev) { list_del(&vdev->entry); + ibverbs_device_put(&vdev->device); + } } for (sysfs_dev = tmp_sysfs_dev_list; sysfs_dev; sysfs_dev = @@ -610,3 +614,21 @@ int ibverbs_init(void) return 0; } + +void ibverbs_device_hold(struct ibv_device *dev) +{ + struct verbs_device *verbs_device = verbs_get_device(dev); + + atomic_fetch_add(&verbs_device->refcount, 1); +} + +void ibverbs_device_put(struct ibv_device *dev) +{ + struct verbs_device *verbs_device = verbs_get_device(dev); + + if (atomic_fetch_sub(&verbs_device->refcount, 1) == 1) { + free(verbs_device->sysfs); + if (verbs_device->ops->uninit_device) + verbs_device->ops->uninit_device(verbs_device); + } +}