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/28 1:56, Saleem, Shiraz wrote:
>> Subject: [PATCH for-next] RDMA/core: Assign the name of device when allocating
>> ib_device
>>
>> If the name of a device is assigned during ib_register_device(), some drivers have
>> to use dev_*() for printing before register device. Bring
>> assign_name() into ib_alloc_device(), so that drivers can use ibdev_*() anywhere.
>>
>> Signed-off-by: Weihang Li <liweihang@xxxxxxxxxx>
>> ---
>>  drivers/infiniband/core/device.c               | 85 +++++++++++++-------------
>>  drivers/infiniband/hw/bnxt_re/main.c           |  4 +-
>>  drivers/infiniband/hw/cxgb4/device.c           |  2 +-
>>  drivers/infiniband/hw/cxgb4/provider.c         |  2 +-
>>  drivers/infiniband/hw/efa/efa_main.c           |  4 +-
>>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c     |  2 +-
>>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c     |  2 +-
>>  drivers/infiniband/hw/hns/hns_roce_main.c      |  2 +-
>>  drivers/infiniband/hw/i40iw/i40iw_verbs.c      |  4 +-
>>  drivers/infiniband/hw/mlx4/main.c              |  4 +-
>>  drivers/infiniband/hw/mlx5/ib_rep.c            |  8 ++-
>>  drivers/infiniband/hw/mlx5/main.c              | 18 +++---
>>  drivers/infiniband/hw/mthca/mthca_main.c       |  2 +-
>>  drivers/infiniband/hw/mthca/mthca_provider.c   |  2 +-
>>  drivers/infiniband/hw/ocrdma/ocrdma_main.c     |  4 +-
>>  drivers/infiniband/hw/qedr/main.c              |  4 +-
>>  drivers/infiniband/hw/usnic/usnic_ib_main.c    |  4 +-
>>  drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c |  4 +-
>>  drivers/infiniband/sw/rxe/rxe.c                |  4 +-
>>  drivers/infiniband/sw/rxe/rxe.h                |  2 +-
>>  drivers/infiniband/sw/rxe/rxe_net.c            |  4 +-
>>  drivers/infiniband/sw/rxe/rxe_verbs.c          |  4 +-
>>  drivers/infiniband/sw/rxe/rxe_verbs.h          |  2 +-
>>  include/rdma/ib_verbs.h                        |  8 +--
>>  24 files changed, 95 insertions(+), 86 deletions(-)
> 
> I think you ll need to update siw driver similarly.
> 
> rvt_register_device should be adapted to use the revised device registration API.
> hfi1/qib also need some rework.
> rvt_alloc_device needs to be adapted for the new one-shot 
> name + device allocation scheme.
> Hoping we can just use move the name setting from rvt_set_ibdev_name
> 

Hi Shiraz,

Sorry for missing hfi1/qib, I will try to modify as your comments.

> A few more comments below.
>> [...]
> 
>>  /**
>>   * _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
>> + * cause a unique index to be added to the passed device name.
>>   *
>>   * Low-level drivers should use ib_alloc_device() to allocate &struct
>>   * ib_device.  @size is the size of the structure to be allocated, @@ -567,7 +603,7
>> @@ static void rdma_init_coredev(struct ib_core_device *coredev,
>>   * ib_dealloc_device() must be used to free structures allocated with
>>   * ib_alloc_device().
>>   */
>> -struct ib_device *_ib_alloc_device(size_t size)
>> +struct ib_device *_ib_alloc_device(size_t size, const char *name)
>>  {
>>  	struct ib_device *device;
>>
>> @@ -586,6 +622,11 @@ struct ib_device *_ib_alloc_device(size_t size)
>>  	device->groups[0] = &ib_dev_attr_group;
>>  	rdma_init_coredev(&device->coredev, device, &init_net);
>>
>> +	if (assign_name(device, name)) {
>> +		kfree(device);
> Don't you need to do a rdma_restrack_clean here?
> 

Yes, I think so. Thanks for your reminder.

> 
>> +		return NULL;
>> +	}
>> +
>>  	INIT_LIST_HEAD(&device->event_handler_list);
>>  	spin_lock_init(&device->qp_open_list_lock);
>>  	init_rwsem(&device->event_handler_rwsem);
>> @@ -1132,40 +1173,6 @@ static __net_init int rdma_dev_init_net(struct net *net)
>>  	return ret;
>>  }
>>
> 
> [...]
> 
>> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
>> b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
>> index 1b6fb13..ccb0d70 100644
>> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
>> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
>> @@ -2692,7 +2692,7 @@ static struct i40iw_ib_device
>> *i40iw_init_rdma_device(struct i40iw_device *iwdev
>>  	struct net_device *netdev = iwdev->netdev;
>>  	struct pci_dev *pcidev = (struct pci_dev *)iwdev->hw.dev_context;
>>
>> -	iwibdev = ib_alloc_device(i40iw_ib_device, ibdev);
>> +	iwibdev = ib_alloc_device(i40iw_ib_device, ibdev, "i40iw%d");
>>  	if (!iwibdev) {
>>  		i40iw_pr_err("iwdev == NULL\n");
>>  		return NULL;
>> @@ -2780,7 +2780,7 @@ int i40iw_register_rdma_device(struct i40iw_device
>> *iwdev)
>>  	if (ret)
>>  		goto error;
>>
>> -	ret = ib_register_device(&iwibdev->ibdev, "i40iw%d");
>> +	ret = ib_register_device(&iwibdev->ibdev);
>>  	if (ret)
>>  		goto error;
>>
> 
> i40iw looks ok except for the missing underscore which I think was brought up already in another provider.
> 
> Thanks for this work!
> 
> Shiraz
> 

OK, will add it.

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