Re: [PATCH rdma-core 03/13] verbs: Introduce ibv_import_device() verb

[Yishai Hadas <yishaih@dev.mellanox.co.il>]

On 6/22/2020 3:52 PM, Jason Gunthorpe wrote:
> On Sun, Jun 21, 2020 at 10:01:24AM +0300, Yishai Hadas wrote:
>> 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.
> 
> Okay, lets have a comment then, it is tricky
> 

Sure, I have updated the PR accordingly.
It includes also fixes to other notes that you already pointed on, thanks.

Yishai