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

[Jason Gunthorpe <jgg@ziepe.ca>]

On Wed, Apr 29, 2020 at 09:32:16AM -0400, Dennis Dalessandro wrote:
> On 4/27/2020 8:04 PM, Jason Gunthorpe wrote:
> > On Mon, Apr 27, 2020 at 05:55:57PM +0000, 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@huawei.com>
> > > >   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.
> > 
> > It is necessary to make such a big change? :(
> > 
> > > 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
> > 
> > I thought so..
> > 
> 
> The issue is hfi1 calls into rvt_alloc_device() which then calls
> _ib_alloc_device(). We don't have the name set at that point. So the obvious
> thing to do is move the rvt_set_ibdev_name(). However there is a catch.
> 
> The name gets set after allocating the device and the device table because
> we get the unit number as part of the xa_alloc_irq(hfi1_dev_table) call
> which needs the pointer to the devdata.
> 
> One solution would be to pass in the pointer for the driver's dev table and
> let rvt_alloc_device() do the xa_alloc_irq().

Just do:

	ret = xa_alloc_irq(&hfi1_dev_table, &unit, NULL, xa_limit_32b,
			GFP_KERNEL);
        if (ret)
                return ERR_PTR(ret);

	dd = (struct hfi1_devdata *)rvt_alloc_device(sizeof(*dd) + extra,
						     nports, unit);
	if (!dd) {
		xa_erase(&hfi1_dev_table, unit);
		return ERR_PTR(-ENOMEM);
	}
	xa_store(&hfi1_dev_table, unit, dd, GFP_KERNEL);

Jason