[openib-general] Re: (SPAM?) [RFC] new ibv_get_devices() API -- avoid dlists
Roland Dreier
rolandd at cisco.com
Sat Nov 12 20:53:49 PST 2005
Michael> Me, I like the
Michael> int ibv_get_device_list(struct ibv_device **list[]);
Michael> better. NULL-terminated arrays are, to me, inconvenient.
I played around with a few things and came up with the interface
below. Basically, I like
struct ibv_device **ibv_get_device_list(int *num_devices);
with num_devices allowed to be NULL best. It makes it possible to get
the number of devices if you want, but puts num_devices in a less
prominent place, which makes sense because the number of devices is
secondary to the actual list of devices. And I think having a
parameter that's an "array of pointer to pointer to struct ibv_device"
is just too weird for me, in the end.
I'll cook up patches for the various users of libibverbs that I know
about (MVAPICH, Open MPI, uDAPL, ibtp) before committing this.
- R.
--- libibverbs/include/infiniband/verbs.h (revision 4024)
+++ libibverbs/include/infiniband/verbs.h (working copy)
@@ -585,9 +585,21 @@ struct ibv_context {
};
/**
- * ibv_get_devices - Return list of IB devices
+ * ibv_get_device_list - Get list of IB devices currently available
+ * @num_devices: optional. if non-NULL, set to the number of devices
+ * returned in the array.
+ *
+ * Return a NULL-terminated array of IB devices. The array can be
+ * released with ibv_free_device_list().
+ */
+extern struct ibv_device **ibv_get_device_list(int *num_devices);
+
+/**
+ * ibv_free_device_list - Free list from ibv_get_device_list()
+ *
+ * Free an array of devices returned from ibv_get_device_list()
*/
-extern struct dlist *ibv_get_devices(void);
+extern void ibv_free_device_list(struct ibv_device **list);
/**
* ibv_get_device_name - Return kernel device name
--- libibverbs/ChangeLog (revision 4024)
+++ libibverbs/ChangeLog (working copy)
@@ -1,4 +1,15 @@
-2005-11-10 Sean Hefty <sean.hefty at intel.com>
+2005-11-11 Roland Dreier <roland at cisco.com>
+
+ * examples/asyncwatch.c, examples/rc_pingpong.c,
+ examples/srq_pingpong.c, examples/uc_pingpong.c,
+ examples/ud_pingpong.c, examples/device_list.c,
+ examples/devinfo.c: Update examples to match new API.
+
+ * include/infiniband/verbs.h, src/device.c, src/init.c,
+ src/ibverbs.h: Change from dlist-based ibv_get_devices() API to
+ simpler ibv_get_device_list() and ibv_free_device_list() API.
+
+2005-11-10 Sean Hefty <sean.hefty at intel.com>
* include/infiniband/sa-kern-abi.h: New include file to contain
definitions of SA structures passed between userspace and kernel.
--- libibverbs/src/libibverbs.map (revision 4024)
+++ libibverbs/src/libibverbs.map (working copy)
@@ -1,6 +1,7 @@
IBVERBS_1.0 {
global:
- ibv_get_devices;
+ ibv_get_device_list;
+ ibv_free_device_list;
ibv_get_device_name;
ibv_get_device_guid;
ibv_open_device;
--- libibverbs/src/device.c (revision 4024)
+++ libibverbs/src/device.c (working copy)
@@ -49,21 +49,36 @@
#include "ibverbs.h"
static pthread_mutex_t device_list_lock = PTHREAD_MUTEX_INITIALIZER;
-static struct dlist *device_list;
+static int num_devices;
+static struct ibv_device **device_list;
-struct dlist *ibv_get_devices(void)
+struct ibv_device **ibv_get_device_list(int *num)
{
- struct dlist *l;
+ struct ibv_device **l;
+ int i;
pthread_mutex_lock(&device_list_lock);
- if (!device_list)
- device_list = ibverbs_init();
- l = device_list;
+
+ if (!num_devices)
+ num_devices = ibverbs_init(&device_list);
+
+ l = calloc(num_devices, sizeof (struct ibv_device *));
+ for (i = 0; i < num_devices; ++i)
+ l[i] = device_list[i];
+
pthread_mutex_unlock(&device_list_lock);
+ if (num)
+ *num = l ? num_devices : 0;
+
return l;
}
+void ibv_free_device_list(struct ibv_device **list)
+{
+ free(list);
+}
+
const char *ibv_get_device_name(struct ibv_device *device)
{
return device->ibdev->name;
--- libibverbs/src/ibverbs.h (revision 4024)
+++ libibverbs/src/ibverbs.h (working copy)
@@ -47,7 +47,8 @@
#define PFX "libibverbs: "
struct ibv_driver {
- ibv_driver_init_func init_func;
+ ibv_driver_init_func init_func;
+ struct ibv_driver *next;
};
struct ibv_abi_compat_v2 {
@@ -57,11 +58,11 @@ struct ibv_abi_compat_v2 {
extern HIDDEN int abi_ver;
-extern struct dlist *ibverbs_init(void);
+extern HIDDEN int ibverbs_init(struct ibv_device ***list);
-extern int ibv_init_mem_map(void);
-extern int ibv_lock_range(void *base, size_t size);
-extern int ibv_unlock_range(void *base, size_t size);
+extern HIDDEN int ibv_init_mem_map(void);
+extern HIDDEN int ibv_lock_range(void *base, size_t size);
+extern HIDDEN int ibv_unlock_range(void *base, size_t size);
#define IBV_INIT_CMD(cmd, size, opcode) \
do { \
--- libibverbs/src/init.c (revision 4024)
+++ libibverbs/src/init.c (working copy)
@@ -55,7 +55,7 @@ HIDDEN int abi_ver;
static char default_path[] = DRIVER_PATH;
static const char *user_path;
-static struct dlist *driver_list;
+static struct ibv_driver *driver_list;
static void load_driver(char *so_path)
{
@@ -82,7 +82,8 @@ static void load_driver(char *so_path)
}
driver->init_func = init_func;
- dlist_push(driver_list, driver);
+ driver->next = driver_list;
+ driver_list = driver;
}
static void find_drivers(char *dir)
@@ -112,8 +113,7 @@ static void find_drivers(char *dir)
load_driver(so_glob.gl_pathv[i]);
}
-static void init_drivers(struct sysfs_class_device *verbs_dev,
- struct dlist *device_list)
+static struct ibv_device *init_drivers(struct sysfs_class_device *verbs_dev)
{
struct sysfs_class_device *ib_dev;
struct sysfs_attribute *attr;
@@ -125,7 +125,7 @@ static void init_drivers(struct sysfs_cl
if (!attr) {
fprintf(stderr, PFX "Warning: no ibdev class attr for %s\n",
verbs_dev->name);
- return;
+ return NULL;
}
sscanf(attr->value, "%63s", ibdev_name);
@@ -134,19 +134,17 @@ static void init_drivers(struct sysfs_cl
if (!ib_dev) {
fprintf(stderr, PFX "Warning: no infiniband class device %s for %s\n",
attr->value, verbs_dev->name);
- return;
+ return NULL;
}
- dlist_for_each_data(driver_list, driver, struct ibv_driver) {
+ for (driver = driver_list; driver; driver = driver->next) {
dev = driver->init_func(verbs_dev);
if (dev) {
dev->dev = verbs_dev;
dev->ibdev = ib_dev;
dev->driver = driver;
- dlist_push(device_list, dev);
-
- return;
+ return dev;
}
}
@@ -155,6 +153,8 @@ static void init_drivers(struct sysfs_cl
if (user_path)
fprintf(stderr, "%s:", user_path);
fprintf(stderr, "%s\n", default_path);
+
+ return NULL;
}
static int check_abi_version(void)
@@ -188,28 +188,23 @@ static int check_abi_version(void)
}
-struct dlist *ibverbs_init(void)
+HIDDEN int ibverbs_init(struct ibv_device ***list)
{
char *wr_path, *dir;
struct sysfs_class *cls;
struct dlist *verbs_dev_list;
- struct dlist *device_list;
struct sysfs_class_device *verbs_dev;
+ struct ibv_device *device;
+ struct ibv_device **new_list;
+ int num_devices = 0;
+ int list_size = 0;
- driver_list = dlist_new(sizeof (struct ibv_driver));
- device_list = dlist_new(sizeof (struct ibv_device));
- if (!driver_list || !device_list) {
- fprintf(stderr, PFX "Fatal: couldn't allocate device/driver list.\n");
- abort();
- }
+ *list = NULL;
if (ibv_init_mem_map())
- return NULL;
+ return 0;
- /*
- * Check if a driver is statically linked, and if so load it first.
- */
- load_driver(NULL);
+ find_drivers(default_path);
/*
* Only follow the path passed in through the calling user's
@@ -224,25 +219,42 @@ struct dlist *ibverbs_init(void)
}
}
- find_drivers(default_path);
+ /*
+ * Now check if a driver is statically linked. Since we push
+ * drivers onto our driver list, the last driver we find will
+ * be the first one we try.
+ */
+ load_driver(NULL);
cls = sysfs_open_class("infiniband_verbs");
if (!cls) {
fprintf(stderr, PFX "Fatal: couldn't open sysfs class 'infiniband_verbs'.\n");
- return NULL;
+ return 0;
}
if (check_abi_version())
- return NULL;
+ return 0;
verbs_dev_list = sysfs_get_class_devices(cls);
if (!verbs_dev_list) {
fprintf(stderr, PFX "Fatal: no infiniband class devices found.\n");
- return NULL;
+ return 0;
}
- dlist_for_each_data(verbs_dev_list, verbs_dev, struct sysfs_class_device)
- init_drivers(verbs_dev, device_list);
+ dlist_for_each_data(verbs_dev_list, verbs_dev, struct sysfs_class_device) {
+ device = init_drivers(verbs_dev);
+ if (device) {
+ if (list_size <= num_devices) {
+ list_size = list_size ? list_size * 2 : 1;
+ new_list = realloc(*list, list_size * sizeof (struct ibv_device *));
+ if (!new_list)
+ goto out;
+ *list = new_list;
+ }
+ *list[num_devices++] = device;
+ }
+ }
- return device_list;
+out:
+ return num_devices;
}
--- libibverbs/examples/asyncwatch.c (revision 4024)
+++ libibverbs/examples/asyncwatch.c (working copy)
@@ -50,34 +50,30 @@ static inline uint64_t be64_to_cpu(uint6
int main(int argc, char *argv[])
{
- struct dlist *dev_list;
- struct ibv_device *ib_dev;
+ struct ibv_device **dev_list;
struct ibv_context *context;
struct ibv_async_event event;
- dev_list = ibv_get_devices();
+ dev_list = ibv_get_device_list(NULL);
if (!dev_list) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
- dlist_start(dev_list);
- ib_dev = dlist_next(dev_list);
-
- if (!ib_dev) {
+ if (!*dev_list) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
- context = ibv_open_device(ib_dev);
+ context = ibv_open_device(*dev_list);
if (!context) {
fprintf(stderr, "Couldn't get context for %s\n",
- ibv_get_device_name(ib_dev));
+ ibv_get_device_name(*dev_list));
return 1;
}
printf("%s: async event FD %d\n",
- ibv_get_device_name(ib_dev), context->async_fd);
+ ibv_get_device_name(*dev_list), context->async_fd);
while (1) {
if (ibv_get_async_event(context, &event))
--- libibverbs/examples/rc_pingpong.c (revision 4024)
+++ libibverbs/examples/rc_pingpong.c (working copy)
@@ -447,7 +447,7 @@ static void usage(const char *argv0)
int main(int argc, char *argv[])
{
- struct dlist *dev_list;
+ struct ibv_device **dev_list;
struct ibv_device *ib_dev;
struct pingpong_context *ctx;
struct pingpong_dest my_dest;
@@ -536,21 +536,20 @@ int main(int argc, char *argv[])
page_size = sysconf(_SC_PAGESIZE);
- dev_list = ibv_get_devices();
+ dev_list = ibv_get_device_list(NULL);
if (!dev_list) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
- dlist_start(dev_list);
if (!ib_devname) {
- ib_dev = dlist_next(dev_list);
+ ib_dev = *dev_list;
if (!ib_dev) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
} else {
- dlist_for_each_data(dev_list, ib_dev, struct ibv_device)
+ for (ib_dev = *dev_list; ib_dev; ++dev_list)
if (!strcmp(ibv_get_device_name(ib_dev), ib_devname))
break;
if (!ib_dev) {
--- libibverbs/examples/srq_pingpong.c (revision 4024)
+++ libibverbs/examples/srq_pingpong.c (working copy)
@@ -509,7 +509,7 @@ static void usage(const char *argv0)
int main(int argc, char *argv[])
{
- struct dlist *dev_list;
+ struct ibv_device **dev_list;
struct ibv_device *ib_dev;
struct pingpong_context *ctx;
struct pingpong_dest my_dest[MAX_QP];
@@ -605,21 +605,20 @@ int main(int argc, char *argv[])
page_size = sysconf(_SC_PAGESIZE);
- dev_list = ibv_get_devices();
+ dev_list = ibv_get_device_list(NULL);
if (!dev_list) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
- dlist_start(dev_list);
if (!ib_devname) {
- ib_dev = dlist_next(dev_list);
+ ib_dev = *dev_list;
if (!ib_dev) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
} else {
- dlist_for_each_data(dev_list, ib_dev, struct ibv_device)
+ for (ib_dev = *dev_list; ib_dev; ++dev_list)
if (!strcmp(ibv_get_device_name(ib_dev), ib_devname))
break;
if (!ib_dev) {
--- libibverbs/examples/uc_pingpong.c (revision 4024)
+++ libibverbs/examples/uc_pingpong.c (working copy)
@@ -435,7 +435,7 @@ static void usage(const char *argv0)
int main(int argc, char *argv[])
{
- struct dlist *dev_list;
+ struct ibv_device **dev_list;
struct ibv_device *ib_dev;
struct pingpong_context *ctx;
struct pingpong_dest my_dest;
@@ -524,21 +524,20 @@ int main(int argc, char *argv[])
page_size = sysconf(_SC_PAGESIZE);
- dev_list = ibv_get_devices();
+ dev_list = ibv_get_device_list(NULL);
if (!dev_list) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
- dlist_start(dev_list);
if (!ib_devname) {
- ib_dev = dlist_next(dev_list);
+ ib_dev = *dev_list;
if (!ib_dev) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
} else {
- dlist_for_each_data(dev_list, ib_dev, struct ibv_device)
+ for (ib_dev = *dev_list; ib_dev; ++dev_list)
if (!strcmp(ibv_get_device_name(ib_dev), ib_devname))
break;
if (!ib_dev) {
--- libibverbs/examples/ud_pingpong.c (revision 4024)
+++ libibverbs/examples/ud_pingpong.c (working copy)
@@ -443,7 +443,7 @@ static void usage(const char *argv0)
int main(int argc, char *argv[])
{
- struct dlist *dev_list;
+ struct ibv_device **dev_list;
struct ibv_device *ib_dev;
struct pingpong_context *ctx;
struct pingpong_dest my_dest;
@@ -532,21 +532,20 @@ int main(int argc, char *argv[])
page_size = sysconf(_SC_PAGESIZE);
- dev_list = ibv_get_devices();
+ dev_list = ibv_get_device_list(NULL);
if (!dev_list) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
- dlist_start(dev_list);
if (!ib_devname) {
- ib_dev = dlist_next(dev_list);
+ ib_dev = *dev_list;
if (!ib_dev) {
fprintf(stderr, "No IB devices found\n");
return 1;
}
} else {
- dlist_for_each_data(dev_list, ib_dev, struct ibv_device)
+ for (ib_dev = *dev_list; ib_dev; ++dev_list)
if (!strcmp(ibv_get_device_name(ib_dev), ib_devname))
break;
if (!ib_dev) {
--- libibverbs/examples/device_list.c (revision 4024)
+++ libibverbs/examples/device_list.c (working copy)
@@ -51,10 +51,9 @@ static inline uint64_t be64_to_cpu(uint6
int main(int argc, char *argv[])
{
- struct dlist *dev_list;
- struct ibv_device *ib_dev;
+ struct ibv_device **dev_list;
- dev_list = ibv_get_devices();
+ dev_list = ibv_get_device_list(NULL);
if (!dev_list) {
fprintf(stderr, "No IB devices found\n");
return 1;
@@ -63,10 +62,12 @@ int main(int argc, char *argv[])
printf(" %-16s\t node GUID\n", "device");
printf(" %-16s\t----------------\n", "------");
- dlist_for_each_data(dev_list, ib_dev, struct ibv_device)
+ while (*dev_list) {
printf(" %-16s\t%016llx\n",
- ibv_get_device_name(ib_dev),
- (unsigned long long) be64_to_cpu(ibv_get_device_guid(ib_dev)));
+ ibv_get_device_name(*dev_list),
+ (unsigned long long) be64_to_cpu(ibv_get_device_guid(*dev_list)));
+ ++dev_list;
+ }
return 0;
}
--- libibverbs/examples/devinfo.c (revision 4024)
+++ libibverbs/examples/devinfo.c (working copy)
@@ -312,8 +312,7 @@ int main(int argc, char *argv[])
{
char *ib_devname = NULL;
int ret = 0;
- struct dlist *dev_list;
- struct ibv_device *ib_dev;
+ struct ibv_device **dev_list;
int num_of_hcas;
int ib_port = 0;
@@ -350,22 +349,19 @@ int main(int argc, char *argv[])
break;
case 'l':
- dev_list = ibv_get_devices();
+ dev_list = ibv_get_device_list(&num_of_hcas);
if (!dev_list) {
fprintf(stderr, "Failed to get IB devices list");
return -1;
}
- num_of_hcas = 0;
- dlist_for_each_data(dev_list, ib_dev, struct ibv_device)
- num_of_hcas ++;
-
printf("%d HCA%s found:\n", num_of_hcas,
num_of_hcas != 1 ? "s" : "");
- dlist_start(dev_list);
- dlist_for_each_data(dev_list, ib_dev, struct ibv_device)
- printf("\t%s\n", ibv_get_device_name(ib_dev));
+ while (*dev_list) {
+ printf("\t%s\n", ibv_get_device_name(*dev_list));
+ ++dev_list;
+ }
printf("\n");
return 0;
@@ -376,28 +372,31 @@ int main(int argc, char *argv[])
}
}
- dev_list = ibv_get_devices();
+ dev_list = ibv_get_device_list(NULL);
if (!dev_list) {
fprintf(stderr, "Failed to get IB device list\n");
return -1;
}
- dlist_start(dev_list);
+
if (ib_devname) {
- dlist_for_each_data(dev_list, ib_dev, struct ibv_device)
- if (!strcmp(ibv_get_device_name(ib_dev), ib_devname))
+ while (*dev_list) {
+ if (!strcmp(ibv_get_device_name(*dev_list), ib_devname))
break;
- if (!ib_dev) {
+ ++dev_list;
+ }
+
+ if (!*dev_list) {
fprintf(stderr, "IB device '%s' wasn't found\n", ib_devname);
return -1;
}
- ret |= print_hca_cap(ib_dev, ib_port);
+
+ ret |= print_hca_cap(*dev_list, ib_port);
} else {
- ib_dev = dlist_next(dev_list);
- if (!ib_dev) {
+ if (!*dev_list) {
fprintf(stderr, "No IB devices found\n");
return -1;
}
- ret |= print_hca_cap(ib_dev, ib_port);
+ ret |= print_hca_cap(*dev_list, ib_port);
}
if (ib_devname)
More information about the general
mailing list