Re: [PATCH for-next] RDMA/core: Assign the name of device when allocating ib_device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020/4/29 16:37, Leon Romanovsky wrote:
> On Tue, Apr 28, 2020 at 12:39:49PM +0000, liweihang wrote:
>> On 2020/4/28 19:19, Leon Romanovsky wrote:
>>> On Tue, Apr 28, 2020 at 08:00:29AM +0000, liweihang wrote:
>>>> On 2020/4/27 20:03, Leon Romanovsky wrote:
>>>>>>>>  /**
>>>>>>>>   * _ib_alloc_device - allocate an IB device struct
>>>>>>>>   * @size:size of structure to allocate
>>>>>>>> + * @name: unique string device name. This may include a '%' which will
>>>>>>> It looks like all drivers are setting "%" in their name and "name" can
>>>>>>> be changed to be "prefix".
>>>>>> Does hfi? I thought the name was forced there for some port swapped
>>>>>> reason?
>>>>> This patch doesn't touch HFI, nothing prohibits from us to make this
>>>>> conversion work for all drivers except HFI and for the HFI add some
>>>>> different callback. There is no need to make API harder just because
>>>>> one driver needs it.
>>>>>
>>>>> Thanks
>>>>>
>>>>>> Jason
>>>>
>>>> Hi Jason and Leon,
>>>>
>>>> I missed some codes related to assign_name() in this series including
>>>> hfi/qib as Shiraz pointed. And I found a "name" without a "%" in following
>>>> funtions in core/nldev.c, and ibdev_name will be used for rxe/siw later.
>>>>
>>>> 	static int nldev_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>>>> 				  struct netlink_ext_ack *extack)
>>>> 	{
>>>> 		...
>>>>
>>>> 		nla_strlcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
>>>> 			    sizeof(ibdev_name));
>>>> 		if (strchr(ibdev_name, '%') || strlen(ibdev_name) == 0)
>>>> 			return -EINVAL;
>>>>
>>>> 		...
>>>> 	}
>>>>
>>>> I'm not familiar with these codes, but I think the judgment in assign_name()
>>>> is for the situaion like above.
>>>>
>>>> 	if (strchr(name, '%'))
>>>> 		ret = alloc_name(device, name);
>>>> 	else
>>>> 		ret = dev_set_name(&device->dev, name);
>>>>
>>>> So is it a better idea to keep using "name" instead of "prefix"?
>>>
>>> nldev_newlink() doesn't call to ib_alloc_device() and alloc_name(). The
>>> check pointed by you is for the user input.
>>>
>>
>> Hi Leon,
>>
>> nldev_newlink() will call "ops->newlink(ibdev_name, ndev)", and it point to
>> siw_newlink() in siw_main.c. And then it will call ib_alloc_device() and
>> ib_register_device().
>>
>> According to the code I pointed before, it seems that nldev_newlink()
>> expects users to input a name without '%', and then passes this name
>> to assign_name(). I think siw/rxe have to call ib_alloc_device() with
>> a name without '%', so we can't treat it as a prefix and add "_%d" to
>> it like for other drivers.
> 
> The opposite is actually true.
> 
> The reason why newlink checks for % is due to the expectation in
> alloc_name() to have a name with % for numbered devices, which is
> nice, but the better API will be to provide "prefix" and a flag
> if to append an index or not.
> 
> Thanks
> 

I see, thank you.

>>
>>>>
>>>> Thanks
>>>> Weihang
>>>
>>
> 




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux