* [PATCH rdma-core 1/5] utils: Add timespec compare utility
[not found] ` <1498744816-23354-1-git-send-email-yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
@ 2017-06-29 14:00 ` Yishai Hadas
2017-06-29 14:00 ` [PATCH rdma-core 2/5] verbs: Split ibverbs_init functionality Yishai Hadas
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Yishai Hadas @ 2017-06-29 14:00 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, maorg-VPRAkNaXOzVWk0Htik3J/w,
majd-VPRAkNaXOzVWk0Htik3J/w
From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Move ts_cmp from srp_daemon.h to util.h, to be used from
another place in a downstream patch.
Signed-off-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
srp_daemon/srp_daemon.h | 7 +------
util/util.h | 6 ++++++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/srp_daemon/srp_daemon.h b/srp_daemon/srp_daemon.h
index 026db41..5d268ed 100644
--- a/srp_daemon/srp_daemon.h
+++ b/srp_daemon/srp_daemon.h
@@ -39,6 +39,7 @@
#include <stdint.h>
#include <signal.h>
#include <endian.h>
+#include <util/util.h>
#include <infiniband/verbs.h>
#include <infiniband/umad.h>
#include <linux/types.h> /* __be16, __be32 and __be64 */
@@ -47,12 +48,6 @@
#include "config.h"
#include "srp_ib_types.h"
-/* a CMP b. See also the BSD macro timercmp(). */
-#define ts_cmp(a, b, CMP) \
- (((a)->tv_sec == (b)->tv_sec) ? \
- ((a)->tv_nsec CMP (b)->tv_nsec) : \
- ((a)->tv_sec CMP (b)->tv_sec))
-
#define SRP_CATAS_ERR SIGUSR1
enum {
diff --git a/util/util.h b/util/util.h
index ed4ffe8..c3e5955 100644
--- a/util/util.h
+++ b/util/util.h
@@ -10,4 +10,10 @@
(rc < len && rc >= 0); \
})
+/* a CMP b. See also the BSD macro timercmp(). */
+#define ts_cmp(a, b, CMP) \
+ (((a)->tv_sec == (b)->tv_sec) ? \
+ ((a)->tv_nsec CMP (b)->tv_nsec) : \
+ ((a)->tv_sec CMP (b)->tv_sec))
+
#endif
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH rdma-core 2/5] verbs: Split ibverbs_init functionality
[not found] ` <1498744816-23354-1-git-send-email-yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2017-06-29 14:00 ` [PATCH rdma-core 1/5] utils: Add timespec compare utility Yishai Hadas
@ 2017-06-29 14:00 ` Yishai Hadas
2017-06-29 14:00 ` [PATCH rdma-core 3/5] verbs: Refresh cahced ibv_device list Yishai Hadas
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Yishai Hadas @ 2017-06-29 14:00 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, maorg-VPRAkNaXOzVWk0Htik3J/w,
majd-VPRAkNaXOzVWk0Htik3J/w
From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
ibverbs_init is called only in the first time that
ibv_get_device_list is called by the user.
This function does two main things:
1) Initialization actions like fork init, memory check, etc.
2) Scan the sysfs device list and build the cached ibv_device list.
This patch refactors the ibverbs_init and moves out the built
device list code into new function - ibverbs_get_device_list.
Motivation: In downstream patch, we refresh the ibv_device list
according to the updated snapshot of the sysfs devices, so we want
that the initialization action will continue to be singleton, but
rescan the sysfs in each call to ibv_get_device_list.
Signed-off-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
libibverbs/init.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/libibverbs/init.c b/libibverbs/init.c
index fdabe82..6e453ea 100644
--- a/libibverbs/init.c
+++ b/libibverbs/init.c
@@ -486,9 +486,8 @@ static void add_device(struct ibv_device *dev,
(*dev_list)[(*num_devices)++] = dev;
}
-int ibverbs_init(struct ibv_device ***list)
+int ibverbs_get_device_list(struct ibv_device ***list)
{
- const char *sysfs_path;
struct ibv_sysfs_dev *sysfs_dev, *next_dev;
struct ibv_device *device;
int num_devices = 0;
@@ -499,23 +498,6 @@ int ibverbs_init(struct ibv_device ***list)
*list = NULL;
- if (getenv("RDMAV_FORK_SAFE") || getenv("IBV_FORK_SAFE"))
- if (ibv_fork_init())
- fprintf(stderr, PFX "Warning: fork()-safety requested "
- "but init failed\n");
-
- sysfs_path = ibv_get_sysfs_path();
- if (!sysfs_path)
- return -ENOSYS;
-
- ret = check_abi_version(sysfs_path);
- if (ret)
- return -ret;
-
- check_memlock_limit();
-
- read_config();
-
ret = find_sysfs_devs();
if (ret)
return -ret;
@@ -582,3 +564,28 @@ out:
return num_devices;
}
+
+int ibverbs_init(struct ibv_device ***list)
+{
+ const char *sysfs_path;
+ int ret;
+
+ if (getenv("RDMAV_FORK_SAFE") || getenv("IBV_FORK_SAFE"))
+ if (ibv_fork_init())
+ fprintf(stderr, PFX "Warning: fork()-safety requested "
+ "but init failed\n");
+
+ sysfs_path = ibv_get_sysfs_path();
+ if (!sysfs_path)
+ return -ENOSYS;
+
+ ret = check_abi_version(sysfs_path);
+ if (ret)
+ return -ret;
+
+ check_memlock_limit();
+
+ read_config();
+
+ return ibverbs_get_device_list(list);
+}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH rdma-core 3/5] verbs: Refresh cahced ibv_device list
[not found] ` <1498744816-23354-1-git-send-email-yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2017-06-29 14:00 ` [PATCH rdma-core 1/5] utils: Add timespec compare utility Yishai Hadas
2017-06-29 14:00 ` [PATCH rdma-core 2/5] verbs: Split ibverbs_init functionality Yishai Hadas
@ 2017-06-29 14:00 ` Yishai Hadas
2017-06-29 14:00 ` [PATCH rdma-core 4/5] verbs: Avoid ibv_device memory leak Yishai Hadas
2017-06-29 14:00 ` [PATCH rdma-core 5/5] providers: Add support for uninit_device for all providers Yishai Hadas
4 siblings, 0 replies; 6+ messages in thread
From: Yishai Hadas @ 2017-06-29 14:00 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, maorg-VPRAkNaXOzVWk0Htik3J/w,
majd-VPRAkNaXOzVWk0Htik3J/w
From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Problem
=======
Currently, libibverbs builds the cached ibv_device list only
in the first time that ibv_get_device_list is called, as a result
the list is never updated, no matter if there were hardware changes
in the system.
Solution
========
Modify the implementation of ibv_get_device_list() so that
consecutive calls will re-scan the sysfs in the same manner as done
today in order to create a fresh ibv_device list each time.
For this purpose, the cached device list is change to be a
real linked list and not a dynamic array.
How we identify new devices
===========================
Identifying same device is done according to timestamp creation of
/sys/class/infiniband_verbs/uverbs%d/ibdev.
We get the file status with stat syscall and use the st_mtime field for
this purpose.
When we rescan the sysfs devices, then we check for each sysfs device
if it was already been in the last scan, if not then we allocate new
ibv_device and add it to the cached device list.
Next patch in this series handles the case that a device is no
longer in use.
Note:
This patch changes the IBVERBS_PRIVATE symbol as required in the note
above verbs_device struct.
Signed-off-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
debian/libibverbs1.symbols | 2 +-
libibverbs/device.c | 37 ++++++++++-----
libibverbs/driver.h | 3 ++
libibverbs/ibverbs.h | 3 +-
libibverbs/init.c | 109 +++++++++++++++++++++++++++------------------
libibverbs/libibverbs.map | 2 +-
6 files changed, 97 insertions(+), 59 deletions(-)
diff --git a/debian/libibverbs1.symbols b/debian/libibverbs1.symbols
index b96ef46..aaee795 100644
--- a/debian/libibverbs1.symbols
+++ b/debian/libibverbs1.symbols
@@ -1,7 +1,7 @@
libibverbs.so.1 libibverbs1 #MINVER#
IBVERBS_1.0@IBVERBS_1.0 1.1.6
IBVERBS_1.1@IBVERBS_1.1 1.1.6
- (symver)IBVERBS_PRIVATE_14 14
+ (symver)IBVERBS_PRIVATE_15 15
ibv_ack_async_event@IBVERBS_1.0 1.1.6
ibv_ack_async_event@IBVERBS_1.1 1.1.6
ibv_ack_cq_events@IBVERBS_1.0 1.1.6
diff --git a/libibverbs/device.c b/libibverbs/device.c
index ebf4970..b8c1fc1 100644
--- a/libibverbs/device.c
+++ b/libibverbs/device.c
@@ -59,41 +59,54 @@ int __ibv_get_async_event(struct ibv_context *context,
struct ibv_async_event *event);
void __ibv_ack_async_event(struct ibv_async_event *event);
-static pthread_once_t device_list_once = PTHREAD_ONCE_INIT;
-static int num_devices;
-static struct ibv_device **device_list;
+static pthread_once_t init_once = PTHREAD_ONCE_INIT;
+static pthread_mutex_t dev_list_lock = PTHREAD_MUTEX_INITIALIZER;
+static int initialized;
+static struct list_head device_list = LIST_HEAD_INIT(device_list);
-static void count_devices(void)
+static void init_resources(void)
{
- num_devices = ibverbs_init(&device_list);
+ initialized = ibverbs_init();
}
struct ibv_device **__ibv_get_device_list(int *num)
{
struct ibv_device **l;
- int i;
+ struct verbs_device *device;
+ int num_devices;
+ int i = 0;
if (num)
*num = 0;
- pthread_once(&device_list_once, count_devices);
+ pthread_once(&init_once, init_resources);
+ if (initialized < 0) {
+ errno = -initialized;
+ return NULL;
+ }
+ pthread_mutex_lock(&dev_list_lock);
+ num_devices = ibverbs_get_device_list(&device_list);
if (num_devices < 0) {
errno = -num_devices;
- return NULL;
+ l = NULL;
+ goto out;
}
l = calloc(num_devices + 1, sizeof (struct ibv_device *));
if (!l) {
errno = ENOMEM;
- return NULL;
+ goto out;
}
- for (i = 0; i < num_devices; ++i)
- l[i] = device_list[i];
+ list_for_each(&device_list, device, entry) {
+ l[i] = &device->device;
+ i++;
+ }
if (num)
*num = num_devices;
-
+out:
+ pthread_mutex_unlock(&dev_list_lock);
return l;
}
default_symver(__ibv_get_device_list, ibv_get_device_list);
diff --git a/libibverbs/driver.h b/libibverbs/driver.h
index ec87afd..2ed846e 100644
--- a/libibverbs/driver.h
+++ b/libibverbs/driver.h
@@ -37,6 +37,7 @@
#include <infiniband/verbs.h>
#include <infiniband/kern-abi.h>
+#include <ccan/list.h>
#ifdef __cplusplus
# define BEGIN_C_DECLS extern "C" {
@@ -119,6 +120,8 @@ struct verbs_device {
const struct verbs_device_ops *ops;
size_t sz;
size_t size_of_context;
+ struct list_node entry;
+ struct ibv_sysfs_dev *sysfs;
};
static inline struct verbs_device *
diff --git a/libibverbs/ibverbs.h b/libibverbs/ibverbs.h
index eb304ba..3e2fe8f 100644
--- a/libibverbs/ibverbs.h
+++ b/libibverbs/ibverbs.h
@@ -59,7 +59,8 @@ struct ibv_abi_compat_v2 {
extern int abi_ver;
-int ibverbs_init(struct ibv_device ***list);
+int ibverbs_get_device_list(struct list_head *list);
+int ibverbs_init(void);
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 6e453ea..5ad95ca 100644
--- a/libibverbs/init.c
+++ b/libibverbs/init.c
@@ -62,6 +62,8 @@ struct ibv_sysfs_dev {
struct ibv_sysfs_dev *next;
int abi_ver;
int have_driver;
+ struct timespec time_created;
+ int used;
};
struct ibv_driver_name {
@@ -75,11 +77,10 @@ struct ibv_driver {
struct ibv_driver *next;
};
-static struct ibv_sysfs_dev *sysfs_dev_list;
static struct ibv_driver_name *driver_name_list;
static struct ibv_driver *head_driver, *tail_driver;
-static int find_sysfs_devs(void)
+static int find_sysfs_devs(struct ibv_sysfs_dev **tmp_sysfs_dev_list)
{
char class_path[IBV_SYSFS_PATH_MAX];
DIR *class_dir;
@@ -140,15 +141,24 @@ static int find_sysfs_devs(void)
sysfs_dev->ibdev_name))
continue;
- sysfs_dev->next = sysfs_dev_list;
+ if (stat(sysfs_dev->ibdev_path, &buf)) {
+ fprintf(stderr, PFX "Warning: couldn't stat '%s'.\n",
+ sysfs_dev->ibdev_path);
+ continue;
+ }
+
+ sysfs_dev->time_created = buf.st_mtim;
+
+ sysfs_dev->next = *tmp_sysfs_dev_list;
sysfs_dev->have_driver = 0;
+ sysfs_dev->used = 0;
if (ibv_read_sysfs_file(sysfs_dev->sysfs_path, "abi_version",
value, sizeof value) > 0)
sysfs_dev->abi_ver = strtol(value, NULL, 10);
else
sysfs_dev->abi_ver = 0;
- sysfs_dev_list = sysfs_dev;
+ *tmp_sysfs_dev_list = sysfs_dev;
sysfs_dev = NULL;
}
@@ -361,8 +371,8 @@ out:
closedir(conf_dir);
}
-static struct ibv_device *try_driver(struct ibv_driver *driver,
- struct ibv_sysfs_dev *sysfs_dev)
+static struct verbs_device *try_driver(struct ibv_driver *driver,
+ struct ibv_sysfs_dev *sysfs_dev)
{
struct verbs_device *vdev;
struct ibv_device *dev;
@@ -410,14 +420,16 @@ static struct ibv_device *try_driver(struct ibv_driver *driver,
strcpy(dev->dev_path, sysfs_dev->sysfs_path);
strcpy(dev->name, sysfs_dev->ibdev_name);
strcpy(dev->ibdev_path, sysfs_dev->ibdev_path);
+ vdev->sysfs = sysfs_dev;
+ sysfs_dev->used = 1;
- return dev;
+ return vdev;
}
-static struct ibv_device *try_drivers(struct ibv_sysfs_dev *sysfs_dev)
+static struct verbs_device *try_drivers(struct ibv_sysfs_dev *sysfs_dev)
{
struct ibv_driver *driver;
- struct ibv_device *dev;
+ struct verbs_device *dev;
for (driver = head_driver; driver; driver = driver->next) {
dev = try_driver(driver, sysfs_dev);
@@ -468,45 +480,51 @@ static void check_memlock_limit(void)
rlim.rlim_cur);
}
-static void add_device(struct ibv_device *dev,
- struct ibv_device ***dev_list,
- int *num_devices,
- int *list_size)
+static int same_sysfs_dev(struct ibv_sysfs_dev *sysfs1,
+ struct ibv_sysfs_dev *sysfs2)
{
- struct ibv_device **new_list;
-
- if (*list_size <= *num_devices) {
- *list_size = *list_size ? *list_size * 2 : 1;
- new_list = realloc(*dev_list, *list_size * sizeof (struct ibv_device *));
- if (!new_list)
- return;
- *dev_list = new_list;
- }
-
- (*dev_list)[(*num_devices)++] = dev;
+ if (!strcmp(sysfs1->sysfs_name, sysfs2->sysfs_name) &&
+ ts_cmp(&sysfs1->time_created,
+ &sysfs2->time_created, ==))
+ return 1;
+ return 0;
}
-int ibverbs_get_device_list(struct ibv_device ***list)
+int ibverbs_get_device_list(struct list_head *list)
{
- struct ibv_sysfs_dev *sysfs_dev, *next_dev;
- struct ibv_device *device;
+ struct ibv_sysfs_dev *tmp_sysfs_dev_list = NULL, *sysfs_dev, *next_dev;
+ struct verbs_device *vdev, *tmp;
int num_devices = 0;
- int list_size = 0;
int statically_linked = 0;
int no_driver = 0;
int ret;
- *list = NULL;
-
- ret = find_sysfs_devs();
+ ret = find_sysfs_devs(&tmp_sysfs_dev_list);
if (ret)
return -ret;
- for (sysfs_dev = sysfs_dev_list; sysfs_dev; sysfs_dev = sysfs_dev->next) {
- device = try_drivers(sysfs_dev);
- if (device) {
- add_device(device, list, &num_devices, &list_size);
+ list_for_each_safe(list, vdev, tmp, entry) {
+ for (sysfs_dev = tmp_sysfs_dev_list; sysfs_dev; sysfs_dev =
+ sysfs_dev->next) {
+ if (same_sysfs_dev(vdev->sysfs, sysfs_dev)) {
+ sysfs_dev->have_driver = 1;
+ num_devices++;
+ break;
+ }
+ }
+ if (!sysfs_dev)
+ list_del(&vdev->entry);
+ }
+
+ for (sysfs_dev = tmp_sysfs_dev_list; sysfs_dev; sysfs_dev =
+ sysfs_dev->next) {
+ if (sysfs_dev->have_driver)
+ continue;
+ vdev = try_drivers(sysfs_dev);
+ if (vdev) {
sysfs_dev->have_driver = 1;
+ list_add(list, &vdev->entry);
+ num_devices++;
} else
no_driver = 1;
}
@@ -536,20 +554,22 @@ int ibverbs_get_device_list(struct ibv_device ***list)
load_drivers();
- for (sysfs_dev = sysfs_dev_list; sysfs_dev; sysfs_dev = sysfs_dev->next) {
+ for (sysfs_dev = tmp_sysfs_dev_list; sysfs_dev; sysfs_dev =
+ sysfs_dev->next) {
if (sysfs_dev->have_driver)
continue;
- device = try_drivers(sysfs_dev);
- if (device) {
- add_device(device, list, &num_devices, &list_size);
+ vdev = try_drivers(sysfs_dev);
+ if (vdev) {
sysfs_dev->have_driver = 1;
+ list_add(list, &vdev->entry);
+ num_devices++;
}
}
out:
- for (sysfs_dev = sysfs_dev_list,
- next_dev = sysfs_dev ? sysfs_dev->next : NULL;
+ for (sysfs_dev = tmp_sysfs_dev_list,
+ next_dev = sysfs_dev ? sysfs_dev->next : NULL;
sysfs_dev;
sysfs_dev = next_dev, next_dev = sysfs_dev ? sysfs_dev->next : NULL) {
if (!sysfs_dev->have_driver && getenv("IBV_SHOW_WARNINGS")) {
@@ -559,13 +579,14 @@ out:
fprintf(stderr, " When linking libibverbs statically, "
"driver must be statically linked too.\n");
}
- free(sysfs_dev);
+ if (!sysfs_dev->used)
+ free(sysfs_dev);
}
return num_devices;
}
-int ibverbs_init(struct ibv_device ***list)
+int ibverbs_init(void)
{
const char *sysfs_path;
int ret;
@@ -587,5 +608,5 @@ int ibverbs_init(struct ibv_device ***list)
read_config();
- return ibverbs_get_device_list(list);
+ return 0;
}
diff --git a/libibverbs/libibverbs.map b/libibverbs/libibverbs.map
index 5401241..56020d0 100644
--- a/libibverbs/libibverbs.map
+++ b/libibverbs/libibverbs.map
@@ -86,7 +86,7 @@ IBVERBS_1.1 {
/* If any symbols in this stanza change ABI then the entire staza gets a new symbol
version. Also see the private_symver() macro */
-IBVERBS_PRIVATE_14 {
+IBVERBS_PRIVATE_15 {
global:
/* These historical symbols are now private to libibverbs */
ibv_cmd_alloc_mw;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH rdma-core 4/5] verbs: Avoid ibv_device memory leak
[not found] ` <1498744816-23354-1-git-send-email-yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
` (2 preceding siblings ...)
2017-06-29 14:00 ` [PATCH rdma-core 3/5] verbs: Refresh cahced ibv_device list Yishai Hadas
@ 2017-06-29 14:00 ` Yishai Hadas
2017-06-29 14:00 ` [PATCH rdma-core 5/5] providers: Add support for uninit_device for all providers Yishai Hadas
4 siblings, 0 replies; 6+ messages in thread
From: Yishai Hadas @ 2017-06-29 14:00 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, maorg-VPRAkNaXOzVWk0Htik3J/w,
majd-VPRAkNaXOzVWk0Htik3J/w
From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
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 <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
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 b8c1fc1..b05e6a1 100644
--- a/libibverbs/device.c
+++ b/libibverbs/device.c
@@ -101,6 +101,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)
@@ -113,6 +114,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);
@@ -262,6 +267,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:
@@ -280,6 +287,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) {
@@ -294,6 +302,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 <stdatomic.h>
#include <infiniband/verbs.h>
#include <infiniband/kern-abi.h>
#include <ccan/list.h>
@@ -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);
+ }
+}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH rdma-core 5/5] providers: Add support for uninit_device for all providers
[not found] ` <1498744816-23354-1-git-send-email-yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
` (3 preceding siblings ...)
2017-06-29 14:00 ` [PATCH rdma-core 4/5] verbs: Avoid ibv_device memory leak Yishai Hadas
@ 2017-06-29 14:00 ` Yishai Hadas
4 siblings, 0 replies; 6+ messages in thread
From: Yishai Hadas @ 2017-06-29 14:00 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, maorg-VPRAkNaXOzVWk0Htik3J/w,
majd-VPRAkNaXOzVWk0Htik3J/w
From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
When libibvers calls to verbs_init_func of the provider,
then the provider allocates the verbs_device.
Add to all providers a function to free this memory once it's
called.
Signed-off-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
providers/cxgb3/iwch.c | 10 +++++++++-
providers/cxgb4/dev.c | 10 +++++++++-
providers/hfi1verbs/hfiverbs.c | 10 +++++++++-
providers/hns/hns_roce_u.c | 10 +++++++++-
providers/i40iw/i40iw_umain.c | 10 +++++++++-
providers/ipathverbs/ipathverbs.c | 10 +++++++++-
providers/mlx4/mlx4.c | 8 ++++++++
providers/mlx5/mlx5.c | 8 ++++++++
providers/mthca/mthca.c | 10 +++++++++-
providers/nes/nes_umain.c | 9 ++++++++-
providers/ocrdma/ocrdma_main.c | 10 +++++++++-
providers/qedr/qelr_main.c | 10 +++++++++-
providers/rxe/rxe.c | 8 ++++++++
providers/vmw_pvrdma/pvrdma_main.c | 10 +++++++++-
14 files changed, 122 insertions(+), 11 deletions(-)
diff --git a/providers/cxgb3/iwch.c b/providers/cxgb3/iwch.c
index be3905c..26cc7bb 100644
--- a/providers/cxgb3/iwch.c
+++ b/providers/cxgb3/iwch.c
@@ -166,9 +166,17 @@ static void iwch_free_context(struct ibv_context *ibctx)
free(context);
}
+static void iwch_uninit_device(struct verbs_device *verbs_device)
+{
+ struct iwch_device *dev = to_iwch_dev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops iwch_dev_ops = {
.alloc_context = iwch_alloc_context,
- .free_context = iwch_free_context
+ .free_context = iwch_free_context,
+ .uninit_device = iwch_uninit_device
};
static struct verbs_device *cxgb3_driver_init(const char *uverbs_sys_path,
diff --git a/providers/cxgb4/dev.c b/providers/cxgb4/dev.c
index 1474378..30182cb 100644
--- a/providers/cxgb4/dev.c
+++ b/providers/cxgb4/dev.c
@@ -220,9 +220,17 @@ static void c4iw_free_context(struct ibv_context *ibctx)
free(context);
}
+static void c4iw_uninit_device(struct verbs_device *verbs_device)
+{
+ struct c4iw_dev *dev = to_c4iw_dev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops c4iw_dev_ops = {
.alloc_context = c4iw_alloc_context,
- .free_context = c4iw_free_context
+ .free_context = c4iw_free_context,
+ .uninit_device = c4iw_uninit_device
};
#ifdef STALL_DETECTION
diff --git a/providers/hfi1verbs/hfiverbs.c b/providers/hfi1verbs/hfiverbs.c
index b27df3d..b5d0099 100644
--- a/providers/hfi1verbs/hfiverbs.c
+++ b/providers/hfi1verbs/hfiverbs.c
@@ -173,9 +173,17 @@ static void hfi1_free_context(struct ibv_context *ibctx)
free(context);
}
+static void hf11_uninit_device(struct verbs_device *verbs_device)
+{
+ struct hfi1_device *dev = to_idev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops hfi1_dev_ops = {
.alloc_context = hfi1_alloc_context,
- .free_context = hfi1_free_context
+ .free_context = hfi1_free_context,
+ .uninit_device = hf11_uninit_device
};
static struct verbs_device *hfi1_driver_init(const char *uverbs_sys_path,
diff --git a/providers/hns/hns_roce_u.c b/providers/hns/hns_roce_u.c
index a080ca6..7dae5fd 100644
--- a/providers/hns/hns_roce_u.c
+++ b/providers/hns/hns_roce_u.c
@@ -173,9 +173,17 @@ static void hns_roce_free_context(struct ibv_context *ibctx)
context = NULL;
}
+static void hns_uninit_device(struct verbs_device *verbs_device)
+{
+ struct hns_roce_device *dev = to_hr_dev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops hns_roce_dev_ops = {
.alloc_context = hns_roce_alloc_context,
- .free_context = hns_roce_free_context
+ .free_context = hns_roce_free_context,
+ .uninit_device = hns_uninit_device
};
static struct verbs_device *hns_roce_driver_init(const char *uverbs_sys_path,
diff --git a/providers/i40iw/i40iw_umain.c b/providers/i40iw/i40iw_umain.c
index 5946b7c..3b7aebb 100644
--- a/providers/i40iw/i40iw_umain.c
+++ b/providers/i40iw/i40iw_umain.c
@@ -208,9 +208,17 @@ static void i40iw_ufree_context(struct ibv_context *ibctx)
free(iwvctx);
}
+static void i40iw_uninit_device(struct verbs_device *verbs_device)
+{
+ struct i40iw_udevice *dev = to_i40iw_udev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops i40iw_udev_ops = {
.alloc_context = i40iw_ualloc_context,
- .free_context = i40iw_ufree_context
+ .free_context = i40iw_ufree_context,
+ .uninit_device = i40iw_uninit_device
};
/**
diff --git a/providers/ipathverbs/ipathverbs.c b/providers/ipathverbs/ipathverbs.c
index 0cb6abf..96bc4d7 100644
--- a/providers/ipathverbs/ipathverbs.c
+++ b/providers/ipathverbs/ipathverbs.c
@@ -172,9 +172,17 @@ static void ipath_free_context(struct ibv_context *ibctx)
free(context);
}
+static void ipath_uninit_device(struct verbs_device *verbs_device)
+{
+ struct ipath_device *dev = to_idev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops ipath_dev_ops = {
.alloc_context = ipath_alloc_context,
- .free_context = ipath_free_context
+ .free_context = ipath_free_context,
+ .uninit_device = ipath_uninit_device
};
static struct verbs_device *ipath_driver_init(const char *uverbs_sys_path,
diff --git a/providers/mlx4/mlx4.c b/providers/mlx4/mlx4.c
index b798b37..d711df3 100644
--- a/providers/mlx4/mlx4.c
+++ b/providers/mlx4/mlx4.c
@@ -262,9 +262,17 @@ static void mlx4_uninit_context(struct verbs_device *v_device,
to_mdev(&v_device->device)->page_size);
}
+static void mlx4_uninit_device(struct verbs_device *verbs_device)
+{
+ struct mlx4_device *dev = to_mdev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops mlx4_dev_ops = {
.init_context = mlx4_init_context,
.uninit_context = mlx4_uninit_context,
+ .uninit_device = mlx4_uninit_device
};
static struct verbs_device *mlx4_driver_init(const char *uverbs_sys_path, int abi_version)
diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index 99ca1ae..ede91cb 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -965,9 +965,17 @@ static void mlx5_cleanup_context(struct verbs_device *device,
close_debug_file(context);
}
+static void mlx5_uninit_device(struct verbs_device *verbs_device)
+{
+ struct mlx5_device *dev = to_mdev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops mlx5_dev_ops = {
.init_context = mlx5_init_context,
.uninit_context = mlx5_cleanup_context,
+ .uninit_device = mlx5_uninit_device
};
static struct verbs_device *mlx5_driver_init(const char *uverbs_sys_path,
diff --git a/providers/mthca/mthca.c b/providers/mthca/mthca.c
index 96e7944..62c912a 100644
--- a/providers/mthca/mthca.c
+++ b/providers/mthca/mthca.c
@@ -209,9 +209,17 @@ static void mthca_free_context(struct ibv_context *ibctx)
free(context);
}
+static void mthca_uninit_device(struct verbs_device *verbs_device)
+{
+ struct mthca_device *dev = to_mdev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops mthca_dev_ops = {
.alloc_context = mthca_alloc_context,
- .free_context = mthca_free_context
+ .free_context = mthca_free_context,
+ .uninit_device = mthca_uninit_device
};
static struct verbs_device *mthca_driver_init(const char *uverbs_sys_path,
diff --git a/providers/nes/nes_umain.c b/providers/nes/nes_umain.c
index 20ddc8d..c9ce700 100644
--- a/providers/nes/nes_umain.c
+++ b/providers/nes/nes_umain.c
@@ -184,10 +184,17 @@ static void nes_ufree_context(struct ibv_context *ibctx)
free(nesvctx);
}
+static void nes_uninit_device(struct verbs_device *verbs_device)
+{
+ struct nes_udevice *dev = to_nes_udev(&verbs_device->device);
+
+ free(dev);
+}
static struct verbs_device_ops nes_udev_ops = {
.alloc_context = nes_ualloc_context,
- .free_context = nes_ufree_context
+ .free_context = nes_ufree_context,
+ .uninit_device = nes_uninit_device
};
diff --git a/providers/ocrdma/ocrdma_main.c b/providers/ocrdma/ocrdma_main.c
index 2f22e8f..5916306 100644
--- a/providers/ocrdma/ocrdma_main.c
+++ b/providers/ocrdma/ocrdma_main.c
@@ -103,9 +103,17 @@ static struct ibv_context_ops ocrdma_ctx_ops = {
.detach_mcast = ocrdma_detach_mcast
};
+static void ocrdma_uninit_device(struct verbs_device *verbs_device)
+{
+ struct ocrdma_device *dev = get_ocrdma_dev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops ocrdma_dev_ops = {
.alloc_context = ocrdma_alloc_context,
- .free_context = ocrdma_free_context
+ .free_context = ocrdma_free_context,
+ .uninit_device = ocrdma_uninit_device
};
/*
diff --git a/providers/qedr/qelr_main.c b/providers/qedr/qelr_main.c
index 9f3feda..4d4390b 100644
--- a/providers/qedr/qelr_main.c
+++ b/providers/qedr/qelr_main.c
@@ -108,9 +108,17 @@ static struct ibv_context_ops qelr_ctx_ops = {
.async_event = qelr_async_event,
};
+static void qelr_uninit_device(struct verbs_device *verbs_device)
+{
+ struct qelr_device *dev = get_qelr_dev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops qelr_dev_ops = {
.alloc_context = qelr_alloc_context,
- .free_context = qelr_free_context
+ .free_context = qelr_free_context,
+ .uninit_device = qelr_uninit_device
};
static void qelr_open_debug_file(struct qelr_devctx *ctx)
diff --git a/providers/rxe/rxe.c b/providers/rxe/rxe.c
index 775caf0..21820f9 100644
--- a/providers/rxe/rxe.c
+++ b/providers/rxe/rxe.c
@@ -886,9 +886,17 @@ static void rxe_free_context(struct ibv_context *ibctx)
free(context);
}
+static void rxe_uninit_device(struct verbs_device *verbs_device)
+{
+ struct rxe_device *dev = to_rdev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops rxe_dev_ops = {
.alloc_context = rxe_alloc_context,
.free_context = rxe_free_context,
+ .uninit_device = rxe_uninit_device
};
static struct verbs_device *rxe_driver_init(const char *uverbs_sys_path,
diff --git a/providers/vmw_pvrdma/pvrdma_main.c b/providers/vmw_pvrdma/pvrdma_main.c
index 047f492..d388663 100644
--- a/providers/vmw_pvrdma/pvrdma_main.c
+++ b/providers/vmw_pvrdma/pvrdma_main.c
@@ -162,9 +162,17 @@ static void pvrdma_free_context(struct ibv_context *ibctx)
free(context);
}
+static void pvrdma_uninit_device(struct verbs_device *verbs_device)
+{
+ struct pvrdma_device *dev = to_vdev(&verbs_device->device);
+
+ free(dev);
+}
+
static struct verbs_device_ops pvrdma_dev_ops = {
.alloc_context = pvrdma_alloc_context,
- .free_context = pvrdma_free_context
+ .free_context = pvrdma_free_context,
+ .uninit_device = pvrdma_uninit_device
};
static struct pvrdma_device *pvrdma_driver_init_shared(
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread