On 6/19/2020 3:29 PM, Jason Gunthorpe wrote:
On Wed, Jun 17, 2020 at 10:45:46AM +0300, Yishai Hadas wrote:+static struct ibv_context *verbs_import_device(int cmd_fd) +{ + struct verbs_device *verbs_device = NULL; + struct verbs_context *context_ex; + struct ibv_device **dev_list; + struct ibv_context *ctx = NULL; + struct stat st; + int i; + + if (fstat(cmd_fd, &st) || !S_ISCHR(st.st_mode)) { + errno = EINVAL; + return NULL; + } + + dev_list = ibv_get_device_list(NULL); + if (!dev_list) { + errno = ENODEV; + return NULL; + } + + for (i = 0; dev_list[i]; ++i) { + if (verbs_get_device(dev_list[i])->sysfs->sysfs_cdev == + st.st_rdev) { + verbs_device = verbs_get_device(dev_list[i]); + break; + } + }Unfortunately it looks like there is a small race here, the struct ib_uverbs_file can exist beyond the lifetime of the cdev number - the uverbs_ida is freed in ib_uverbs_remove_one() and files can still be open past that point.
Are you referring to the option that we might end up with importing a device that was already dissociated ? the below call to ops->import_context() will just fail with -EIO upon calling on this FD to the query_context method, so I believe that we should be fine here.
I guess we should add a special ioctl to return the driver_id and match that way?+ if (!verbs_device) { + errno = ENODEV; + goto out; + } + + if (!verbs_device->ops->import_context) { + errno = EOPNOTSUPP; + goto out; + } + + context_ex = verbs_device->ops->import_context(&verbs_device->device, cmd_fd); + if (!context_ex) + goto out; + + set_lib_ops(context_ex); + + ctx = &context_ex->context; +out: + ibv_free_device_list(dev_list); + return ctx; +} + +struct ibv_context *ibv_import_device(int cmd_fd) +{ + return verbs_import_device(cmd_fd); +}Why two functions? No reason for verbs_import_device()..
Sure, thanks.
Jason
