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 <[EMAIL PROTECTED]>
+2005-11-11  Roland Dreier  <[EMAIL PROTECTED]>
+
+       * 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  <[EMAIL PROTECTED]>
 
        * 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)
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to