[1/6] RDMA: Fully setup the device name in ib_register_device

Commit Message

Jason Gunthorpe Sept. 20, 2018, 10:42 p.m. UTC

From: Jason Gunthorpe <[e-mail]>

The current code has two copies of the device name, ibdev->dev and
dev_name(&ibdev->dev), and they are setup at different times, which is
very confusing.

Set them both up at the same time and make dev_name() the lead name, which
is the proper use of the driver core APIs. To make it very clear that the
name is not valid until registration pass it in to the
ib_register_device() call rather than messing with ibdev->name directly.

Also the reorganization now checks that dev_name is unique even if it does
not contain a %.

Signed-off-by: Jason Gunthorpe <[e-mail]>
---
 drivers/infiniband/core/device.c              | 35 +++++++++++--------
 drivers/infiniband/core/sysfs.c               |  4 ---
 drivers/infiniband/hw/bnxt_re/main.c          |  3 +-
 drivers/infiniband/hw/cxgb3/iwch_provider.c   |  3 +-
 drivers/infiniband/hw/cxgb4/provider.c        |  3 +-
 drivers/infiniband/hw/hfi1/init.c             |  1 -
 drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
 drivers/infiniband/hw/hns/hns_roce_main.c     |  3 +-
 drivers/infiniband/hw/i40iw/i40iw_verbs.c     |  3 +-
 drivers/infiniband/hw/mlx4/main.c             |  3 +-
 drivers/infiniband/hw/mlx5/main.c             | 15 ++++----
 drivers/infiniband/hw/mthca/mthca_provider.c  |  3 +-
 drivers/infiniband/hw/nes/nes_verbs.c         |  3 +-
 drivers/infiniband/hw/ocrdma/ocrdma_main.c    |  3 +-
 drivers/infiniband/hw/qedr/main.c             |  4 +--
 drivers/infiniband/hw/qib/qib_init.c          |  1 -
 drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
 drivers/infiniband/hw/usnic/usnic_ib_main.c   |  3 +-
 .../infiniband/hw/vmw_pvrdma/pvrdma_main.c    |  3 +-
 drivers/infiniband/sw/rdmavt/vt.c             |  6 ++--
 drivers/infiniband/sw/rxe/rxe_verbs.c         |  3 +-
 include/rdma/ib_verbs.h                       |  6 ++--
 include/rdma/rdma_vt.h                        | 16 ++-------
 23 files changed, 55 insertions(+), 77 deletions(-)

Comments

Adit Ranadive Sept. 21, 2018, 3:37 a.m. UTC | #1

> The current code has two copies of the device name, ibdev->dev and
> dev_name(&ibdev->dev), and they are setup at different times, which is
> very confusing.
> 
> Set them both up at the same time and make dev_name() the lead name, which
> is the proper use of the driver core APIs. To make it very clear that the
> name is not valid until registration pass it in to the
> ib_register_device() call rather than messing with ibdev->name directly.
> 
> Also the reorganization now checks that dev_name is unique even if it does
> not contain a %.
> 
> Signed-off-by: Jason Gunthorpe <[e-mail]>
> ---
>  drivers/infiniband/core/device.c              | 35 +++++++++++--------
>  drivers/infiniband/core/sysfs.c               |  4 ---
>  drivers/infiniband/hw/bnxt_re/main.c          |  3 +-
>  drivers/infiniband/hw/cxgb3/iwch_provider.c   |  3 +-
>  drivers/infiniband/hw/cxgb4/provider.c        |  3 +-
>  drivers/infiniband/hw/hfi1/init.c             |  1 -
>  drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
>  drivers/infiniband/hw/hns/hns_roce_main.c     |  3 +-
>  drivers/infiniband/hw/i40iw/i40iw_verbs.c     |  3 +-
>  drivers/infiniband/hw/mlx4/main.c             |  3 +-
>  drivers/infiniband/hw/mlx5/main.c             | 15 ++++----
>  drivers/infiniband/hw/mthca/mthca_provider.c  |  3 +-
>  drivers/infiniband/hw/nes/nes_verbs.c         |  3 +-
>  drivers/infiniband/hw/ocrdma/ocrdma_main.c    |  3 +-
>  drivers/infiniband/hw/qedr/main.c             |  4 +--
>  drivers/infiniband/hw/qib/qib_init.c          |  1 -
>  drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
>  drivers/infiniband/hw/usnic/usnic_ib_main.c   |  3 +-
>  .../infiniband/hw/vmw_pvrdma/pvrdma_main.c    |  3 +-
>  drivers/infiniband/sw/rdmavt/vt.c             |  6 ++--
>  drivers/infiniband/sw/rxe/rxe_verbs.c         |  3 +-
>  include/rdma/ib_verbs.h                       |  6 ++--
>  include/rdma/rdma_vt.h                        | 16 ++-------
>  23 files changed, 55 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 5a680a88aa87b7..faacf95699d794 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -170,10 +170,9 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
>          return NULL;
>  }
>  
> -static int alloc_name(char *name)
> +static int alloc_name(struct ib_device *ibdev, const char *name)
>  {
>          unsigned long *inuse;
> -       char buf[IB_DEVICE_NAME_MAX];
>          struct ib_device *device;
>          int i;
>  
> @@ -182,24 +181,21 @@ static int alloc_name(char *name)
>                  return -ENOMEM;
>  
>          list_for_each_entry(device, &device_list, core_list) {
> -               if (!sscanf(device->name, name, &i))
> +               char buf[IB_DEVICE_NAME_MAX];
> +
> +               if (sscanf(device->name, name, &i) != 1)
>                          continue;
>                  if (i < 0 || i >= PAGE_SIZE * 8)
>                          continue;
>                  snprintf(buf, sizeof buf, name, i);
> -               if (!strncmp(buf, device->name, IB_DEVICE_NAME_MAX))
> +               if (!strcmp(buf, dev_name(&device->dev)))
>                          set_bit(i, inuse);
>          }
>  
>          i = find_first_zero_bit(inuse, PAGE_SIZE * 8);
>          free_page((unsigned long) inuse);
> -       snprintf(buf, sizeof buf, name, i);
> -
> -       if (__ib_device_get_by_name(buf))
> -               return -ENFILE;
>  
> -       strlcpy(name, buf, IB_DEVICE_NAME_MAX);
> -       return 0;
> +       return dev_set_name(&ibdev->dev, name, i);
>  }
>  
>  static void ib_device_release(struct device *device)
> @@ -454,9 +450,9 @@ static u32 __dev_new_index(void)
>   * callback for each device that is added. @device must be allocated
>   * with ib_alloc_device().
>   */
> -int ib_register_device(struct ib_device *device,
> -                      int (*port_callback)(struct ib_device *,
> -                                           u8, struct kobject *))
> +int ib_register_device(struct ib_device *device, const char *name,
> +                      int (*port_callback)(struct ib_device *, u8,
> +                                           struct kobject *))
>  {
>          int ret;
>          struct ib_client *client;
> @@ -495,11 +491,20 @@ int ib_register_device(struct ib_device *device,
>  
>          mutex_lock(&device_mutex);
>  
> -       if (strchr(device->name, '%')) {
> -               ret = alloc_name(device->name);
> +       if (strchr(name, '%')) {
> +               ret = alloc_name(device, name);
> +               if (ret)
> +                       goto out;
> +       } else {
> +               ret = dev_set_name(&device->dev, name);
>                  if (ret)
>                          goto out;
>          }
> +       if (__ib_device_get_by_name(dev_name(&device->dev))) {
> +               ret = -ENFILE;
> +               goto out;
> +       }
> +       strlcpy(device->name, dev_name(&device->dev), IB_DEVICE_NAME_MAX);
>  
>          if (ib_device_check_mandatory(device)) {
>                  ret = -EINVAL;
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index 0b04dbff884f48..bc947a863b34c7 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -1311,10 +1311,6 @@ int ib_device_register_sysfs(struct ib_device *device,
>          int ret;
>          int i;
>  
> -       ret = dev_set_name(class_dev, "%s", device->name);
> -       if (ret)
> -               return ret;
> -
>          device->groups[0] = &dev_attr_group;
>          class_dev->groups = device->groups;
>  
> diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> index 20b9f31052bf97..73632e5b819f01 100644
> --- a/drivers/infiniband/hw/bnxt_re/main.c
> +++ b/drivers/infiniband/hw/bnxt_re/main.c
> @@ -579,7 +579,6 @@ static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
>          /* ib device init */
>          ibdev->owner = THIS_MODULE;
>          ibdev->node_type = RDMA_NODE_IB_CA;
> -       strlcpy(ibdev->name, "bnxt_re%d", IB_DEVICE_NAME_MAX);
>          strlcpy(ibdev->node_desc, BNXT_RE_DESC " HCA",
>                  strlen(BNXT_RE_DESC) + 5);
>          ibdev->phys_port_cnt = 1;
> @@ -672,7 +671,7 @@ static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
>          ibdev->alloc_hw_stats           = bnxt_re_ib_alloc_hw_stats;
>  
>          ibdev->driver_id = RDMA_DRIVER_BNXT_RE;
> -       return ib_register_device(ibdev, NULL);
> +       return ib_register_device(ibdev, "bnxt_re%d", NULL);
>  }
>  
>  static ssize_t show_rev(struct device *device, struct device_attribute *attr,
> diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> index 1b9ff21aa1d528..39530cc15f9588 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
> +++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> @@ -1319,7 +1319,6 @@ int iwch_register_device(struct iwch_dev *dev)
>          int i;
>  
>          pr_debug("%s iwch_dev %p\n", __func__, dev);
> -       strlcpy(dev->ibdev.name, "cxgb3_%d", IB_DEVICE_NAME_MAX);
>          memset(&dev->ibdev.node_guid, 0, sizeof(dev->ibdev.node_guid));
>          memcpy(&dev->ibdev.node_guid, dev->rdev.t3cdev_p->lldev->dev_addr, 6);
>          dev->ibdev.owner = THIS_MODULE;
> @@ -1402,7 +1401,7 @@ int iwch_register_device(struct iwch_dev *dev)
>                 sizeof(dev->ibdev.iwcm->ifname));
>  
>          dev->ibdev.driver_id = RDMA_DRIVER_CXGB3;
> -       ret = ib_register_device(&dev->ibdev, NULL);
> +       ret = ib_register_device(&dev->ibdev, "cxgb3_%d", NULL);
>          if (ret)
>                  goto bail1;
>  
> diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
> index 4eda6872e617ac..416f8d1af610ae 100644
> --- a/drivers/infiniband/hw/cxgb4/provider.c
> +++ b/drivers/infiniband/hw/cxgb4/provider.c
> @@ -535,7 +535,6 @@ void c4iw_register_device(struct work_struct *work)
>          struct c4iw_dev *dev = ctx->dev;
>  
>          pr_debug("c4iw_dev %p\n", dev);
> -       strlcpy(dev->ibdev.name, "cxgb4_%d", IB_DEVICE_NAME_MAX);
>          memset(&dev->ibdev.node_guid, 0, sizeof(dev->ibdev.node_guid));
>          memcpy(&dev->ibdev.node_guid, dev->rdev.lldi.ports[0]->dev_addr, 6);
>          dev->ibdev.owner = THIS_MODULE;
> @@ -627,7 +626,7 @@ void c4iw_register_device(struct work_struct *work)
>                 sizeof(dev->ibdev.iwcm->ifname));
>  
>          dev->ibdev.driver_id = RDMA_DRIVER_CXGB4;
> -       ret = ib_register_device(&dev->ibdev, NULL);
> +       ret = ib_register_device(&dev->ibdev, "cxgb4_%d", NULL);
>          if (ret)
>                  goto err_kfree_iwcm;
>  
> diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c
> index 1e770a1337793e..e87ff7a544eb1a 100644
> --- a/drivers/infiniband/hw/hfi1/init.c
> +++ b/drivers/infiniband/hw/hfi1/init.c
> @@ -1313,7 +1313,6 @@ static struct hfi1_devdata *hfi1_alloc_devdata(struct pci_dev *pdev,
>                          "Could not allocate unit ID: error %d\n", -ret);
>                  goto bail;
>          }
> -       rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(), dd->unit);
>  
>          /*
>           * Initialize all locks for the device. This needs to be as early as
> diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
> index 13374c727b142d..0376964a4eba15 100644
> --- a/drivers/infiniband/hw/hfi1/verbs.c
> +++ b/drivers/infiniband/hw/hfi1/verbs.c
> @@ -1843,6 +1843,7 @@ int hfi1_register_ib_device(struct hfi1_devdata *dd)
>          struct ib_device *ibdev = &dev->rdi.ibdev;
>          struct hfi1_pportdata *ppd = dd->pport;
>          struct hfi1_ibport *ibp = &ppd->ibport_data;
> +       char name[IB_DEVICE_NAME_MAX];
>          unsigned i;
>          int ret;
>  
> @@ -1961,7 +1962,8 @@ int hfi1_register_ib_device(struct hfi1_devdata *dd)
>                                i,
>                                ppd->pkeys);
>  
> -       ret = rvt_register_device(&dd->verbs_dev.rdi, RDMA_DRIVER_HFI1);
> +       snprintf(name, sizeof(name), "%s_%d", class_name(), dd->unit);
> +       ret = rvt_register_device(&dd->verbs_dev.rdi, name, RDMA_DRIVER_HFI1);
>          if (ret)
>                  goto err_verbs_txreq;
>  
> diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
> index c5cae9a38c0443..46357f94e6ef84 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_main.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
> @@ -508,7 +508,6 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>          spin_lock_init(&iboe->lock);
>  
>          ib_dev = &hr_dev->ib_dev;
> -       strlcpy(ib_dev->name, "hns_%d", IB_DEVICE_NAME_MAX);
>  
>          ib_dev->owner                   = THIS_MODULE;
>          ib_dev->node_type               = RDMA_NODE_IB_CA;
> @@ -589,7 +588,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>          ib_dev->disassociate_ucontext   = hns_roce_disassociate_ucontext;
>  
>          ib_dev->driver_id = RDMA_DRIVER_HNS;
> -       ret = ib_register_device(ib_dev, NULL);
> +       ret = ib_register_device(ib_dev, "hns_%d", NULL);
>          if (ret) {
>                  dev_err(dev, "ib_register_device failed!\n");
>                  return ret;
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> index e2e6c74a745225..cb2aef874ca85f 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> @@ -2752,7 +2752,6 @@ static struct i40iw_ib_device *i40iw_init_rdma_device(struct i40iw_device *iwdev
>                  i40iw_pr_err("iwdev == NULL\n");
>                  return NULL;
>          }
> -       strlcpy(iwibdev->ibdev.name, "i40iw%d", IB_DEVICE_NAME_MAX);
>          iwibdev->ibdev.owner = THIS_MODULE;
>          iwdev->iwibdev = iwibdev;
>          iwibdev->iwdev = iwdev;
> @@ -2897,7 +2896,7 @@ int i40iw_register_rdma_device(struct i40iw_device *iwdev)
>          iwibdev = iwdev->iwibdev;
>  
>          iwibdev->ibdev.driver_id = RDMA_DRIVER_I40IW;
> -       ret = ib_register_device(&iwibdev->ibdev, NULL);
> +       ret = ib_register_device(&iwibdev->ibdev, "i40iw%d", NULL);
>          if (ret)
>                  goto error;
>  
> diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
> index ca0f1ee26091b3..be4dcefa058377 100644
> --- a/drivers/infiniband/hw/mlx4/main.c
> +++ b/drivers/infiniband/hw/mlx4/main.c
> @@ -2634,7 +2634,6 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
>          ibdev->dev = dev;
>          ibdev->bond_next_port   = 0;
>  
> -       strlcpy(ibdev->ib_dev.name, "mlx4_%d", IB_DEVICE_NAME_MAX);
>          ibdev->ib_dev.owner             = THIS_MODULE;
>          ibdev->ib_dev.node_type         = RDMA_NODE_IB_CA;
>          ibdev->ib_dev.local_dma_lkey    = dev->caps.reserved_lkey;
> @@ -2897,7 +2896,7 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
>                  goto err_steer_free_bitmap;
>  
>          ibdev->ib_dev.driver_id = RDMA_DRIVER_MLX4;
> -       if (ib_register_device(&ibdev->ib_dev, NULL))
> +       if (ib_register_device(&ibdev->ib_dev, "mlx4_%d", NULL))
>                  goto err_diag_counters;
>  
>          if (mlx4_ib_mad_init(ibdev))
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 2be6a437755819..4e8d497d99bd5d 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -5724,7 +5724,6 @@ void mlx5_ib_stage_init_cleanup(struct mlx5_ib_dev *dev)
>  int mlx5_ib_stage_init_init(struct mlx5_ib_dev *dev)
>  {
>          struct mlx5_core_dev *mdev = dev->mdev;
> -       const char *name;
>          int err;
>          int i;
>  
> @@ -5757,12 +5756,6 @@ int mlx5_ib_stage_init_init(struct mlx5_ib_dev *dev)
>          if (mlx5_use_mad_ifc(dev))
>                  get_ext_port_caps(dev);
>  
> -       if (!mlx5_lag_is_active(mdev))
> -               name = "mlx5_%d";
> -       else
> -               name = "mlx5_bond_%d";
> -
> -       strlcpy(dev->ib_dev.name, name, IB_DEVICE_NAME_MAX);
>          dev->ib_dev.owner               = THIS_MODULE;
>          dev->ib_dev.node_type           = RDMA_NODE_IB_CA;
>          dev->ib_dev.local_dma_lkey      = 0 /* not supported for now */;
> @@ -6175,7 +6168,13 @@ static int mlx5_ib_stage_populate_specs(struct mlx5_ib_dev *dev)
>  
>  int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev *dev)
>  {
> -       return ib_register_device(&dev->ib_dev, NULL);
> +       const char *name;
> +
> +       if (!mlx5_lag_is_active(dev->mdev))
> +               name = "mlx5_%d";
> +       else
> +               name = "mlx5_bond_%d";
> +       return ib_register_device(&dev->ib_dev, name, NULL);
>  }
>  
>  void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev)
> diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
> index 0d3473b4596e16..7bd7e2ad17e4cd 100644
> --- a/drivers/infiniband/hw/mthca/mthca_provider.c
> +++ b/drivers/infiniband/hw/mthca/mthca_provider.c
> @@ -1198,7 +1198,6 @@ int mthca_register_device(struct mthca_dev *dev)
>          if (ret)
>                  return ret;
>  
> -       strlcpy(dev->ib_dev.name, "mthca%d", IB_DEVICE_NAME_MAX);
>          dev->ib_dev.owner                = THIS_MODULE;
>  
>          dev->ib_dev.uverbs_abi_ver       = MTHCA_UVERBS_ABI_VERSION;
> @@ -1297,7 +1296,7 @@ int mthca_register_device(struct mthca_dev *dev)
>          mutex_init(&dev->cap_mask_mutex);
>  
>          dev->ib_dev.driver_id = RDMA_DRIVER_MTHCA;
> -       ret = ib_register_device(&dev->ib_dev, NULL);
> +       ret = ib_register_device(&dev->ib_dev, "mthca%d", NULL);
>          if (ret)
>                  return ret;
>  
> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
> index 6940c72159610d..2127cd2f4becc1 100644
> --- a/drivers/infiniband/hw/nes/nes_verbs.c
> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
> @@ -3640,7 +3640,6 @@ struct nes_ib_device *nes_init_ofa_device(struct net_device *netdev)
>          if (nesibdev == NULL) {
>                  return NULL;
>          }
> -       strlcpy(nesibdev->ibdev.name, "nes%d", IB_DEVICE_NAME_MAX);
>          nesibdev->ibdev.owner = THIS_MODULE;
>  
>          nesibdev->ibdev.node_type = RDMA_NODE_RNIC;
> @@ -3798,7 +3797,7 @@ int nes_register_ofa_device(struct nes_ib_device *nesibdev)
>          int i, ret;
>  
>          nesvnic->nesibdev->ibdev.driver_id = RDMA_DRIVER_NES;
> -       ret = ib_register_device(&nesvnic->nesibdev->ibdev, NULL);
> +       ret = ib_register_device(&nesvnic->nesibdev->ibdev, "nes%d", NULL);
>          if (ret) {
>                  return ret;
>          }
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
> index 7832ee3e0c8491..4d3c27613351d8 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
> @@ -116,7 +116,6 @@ static void get_dev_fw_str(struct ib_device *device, char *str)
>  
>  static int ocrdma_register_device(struct ocrdma_dev *dev)
>  {
> -       strlcpy(dev->ibdev.name, "ocrdma%d", IB_DEVICE_NAME_MAX);
>          ocrdma_get_guid(dev, (u8 *)&dev->ibdev.node_guid);
>          BUILD_BUG_ON(sizeof(OCRDMA_NODE_DESC) > IB_DEVICE_NODE_DESC_MAX);
>          memcpy(dev->ibdev.node_desc, OCRDMA_NODE_DESC,
> @@ -214,7 +213,7 @@ static int ocrdma_register_device(struct ocrdma_dev *dev)
>                  dev->ibdev.post_srq_recv = ocrdma_post_srq_recv;
>          }
>          dev->ibdev.driver_id = RDMA_DRIVER_OCRDMA;
> -       return ib_register_device(&dev->ibdev, NULL);
> +       return ib_register_device(&dev->ibdev, "ocrdma%d", NULL);
>  }
>  
>  static int ocrdma_alloc_resources(struct ocrdma_dev *dev)
> diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
> index a0af6d424aeda5..cd7b8b39a12983 100644
> --- a/drivers/infiniband/hw/qedr/main.c
> +++ b/drivers/infiniband/hw/qedr/main.c
> @@ -170,8 +170,6 @@ static int qedr_register_device(struct qedr_dev *dev)
>  {
>          int rc;
>  
> -       strlcpy(dev->ibdev.name, "qedr%d", IB_DEVICE_NAME_MAX);
> -
>          dev->ibdev.node_guid = dev->attr.node_guid;
>          memcpy(dev->ibdev.node_desc, QEDR_NODE_DESC, sizeof(QEDR_NODE_DESC));
>          dev->ibdev.owner = THIS_MODULE;
> @@ -264,7 +262,7 @@ static int qedr_register_device(struct qedr_dev *dev)
>          dev->ibdev.get_dev_fw_str = qedr_get_dev_fw_str;
>  
>          dev->ibdev.driver_id = RDMA_DRIVER_QEDR;
> -       return ib_register_device(&dev->ibdev, NULL);
> +       return ib_register_device(&dev->ibdev, "qedr%d", NULL);
>  }
>  
>  /* This function allocates fast-path status block memory */
> diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
> index d7cdc77d630648..af9f4978e50b31 100644
> --- a/drivers/infiniband/hw/qib/qib_init.c
> +++ b/drivers/infiniband/hw/qib/qib_init.c
> @@ -1123,7 +1123,6 @@ struct qib_devdata *qib_alloc_devdata(struct pci_dev *pdev, size_t extra)
>                                "Could not allocate unit ID: error %d\n", -ret);
>                  goto bail;
>          }
> -       rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s%d", "qib", dd->unit);
>  
>          dd->int_counter = alloc_percpu(u64);
>          if (!dd->int_counter) {
> diff --git a/drivers/infiniband/hw/qib/qib_verbs.c b/drivers/infiniband/hw/qib/qib_verbs.c
> index 41babbc0db583c..9d0ddb42eafe97 100644
> --- a/drivers/infiniband/hw/qib/qib_verbs.c
> +++ b/drivers/infiniband/hw/qib/qib_verbs.c
> @@ -1524,6 +1524,7 @@ int qib_register_ib_device(struct qib_devdata *dd)
>          struct qib_ibdev *dev = &dd->verbs_dev;
>          struct ib_device *ibdev = &dev->rdi.ibdev;
>          struct qib_pportdata *ppd = dd->pport;
> +       char name[IB_DEVICE_NAME_MAX];
>          unsigned i, ctxt;
>          int ret;
>  
> @@ -1643,7 +1644,8 @@ int qib_register_ib_device(struct qib_devdata *dd)
>                                dd->rcd[ctxt]->pkeys);
>          }
>  
> -       ret = rvt_register_device(&dd->verbs_dev.rdi, RDMA_DRIVER_QIB);
> +       snprintf(name, sizeof(name), "%s%d", "qib", dd->unit);
> +       ret = rvt_register_device(&dd->verbs_dev.rdi, name, RDMA_DRIVER_QIB);
>          if (ret)
>                  goto err_tx;
>  
> diff --git a/drivers/infiniband/hw/usnic/usnic_ib_main.c b/drivers/infiniband/hw/usnic/usnic_ib_main.c
> index f0538a460328dd..3b9f12928314fd 100644
> --- a/drivers/infiniband/hw/usnic/usnic_ib_main.c
> +++ b/drivers/infiniband/hw/usnic/usnic_ib_main.c
> @@ -364,7 +364,6 @@ static void *usnic_ib_device_add(struct pci_dev *dev)
>          us_ibdev->ib_dev.num_comp_vectors = USNIC_IB_NUM_COMP_VECTORS;
>          us_ibdev->ib_dev.dev.parent = &dev->dev;
>          us_ibdev->ib_dev.uverbs_abi_ver = USNIC_UVERBS_ABI_VERSION;
> -       strlcpy(us_ibdev->ib_dev.name, "usnic_%d", IB_DEVICE_NAME_MAX);
>  
>          us_ibdev->ib_dev.uverbs_cmd_mask =
>                  (1ull << IB_USER_VERBS_CMD_GET_CONTEXT) |
> @@ -416,7 +415,7 @@ static void *usnic_ib_device_add(struct pci_dev *dev)
>  
>  
>          us_ibdev->ib_dev.driver_id = RDMA_DRIVER_USNIC;
> -       if (ib_register_device(&us_ibdev->ib_dev, NULL))
> +       if (ib_register_device(&us_ibdev->ib_dev, "usnic_%d", NULL))
>                  goto err_fwd_dealloc;
>  
>          usnic_fwd_set_mtu(us_ibdev->ufdev, us_ibdev->netdev->mtu);
> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> index a5719899f49ad6..6878107fc637d7 100644
> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> @@ -162,7 +162,6 @@ static int pvrdma_register_device(struct pvrdma_dev *dev)
>          int ret = -1;
>          int i = 0;
>  
> -       strlcpy(dev->ib_dev.name, "vmw_pvrdma%d", IB_DEVICE_NAME_MAX);
>          dev->ib_dev.node_guid = dev->dsr->caps.node_guid;
>          dev->sys_image_guid = dev->dsr->caps.sys_image_guid;
>          dev->flags = 0;
> @@ -267,7 +266,7 @@ static int pvrdma_register_device(struct pvrdma_dev *dev)
>          dev->ib_dev.driver_id = RDMA_DRIVER_VMW_PVRDMA;
>          spin_lock_init(&dev->srq_tbl_lock);
>  
> -       ret = ib_register_device(&dev->ib_dev, NULL);
> +       ret = ib_register_device(&dev->ib_dev, "vmw_pvrdma%d", NULL);
>          if (ret)
>                  goto err_srq_free;

Is this supposed to be complementary to Leon's device rename patch -
https://patchwork.kernel.org/patch/10607477/?

Otherwise, vmw_pvrdma looks fine.

Acked-by: Adit Ranadive <[e-mail]>

Steve Wise Sept. 21, 2018, 4:36 p.m. UTC | #2

> 
> From: Jason Gunthorpe <[e-mail]>
> 
> The current code has two copies of the device name, ibdev->dev and
> dev_name(&ibdev->dev), and they are setup at different times, which is
> very confusing.
> 
> Set them both up at the same time and make dev_name() the lead name,
> which
> is the proper use of the driver core APIs. To make it very clear that the
> name is not valid until registration pass it in to the
> ib_register_device() call rather than messing with ibdev->name directly.
> 
> Also the reorganization now checks that dev_name is unique even if it does
> not contain a %.
> 
> Signed-off-by: Jason Gunthorpe <[e-mail]>


The core and cxgb changes look fine.

Reviewed-by: Steve Wise <[e-mail]>

Leon Romanovsky Sept. 21, 2018, 7:41 p.m. UTC | #3

On Fri, Sep 21, 2018 at 03:37:23AM +0000, Adit Ranadive wrote:
> > The current code has two copies of the device name, ibdev->dev and
> > dev_name(&ibdev->dev), and they are setup at different times, which is
> > very confusing.
> > 
> > Set them both up at the same time and make dev_name() the lead name, which
> > is the proper use of the driver core APIs. To make it very clear that the
> > name is not valid until registration pass it in to the
> > ib_register_device() call rather than messing with ibdev->name directly.
> > 
> > Also the reorganization now checks that dev_name is unique even if it does
> > not contain a %.
> > 
> > Signed-off-by: Jason Gunthorpe <[e-mail]>
> > ---

<...>

>
> Is this supposed to be complementary to Leon's device rename patch -
> https://patchwork.kernel.org/patch/10607477/?
>

It wasn't designed but it looks like yes.

Both of us come to the same conclusion that alloc_names() and
ibdev->name needs to be changed, but from different reasons.

I came to this conclusion while working on IB persistence naming
task (first step device rename, second rdma-core autodiscovery
over netlink, third possible step some changes to mlx4 and
fourth change will be change of names).

And Jason came to this conclusion while tried to solve the race and
lockdep warning reported by Bart and Mark.

> Otherwise, vmw_pvrdma looks fine.
>
> Acked-by: Adit Ranadive <[e-mail]>

Devesh Sharma Sept. 22, 2018, 5:12 a.m. UTC | #4

On Sat, Sep 22, 2018 at 1:11 AM Leon Romanovsky <[e-mail]> wrote:
>
> On Fri, Sep 21, 2018 at 03:37:23AM +0000, Adit Ranadive wrote:
> > > The current code has two copies of the device name, ibdev->dev and
> > > dev_name(&ibdev->dev), and they are setup at different times, which is
> > > very confusing.
> > >
> > > Set them both up at the same time and make dev_name() the lead name, which
> > > is the proper use of the driver core APIs. To make it very clear that the
> > > name is not valid until registration pass it in to the
> > > ib_register_device() call rather than messing with ibdev->name directly.
> > >
> > > Also the reorganization now checks that dev_name is unique even if it does
> > > not contain a %.
> > >
> > > Signed-off-by: Jason Gunthorpe <[e-mail]>
> > > ---
>
> <...>
>
> >
> > Is this supposed to be complementary to Leon's device rename patch -
> > https://patchwork.kernel.org/patch/10607477/?
> >
>
> It wasn't designed but it looks like yes.
>
> Both of us come to the same conclusion that alloc_names() and
> ibdev->name needs to be changed, but from different reasons.
>
> I came to this conclusion while working on IB persistence naming
> task (first step device rename, second rdma-core autodiscovery
> over netlink, third possible step some changes to mlx4 and
> fourth change will be change of names).
>
> And Jason came to this conclusion while tried to solve the race and
> lockdep warning reported by Bart and Mark.
>
> > Otherwise, vmw_pvrdma looks fine.
> >
> > Acked-by: Adit Ranadive <[e-mail]>

Looks good for bnxt_re:
Acked-by: Devesh Sharma <[e-mail]>

Saleem, Shiraz Sept. 23, 2018, 2:16 p.m. UTC | #5

>Subject: [PATCH 1/6] RDMA: Fully setup the device name in ib_register_device
>
>From: Jason Gunthorpe <[e-mail]>
>
>The current code has two copies of the device name, ibdev->dev and
>dev_name(&ibdev->dev), and they are setup at different times, which is very
>confusing.
>
>Set them both up at the same time and make dev_name() the lead name, which
>is the proper use of the driver core APIs. To make it very clear that the name is
>not valid until registration pass it in to the
>ib_register_device() call rather than messing with ibdev->name directly.
>
>Also the reorganization now checks that dev_name is unique even if it does not
>contain a %.
>
>Signed-off-by: Jason Gunthorpe <[e-mail]>
>---
> drivers/infiniband/hw/i40iw/i40iw_verbs.c     |  3 +-

Reviewed-by: Shiraz Saleem <[e-mail]>

Ruhl, Michael J Sept. 24, 2018, 9:01 p.m. UTC | #6

>-----Original Message-----
>From: Jason Gunthorpe <[e-mail]>
>
>The current code has two copies of the device name, ibdev->dev and
>dev_name(&ibdev->dev), and they are setup at different times, which is
>very confusing.
>
>Set them both up at the same time and make dev_name() the lead name,
>which
>is the proper use of the driver core APIs. To make it very clear that the
>name is not valid until registration pass it in to the
>ib_register_device() call rather than messing with ibdev->name directly.
>
>Also the reorganization now checks that dev_name is unique even if it does
>not contain a %.
>
>Signed-off-by: Jason Gunthorpe <[e-mail]>
>---
> drivers/infiniband/hw/hfi1/init.c             |  1 -
> drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
> drivers/infiniband/hw/qib/qib_init.c          |  1 -
> drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
>diff --git a/drivers/infiniband/hw/hfi1/init.c
>b/drivers/infiniband/hw/hfi1/init.c
>index 1e770a1337793e..e87ff7a544eb1a 100644
>--- a/drivers/infiniband/hw/hfi1/init.c
>+++ b/drivers/infiniband/hw/hfi1/init.c
>@@ -1313,7 +1313,6 @@ static struct hfi1_devdata *hfi1_alloc_devdata(struct
>pci_dev *pdev,
> 			"Could not allocate unit ID: error %d\n", -ret);
> 		goto bail;
> 	}
>-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(),
>dd->unit);

If you remove this call, the HFI (and probably the QIB) dd_dev_xxx() macros will
not work correctly until we call ib_register().  Since we do a lot of work before
that, this will lose our ability to print out the device name.

I will need to look at this a bit more and see what we need to change from our
perspective to make this change work.

For the moment NAK.

Mike


>
> 	/*
> 	 * Initialize all locks for the device. This needs to be as early as
>diff --git a/drivers/infiniband/hw/hfi1/verbs.c
>b/drivers/infiniband/hw/hfi1/verbs.c
>index 13374c727b142d..0376964a4eba15 100644
>--- a/drivers/infiniband/hw/hfi1/verbs.c
>+++ b/drivers/infiniband/hw/hfi1/verbs.c
>@@ -1843,6 +1843,7 @@ int hfi1_register_ib_device(struct hfi1_devdata *dd)
> 	struct ib_device *ibdev = &dev->rdi.ibdev;
> 	struct hfi1_pportdata *ppd = dd->pport;
> 	struct hfi1_ibport *ibp = &ppd->ibport_data;
>+	char name[IB_DEVICE_NAME_MAX];
> 	unsigned i;
> 	int ret;
>
>@@ -1961,7 +1962,8 @@ int hfi1_register_ib_device(struct hfi1_devdata *dd)
> 			      i,
> 			      ppd->pkeys);
>
>-	ret = rvt_register_device(&dd->verbs_dev.rdi, RDMA_DRIVER_HFI1);
>+	snprintf(name, sizeof(name), "%s_%d", class_name(), dd->unit);
>+	ret = rvt_register_device(&dd->verbs_dev.rdi, name,
>RDMA_DRIVER_HFI1);
> 	if (ret)
> 		goto err_verbs_txreq;
>
>diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c
>b/drivers/infiniband/hw/hns/hns_roce_main.c
>index c5cae9a38c0443..46357f94e6ef84 100644
>--- a/drivers/infiniband/hw/hns/hns_roce_main.c
>+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
>@@ -508,7 +508,6 @@ static int hns_roce_register_device(struct
>hns_roce_dev *hr_dev)
> 	spin_lock_init(&iboe->lock);
>
> 	ib_dev = &hr_dev->ib_dev;
>-	strlcpy(ib_dev->name, "hns_%d", IB_DEVICE_NAME_MAX);
>
> 	ib_dev->owner			= THIS_MODULE;
> 	ib_dev->node_type		= RDMA_NODE_IB_CA;
>@@ -589,7 +588,7 @@ static int hns_roce_register_device(struct
>hns_roce_dev *hr_dev)
> 	ib_dev->disassociate_ucontext	=
>hns_roce_disassociate_ucontext;
>
> 	ib_dev->driver_id = RDMA_DRIVER_HNS;
>-	ret = ib_register_device(ib_dev, NULL);
>+	ret = ib_register_device(ib_dev, "hns_%d", NULL);
> 	if (ret) {
> 		dev_err(dev, "ib_register_device failed!\n");
> 		return ret;
>diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
>b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
>index e2e6c74a745225..cb2aef874ca85f 100644
>--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
>+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
>@@ -2752,7 +2752,6 @@ static struct i40iw_ib_device
>*i40iw_init_rdma_device(struct i40iw_device *iwdev
> 		i40iw_pr_err("iwdev == NULL\n");
> 		return NULL;
> 	}
>-	strlcpy(iwibdev->ibdev.name, "i40iw%d", IB_DEVICE_NAME_MAX);
> 	iwibdev->ibdev.owner = THIS_MODULE;
> 	iwdev->iwibdev = iwibdev;
> 	iwibdev->iwdev = iwdev;
>@@ -2897,7 +2896,7 @@ int i40iw_register_rdma_device(struct i40iw_device
>*iwdev)
> 	iwibdev = iwdev->iwibdev;
>
> 	iwibdev->ibdev.driver_id = RDMA_DRIVER_I40IW;
>-	ret = ib_register_device(&iwibdev->ibdev, NULL);
>+	ret = ib_register_device(&iwibdev->ibdev, "i40iw%d", NULL);
> 	if (ret)
> 		goto error;
>
>diff --git a/drivers/infiniband/hw/mlx4/main.c
>b/drivers/infiniband/hw/mlx4/main.c
>index ca0f1ee26091b3..be4dcefa058377 100644
>--- a/drivers/infiniband/hw/mlx4/main.c
>+++ b/drivers/infiniband/hw/mlx4/main.c
>@@ -2634,7 +2634,6 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
> 	ibdev->dev = dev;
> 	ibdev->bond_next_port	= 0;
>
>-	strlcpy(ibdev->ib_dev.name, "mlx4_%d", IB_DEVICE_NAME_MAX);
> 	ibdev->ib_dev.owner		= THIS_MODULE;
> 	ibdev->ib_dev.node_type		= RDMA_NODE_IB_CA;
> 	ibdev->ib_dev.local_dma_lkey	= dev->caps.reserved_lkey;
>@@ -2897,7 +2896,7 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
> 		goto err_steer_free_bitmap;
>
> 	ibdev->ib_dev.driver_id = RDMA_DRIVER_MLX4;
>-	if (ib_register_device(&ibdev->ib_dev, NULL))
>+	if (ib_register_device(&ibdev->ib_dev, "mlx4_%d", NULL))
> 		goto err_diag_counters;
>
> 	if (mlx4_ib_mad_init(ibdev))
>diff --git a/drivers/infiniband/hw/mlx5/main.c
>b/drivers/infiniband/hw/mlx5/main.c
>index 2be6a437755819..4e8d497d99bd5d 100644
>--- a/drivers/infiniband/hw/mlx5/main.c
>+++ b/drivers/infiniband/hw/mlx5/main.c
>@@ -5724,7 +5724,6 @@ void mlx5_ib_stage_init_cleanup(struct
>mlx5_ib_dev *dev)
> int mlx5_ib_stage_init_init(struct mlx5_ib_dev *dev)
> {
> 	struct mlx5_core_dev *mdev = dev->mdev;
>-	const char *name;
> 	int err;
> 	int i;
>
>@@ -5757,12 +5756,6 @@ int mlx5_ib_stage_init_init(struct mlx5_ib_dev
>*dev)
> 	if (mlx5_use_mad_ifc(dev))
> 		get_ext_port_caps(dev);
>
>-	if (!mlx5_lag_is_active(mdev))
>-		name = "mlx5_%d";
>-	else
>-		name = "mlx5_bond_%d";
>-
>-	strlcpy(dev->ib_dev.name, name, IB_DEVICE_NAME_MAX);
> 	dev->ib_dev.owner		= THIS_MODULE;
> 	dev->ib_dev.node_type		= RDMA_NODE_IB_CA;
> 	dev->ib_dev.local_dma_lkey	= 0 /* not supported for now */;
>@@ -6175,7 +6168,13 @@ static int mlx5_ib_stage_populate_specs(struct
>mlx5_ib_dev *dev)
>
> int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev *dev)
> {
>-	return ib_register_device(&dev->ib_dev, NULL);
>+	const char *name;
>+
>+	if (!mlx5_lag_is_active(dev->mdev))
>+		name = "mlx5_%d";
>+	else
>+		name = "mlx5_bond_%d";
>+	return ib_register_device(&dev->ib_dev, name, NULL);
> }
>
> void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev)
>diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c
>b/drivers/infiniband/hw/mthca/mthca_provider.c
>index 0d3473b4596e16..7bd7e2ad17e4cd 100644
>--- a/drivers/infiniband/hw/mthca/mthca_provider.c
>+++ b/drivers/infiniband/hw/mthca/mthca_provider.c
>@@ -1198,7 +1198,6 @@ int mthca_register_device(struct mthca_dev *dev)
> 	if (ret)
> 		return ret;
>
>-	strlcpy(dev->ib_dev.name, "mthca%d", IB_DEVICE_NAME_MAX);
> 	dev->ib_dev.owner                = THIS_MODULE;
>
> 	dev->ib_dev.uverbs_abi_ver	 = MTHCA_UVERBS_ABI_VERSION;
>@@ -1297,7 +1296,7 @@ int mthca_register_device(struct mthca_dev *dev)
> 	mutex_init(&dev->cap_mask_mutex);
>
> 	dev->ib_dev.driver_id = RDMA_DRIVER_MTHCA;
>-	ret = ib_register_device(&dev->ib_dev, NULL);
>+	ret = ib_register_device(&dev->ib_dev, "mthca%d", NULL);
> 	if (ret)
> 		return ret;
>
>diff --git a/drivers/infiniband/hw/nes/nes_verbs.c
>b/drivers/infiniband/hw/nes/nes_verbs.c
>index 6940c72159610d..2127cd2f4becc1 100644
>--- a/drivers/infiniband/hw/nes/nes_verbs.c
>+++ b/drivers/infiniband/hw/nes/nes_verbs.c
>@@ -3640,7 +3640,6 @@ struct nes_ib_device *nes_init_ofa_device(struct
>net_device *netdev)
> 	if (nesibdev == NULL) {
> 		return NULL;
> 	}
>-	strlcpy(nesibdev->ibdev.name, "nes%d", IB_DEVICE_NAME_MAX);
> 	nesibdev->ibdev.owner = THIS_MODULE;
>
> 	nesibdev->ibdev.node_type = RDMA_NODE_RNIC;
>@@ -3798,7 +3797,7 @@ int nes_register_ofa_device(struct nes_ib_device
>*nesibdev)
> 	int i, ret;
>
> 	nesvnic->nesibdev->ibdev.driver_id = RDMA_DRIVER_NES;
>-	ret = ib_register_device(&nesvnic->nesibdev->ibdev, NULL);
>+	ret = ib_register_device(&nesvnic->nesibdev->ibdev, "nes%d",
>NULL);
> 	if (ret) {
> 		return ret;
> 	}
>diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c
>b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
>index 7832ee3e0c8491..4d3c27613351d8 100644
>--- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c
>+++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
>@@ -116,7 +116,6 @@ static void get_dev_fw_str(struct ib_device *device,
>char *str)
>
> static int ocrdma_register_device(struct ocrdma_dev *dev)
> {
>-	strlcpy(dev->ibdev.name, "ocrdma%d", IB_DEVICE_NAME_MAX);
> 	ocrdma_get_guid(dev, (u8 *)&dev->ibdev.node_guid);
> 	BUILD_BUG_ON(sizeof(OCRDMA_NODE_DESC) >
>IB_DEVICE_NODE_DESC_MAX);
> 	memcpy(dev->ibdev.node_desc, OCRDMA_NODE_DESC,
>@@ -214,7 +213,7 @@ static int ocrdma_register_device(struct ocrdma_dev
>*dev)
> 		dev->ibdev.post_srq_recv = ocrdma_post_srq_recv;
> 	}
> 	dev->ibdev.driver_id = RDMA_DRIVER_OCRDMA;
>-	return ib_register_device(&dev->ibdev, NULL);
>+	return ib_register_device(&dev->ibdev, "ocrdma%d", NULL);
> }
>
> static int ocrdma_alloc_resources(struct ocrdma_dev *dev)
>diff --git a/drivers/infiniband/hw/qedr/main.c
>b/drivers/infiniband/hw/qedr/main.c
>index a0af6d424aeda5..cd7b8b39a12983 100644
>--- a/drivers/infiniband/hw/qedr/main.c
>+++ b/drivers/infiniband/hw/qedr/main.c
>@@ -170,8 +170,6 @@ static int qedr_register_device(struct qedr_dev *dev)
> {
> 	int rc;
>
>-	strlcpy(dev->ibdev.name, "qedr%d", IB_DEVICE_NAME_MAX);
>-
> 	dev->ibdev.node_guid = dev->attr.node_guid;
> 	memcpy(dev->ibdev.node_desc, QEDR_NODE_DESC,
>sizeof(QEDR_NODE_DESC));
> 	dev->ibdev.owner = THIS_MODULE;
>@@ -264,7 +262,7 @@ static int qedr_register_device(struct qedr_dev *dev)
> 	dev->ibdev.get_dev_fw_str = qedr_get_dev_fw_str;
>
> 	dev->ibdev.driver_id = RDMA_DRIVER_QEDR;
>-	return ib_register_device(&dev->ibdev, NULL);
>+	return ib_register_device(&dev->ibdev, "qedr%d", NULL);
> }
>
> /* This function allocates fast-path status block memory */
>diff --git a/drivers/infiniband/hw/qib/qib_init.c
>b/drivers/infiniband/hw/qib/qib_init.c
>index d7cdc77d630648..af9f4978e50b31 100644
>--- a/drivers/infiniband/hw/qib/qib_init.c
>+++ b/drivers/infiniband/hw/qib/qib_init.c
>@@ -1123,7 +1123,6 @@ struct qib_devdata *qib_alloc_devdata(struct
>pci_dev *pdev, size_t extra)
> 			      "Could not allocate unit ID: error %d\n", -ret);
> 		goto bail;
> 	}
>-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s%d", "qib", dd->unit);
>
> 	dd->int_counter = alloc_percpu(u64);
> 	if (!dd->int_counter) {
>diff --git a/drivers/infiniband/hw/qib/qib_verbs.c
>b/drivers/infiniband/hw/qib/qib_verbs.c
>index 41babbc0db583c..9d0ddb42eafe97 100644
>--- a/drivers/infiniband/hw/qib/qib_verbs.c
>+++ b/drivers/infiniband/hw/qib/qib_verbs.c
>@@ -1524,6 +1524,7 @@ int qib_register_ib_device(struct qib_devdata *dd)
> 	struct qib_ibdev *dev = &dd->verbs_dev;
> 	struct ib_device *ibdev = &dev->rdi.ibdev;
> 	struct qib_pportdata *ppd = dd->pport;
>+	char name[IB_DEVICE_NAME_MAX];
> 	unsigned i, ctxt;
> 	int ret;
>
>@@ -1643,7 +1644,8 @@ int qib_register_ib_device(struct qib_devdata *dd)
> 			      dd->rcd[ctxt]->pkeys);
> 	}
>
>-	ret = rvt_register_device(&dd->verbs_dev.rdi, RDMA_DRIVER_QIB);
>+	snprintf(name, sizeof(name), "%s%d", "qib", dd->unit);
>+	ret = rvt_register_device(&dd->verbs_dev.rdi, name,
>RDMA_DRIVER_QIB);
> 	if (ret)
> 		goto err_tx;
>
>diff --git a/drivers/infiniband/hw/usnic/usnic_ib_main.c
>b/drivers/infiniband/hw/usnic/usnic_ib_main.c
>index f0538a460328dd..3b9f12928314fd 100644
>--- a/drivers/infiniband/hw/usnic/usnic_ib_main.c
>+++ b/drivers/infiniband/hw/usnic/usnic_ib_main.c
>@@ -364,7 +364,6 @@ static void *usnic_ib_device_add(struct pci_dev *dev)
> 	us_ibdev->ib_dev.num_comp_vectors =
>USNIC_IB_NUM_COMP_VECTORS;
> 	us_ibdev->ib_dev.dev.parent = &dev->dev;
> 	us_ibdev->ib_dev.uverbs_abi_ver = USNIC_UVERBS_ABI_VERSION;
>-	strlcpy(us_ibdev->ib_dev.name, "usnic_%d",
>IB_DEVICE_NAME_MAX);
>
> 	us_ibdev->ib_dev.uverbs_cmd_mask =
> 		(1ull << IB_USER_VERBS_CMD_GET_CONTEXT) |
>@@ -416,7 +415,7 @@ static void *usnic_ib_device_add(struct pci_dev *dev)
>
>
> 	us_ibdev->ib_dev.driver_id = RDMA_DRIVER_USNIC;
>-	if (ib_register_device(&us_ibdev->ib_dev, NULL))
>+	if (ib_register_device(&us_ibdev->ib_dev, "usnic_%d", NULL))
> 		goto err_fwd_dealloc;
>
> 	usnic_fwd_set_mtu(us_ibdev->ufdev, us_ibdev->netdev->mtu);
>diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
>b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
>index a5719899f49ad6..6878107fc637d7 100644
>--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
>+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
>@@ -162,7 +162,6 @@ static int pvrdma_register_device(struct pvrdma_dev
>*dev)
> 	int ret = -1;
> 	int i = 0;
>
>-	strlcpy(dev->ib_dev.name, "vmw_pvrdma%d",
>IB_DEVICE_NAME_MAX);
> 	dev->ib_dev.node_guid = dev->dsr->caps.node_guid;
> 	dev->sys_image_guid = dev->dsr->caps.sys_image_guid;
> 	dev->flags = 0;
>@@ -267,7 +266,7 @@ static int pvrdma_register_device(struct pvrdma_dev
>*dev)
> 	dev->ib_dev.driver_id = RDMA_DRIVER_VMW_PVRDMA;
> 	spin_lock_init(&dev->srq_tbl_lock);
>
>-	ret = ib_register_device(&dev->ib_dev, NULL);
>+	ret = ib_register_device(&dev->ib_dev, "vmw_pvrdma%d", NULL);
> 	if (ret)
> 		goto err_srq_free;
>
>diff --git a/drivers/infiniband/sw/rdmavt/vt.c
>b/drivers/infiniband/sw/rdmavt/vt.c
>index 17e4abc067afa3..510ab492d1be57 100644
>--- a/drivers/infiniband/sw/rdmavt/vt.c
>+++ b/drivers/infiniband/sw/rdmavt/vt.c
>@@ -728,7 +728,8 @@ static noinline int check_support(struct rvt_dev_info
>*rdi, int verb)
>  *
>  * Return: 0 on success otherwise an errno.
>  */
>-int rvt_register_device(struct rvt_dev_info *rdi, u32 driver_id)
>+int rvt_register_device(struct rvt_dev_info *rdi, const char *name,
>+			u32 driver_id)
> {
> 	int ret = 0, i;
>
>@@ -828,7 +829,8 @@ int rvt_register_device(struct rvt_dev_info *rdi, u32
>driver_id)
>
> 	rdi->ibdev.driver_id = driver_id;
> 	/* We are now good to announce we exist */
>-	ret =  ib_register_device(&rdi->ibdev, rdi->driver_f.port_callback);
>+	ret = ib_register_device(&rdi->ibdev, name,
>+				 rdi->driver_f.port_callback);
> 	if (ret) {
> 		rvt_pr_err(rdi, "Failed to register driver with ib core.\n");
> 		goto bail_mr;
>diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c
>b/drivers/infiniband/sw/rxe/rxe_verbs.c
>index f5b1e0ad614204..e4da5b671e4a21 100644
>--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
>+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
>@@ -1159,7 +1159,6 @@ int rxe_register_device(struct rxe_dev *rxe)
> 	struct ib_device *dev = &rxe->ib_dev;
> 	struct crypto_shash *tfm;
>
>-	strlcpy(dev->name, "rxe%d", IB_DEVICE_NAME_MAX);
> 	strlcpy(dev->node_desc, "rxe", sizeof(dev->node_desc));
>
> 	dev->owner = THIS_MODULE;
>@@ -1261,7 +1260,7 @@ int rxe_register_device(struct rxe_dev *rxe)
> 	rxe->tfm = tfm;
>
> 	dev->driver_id = RDMA_DRIVER_RXE;
>-	err = ib_register_device(dev, NULL);
>+	err = ib_register_device(dev, "rxe%d", NULL);
> 	if (err) {
> 		pr_warn("%s failed with error %d\n", __func__, err);
> 		goto err1;
>diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>index e463d3007a356e..2096e55ff09b63 100644
>--- a/include/rdma/ib_verbs.h
>+++ b/include/rdma/ib_verbs.h
>@@ -2634,9 +2634,9 @@ void ib_dealloc_device(struct ib_device *device);
>
> void ib_get_device_fw_str(struct ib_device *device, char *str);
>
>-int ib_register_device(struct ib_device *device,
>-		       int (*port_callback)(struct ib_device *,
>-					    u8, struct kobject *));
>+int ib_register_device(struct ib_device *device, const char *name,
>+		       int (*port_callback)(struct ib_device *, u8,
>+					    struct kobject *));
> void ib_unregister_device(struct ib_device *device);
>
> int ib_register_client   (struct ib_client *client);
>diff --git a/include/rdma/rdma_vt.h b/include/rdma/rdma_vt.h
>index e32facdd9fd3d0..496f31ac9d8421 100644
>--- a/include/rdma/rdma_vt.h
>+++ b/include/rdma/rdma_vt.h
>@@ -419,19 +419,6 @@ struct rvt_dev_info {
>
> };
>
>-/**
>- * rvt_set_ibdev_name - Craft an IB device name from client info
>- * @rdi: pointer to the client rvt_dev_info structure
>- * @name: client specific name
>- * @unit: client specific unit number.
>- */
>-static inline void rvt_set_ibdev_name(struct rvt_dev_info *rdi,
>-				      const char *fmt, const char *name,
>-				      const int unit)
>-{
>-	snprintf(rdi->ibdev.name, sizeof(rdi->ibdev.name), fmt, name, unit);
>-}
>-
> /**
>  * rvt_get_ibdev_name - return the IB name
>  * @rdi: rdmavt device
>@@ -545,7 +532,8 @@ static inline void rvt_mod_retry_timer(struct rvt_qp
>*qp)
>
> struct rvt_dev_info *rvt_alloc_device(size_t size, int nports);
> void rvt_dealloc_device(struct rvt_dev_info *rdi);
>-int rvt_register_device(struct rvt_dev_info *rvd, u32 driver_id);
>+int rvt_register_device(struct rvt_dev_info *rvd, const char *name,
>+			u32 driver_id);
> void rvt_unregister_device(struct rvt_dev_info *rvd);
> int rvt_check_ah(struct ib_device *ibdev, struct rdma_ah_attr *ah_attr);
> int rvt_init_port(struct rvt_dev_info *rdi, struct rvt_ibport *port,
>--
>2.19.0

Jason Gunthorpe Sept. 24, 2018, 9:08 p.m. UTC | #7

On Mon, Sep 24, 2018 at 09:01:32PM +0000, Ruhl, Michael J wrote:
> >From: Jason Gunthorpe <[e-mail]>
> >
> >The current code has two copies of the device name, ibdev->dev and
> >dev_name(&ibdev->dev), and they are setup at different times, which is
> >very confusing.
> >
> >Set them both up at the same time and make dev_name() the lead name,
> >which
> >is the proper use of the driver core APIs. To make it very clear that the
> >name is not valid until registration pass it in to the
> >ib_register_device() call rather than messing with ibdev->name directly.
> >
> >Also the reorganization now checks that dev_name is unique even if it does
> >not contain a %.
> >
> >Signed-off-by: Jason Gunthorpe <[e-mail]>
> > drivers/infiniband/hw/hfi1/init.c             |  1 -
> > drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
> > drivers/infiniband/hw/qib/qib_init.c          |  1 -
> > drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
> >diff --git a/drivers/infiniband/hw/hfi1/init.c
> >b/drivers/infiniband/hw/hfi1/init.c
> >index 1e770a1337793e..e87ff7a544eb1a 100644
> >+++ b/drivers/infiniband/hw/hfi1/init.c
> >@@ -1313,7 +1313,6 @@ static struct hfi1_devdata *hfi1_alloc_devdata(struct
> >pci_dev *pdev,
> > 			"Could not allocate unit ID: error %d\n", -ret);
> > 		goto bail;
> > 	}
> >-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(),
> >dd->unit);
> 
> If you remove this call, the HFI (and probably the QIB) dd_dev_xxx() macros will
> not work correctly until we call ib_register().  Since we do a lot of work before
> that, this will lose our ability to print out the device name.

What prints exactly?

From what I can see the hfi driver uses macros like this:

#define dd_dev_warn(dd, fmt, ...) \
        dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \

Which uses the physical device dev for printing, not the IB device.

Generally speaking, drivers are supposed to use the physical device
for printing until registration completes, then they should use the ib
device.

Jason

Leon Romanovsky Sept. 25, 2018, 11:08 a.m. UTC | #8

On Mon, Sep 24, 2018 at 03:08:52PM -0600, Jason Gunthorpe wrote:
> On Mon, Sep 24, 2018 at 09:01:32PM +0000, Ruhl, Michael J wrote:
> > >From: Jason Gunthorpe <[e-mail]>
> > >
> > >The current code has two copies of the device name, ibdev->dev and
> > >dev_name(&ibdev->dev), and they are setup at different times, which is
> > >very confusing.
> > >
> > >Set them both up at the same time and make dev_name() the lead name,
> > >which
> > >is the proper use of the driver core APIs. To make it very clear that the
> > >name is not valid until registration pass it in to the
> > >ib_register_device() call rather than messing with ibdev->name directly.
> > >
> > >Also the reorganization now checks that dev_name is unique even if it does
> > >not contain a %.
> > >
> > >Signed-off-by: Jason Gunthorpe <[e-mail]>
> > > drivers/infiniband/hw/hfi1/init.c             |  1 -
> > > drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
> > > drivers/infiniband/hw/qib/qib_init.c          |  1 -
> > > drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
> > >diff --git a/drivers/infiniband/hw/hfi1/init.c
> > >b/drivers/infiniband/hw/hfi1/init.c
> > >index 1e770a1337793e..e87ff7a544eb1a 100644
> > >+++ b/drivers/infiniband/hw/hfi1/init.c
> > >@@ -1313,7 +1313,6 @@ static struct hfi1_devdata *hfi1_alloc_devdata(struct
> > >pci_dev *pdev,
> > > 			"Could not allocate unit ID: error %d\n", -ret);
> > > 		goto bail;
> > > 	}
> > >-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(),
> > >dd->unit);
> >
> > If you remove this call, the HFI (and probably the QIB) dd_dev_xxx() macros will
> > not work correctly until we call ib_register().  Since we do a lot of work before
> > that, this will lose our ability to print out the device name.
>
> What prints exactly?
>
> From what I can see the hfi driver uses macros like this:
>
> #define dd_dev_warn(dd, fmt, ...) \
>         dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \
>
> Which uses the physical device dev for printing, not the IB device.
>
> Generally speaking, drivers are supposed to use the physical device
> for printing until registration completes, then they should use the ib
> device.

Isn't init_name supposed to be used before ib_register_device?

Thanks

>
> Jason

Ruhl, Michael J Sept. 25, 2018, 11:28 a.m. UTC | #9

>-----Original Message-----
>From: Jason Gunthorpe [mailto:[e-mail]]
>Sent: Monday, September 24, 2018 5:09 PM
>To: Ruhl, Michael J <[e-mail]>
>Cc: [e-mail]; Doug Ledford <[e-mail]>; Selvin
>Xavier <[e-mail]>; Devesh Sharma
><[e-mail]>; Somnath Kotur
><[e-mail]>; Sriharsha Basavapatna
><[e-mail]>; Steve Wise <[e-mail]>;
>Marciniszyn, Mike <[e-mail]>; Dalessandro, Dennis
><[e-mail]>; Lijun Ou <[e-mail]>; Wei
>Hu(Xavier) <[e-mail]>; Latif, Faisal <[e-mail]>;
>Saleem, Shiraz <[e-mail]>; Yishai Hadas
><[e-mail]>; Leon Romanovsky <[e-mail]>; Michal
>Kalderon <[e-mail]>; Ariel Elior <[e-mail]>;
>Christian Benvenuti <[e-mail]>; Adit Ranadive <[e-mail]>;
>VMware PV-Drivers <[e-mail]>; Bart Van Assche
><[e-mail]>
>Subject: Re: [PATCH 1/6] RDMA: Fully setup the device name in ib_register_device
>
>On Mon, Sep 24, 2018 at 09:01:32PM +0000, Ruhl, Michael J wrote:
>> >From: Jason Gunthorpe <[e-mail]>
>> >
>> >The current code has two copies of the device name, ibdev->dev and
>> >dev_name(&ibdev->dev), and they are setup at different times, which is
>> >very confusing.
>> >
>> >Set them both up at the same time and make dev_name() the lead name,
>> >which
>> >is the proper use of the driver core APIs. To make it very clear that the
>> >name is not valid until registration pass it in to the
>> >ib_register_device() call rather than messing with ibdev->name directly.
>> >
>> >Also the reorganization now checks that dev_name is unique even if it does
>> >not contain a %.
>> >
>> >Signed-off-by: Jason Gunthorpe <[e-mail]>
>> > drivers/infiniband/hw/hfi1/init.c             |  1 -
>> > drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
>> > drivers/infiniband/hw/qib/qib_init.c          |  1 -
>> > drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
>> >diff --git a/drivers/infiniband/hw/hfi1/init.c
>> >b/drivers/infiniband/hw/hfi1/init.c
>> >index 1e770a1337793e..e87ff7a544eb1a 100644
>> >+++ b/drivers/infiniband/hw/hfi1/init.c
>> >@@ -1313,7 +1313,6 @@ static struct hfi1_devdata
>*hfi1_alloc_devdata(struct
>> >pci_dev *pdev,
>> > 			"Could not allocate unit ID: error %d\n", -ret);
>> > 		goto bail;
>> > 	}
>> >-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(),
>> >dd->unit);
>>
>> If you remove this call, the HFI (and probably the QIB) dd_dev_xxx() macros will
>> not work correctly until we call ib_register().  Since we do a lot of work before
>> that, this will lose our ability to print out the device name.
>
>What prints exactly?
>
>From what I can see the hfi driver uses macros like this:
>
>#define dd_dev_warn(dd, fmt, ...) \
>        dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \

You missed the important part of this macro: 

#define dd_dev_warn(dd, fmt, ...) \
	dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \
		 rvt_get_ibdev_name(&(dd)->verbs_dev.rdi), ##__VA_ARGS__)

The unit name is retrieved with the rvt_get_ibdev_name() function.

>Which uses the physical device dev for printing, not the IB device.
>
>Generally speaking, drivers are supposed to use the physical device
>for printing until registration completes, then they should use the ib
>device.

Yah.  Need to ponder the best way to clean this up.

M

>Jason

Leon Romanovsky Sept. 25, 2018, 11:29 a.m. UTC | #10

On Thu, Sep 20, 2018 at 04:42:22PM -0600, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <[e-mail]>
>
> The current code has two copies of the device name, ibdev->dev and
> dev_name(&ibdev->dev), and they are setup at different times, which is
> very confusing.
>
> Set them both up at the same time and make dev_name() the lead name, which
> is the proper use of the driver core APIs. To make it very clear that the
> name is not valid until registration pass it in to the
> ib_register_device() call rather than messing with ibdev->name directly.
>
> Also the reorganization now checks that dev_name is unique even if it does
> not contain a %.
>
> Signed-off-by: Jason Gunthorpe <[e-mail]>
> ---
>  drivers/infiniband/core/device.c              | 35 +++++++++++--------
>  drivers/infiniband/core/sysfs.c               |  4 ---
>  drivers/infiniband/hw/bnxt_re/main.c          |  3 +-
>  drivers/infiniband/hw/cxgb3/iwch_provider.c   |  3 +-
>  drivers/infiniband/hw/cxgb4/provider.c        |  3 +-
>  drivers/infiniband/hw/hfi1/init.c             |  1 -
>  drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
>  drivers/infiniband/hw/hns/hns_roce_main.c     |  3 +-
>  drivers/infiniband/hw/i40iw/i40iw_verbs.c     |  3 +-
>  drivers/infiniband/hw/mlx4/main.c             |  3 +-
>  drivers/infiniband/hw/mlx5/main.c             | 15 ++++----
>  drivers/infiniband/hw/mthca/mthca_provider.c  |  3 +-
>  drivers/infiniband/hw/nes/nes_verbs.c         |  3 +-
>  drivers/infiniband/hw/ocrdma/ocrdma_main.c    |  3 +-
>  drivers/infiniband/hw/qedr/main.c             |  4 +--
>  drivers/infiniband/hw/qib/qib_init.c          |  1 -
>  drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
>  drivers/infiniband/hw/usnic/usnic_ib_main.c   |  3 +-
>  .../infiniband/hw/vmw_pvrdma/pvrdma_main.c    |  3 +-
>  drivers/infiniband/sw/rdmavt/vt.c             |  6 ++--
>  drivers/infiniband/sw/rxe/rxe_verbs.c         |  3 +-
>  include/rdma/ib_verbs.h                       |  6 ++--
>  include/rdma/rdma_vt.h                        | 16 ++-------
>  23 files changed, 55 insertions(+), 77 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <[e-mail]>

Leon Romanovsky Sept. 25, 2018, 11:36 a.m. UTC | #11

On Tue, Sep 25, 2018 at 11:28:24AM +0000, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: Jason Gunthorpe [mailto:[e-mail]]
> >Sent: Monday, September 24, 2018 5:09 PM
> >To: Ruhl, Michael J <[e-mail]>
> >Cc: [e-mail]; Doug Ledford <[e-mail]>; Selvin
> >Xavier <[e-mail]>; Devesh Sharma
> ><[e-mail]>; Somnath Kotur
> ><[e-mail]>; Sriharsha Basavapatna
> ><[e-mail]>; Steve Wise <[e-mail]>;
> >Marciniszyn, Mike <[e-mail]>; Dalessandro, Dennis
> ><[e-mail]>; Lijun Ou <[e-mail]>; Wei
> >Hu(Xavier) <[e-mail]>; Latif, Faisal <[e-mail]>;
> >Saleem, Shiraz <[e-mail]>; Yishai Hadas
> ><[e-mail]>; Leon Romanovsky <[e-mail]>; Michal
> >Kalderon <[e-mail]>; Ariel Elior <[e-mail]>;
> >Christian Benvenuti <[e-mail]>; Adit Ranadive <[e-mail]>;
> >VMware PV-Drivers <[e-mail]>; Bart Van Assche
> ><[e-mail]>
> >Subject: Re: [PATCH 1/6] RDMA: Fully setup the device name in ib_register_device
> >
> >On Mon, Sep 24, 2018 at 09:01:32PM +0000, Ruhl, Michael J wrote:
> >> >From: Jason Gunthorpe <[e-mail]>
> >> >
> >> >The current code has two copies of the device name, ibdev->dev and
> >> >dev_name(&ibdev->dev), and they are setup at different times, which is
> >> >very confusing.
> >> >
> >> >Set them both up at the same time and make dev_name() the lead name,
> >> >which
> >> >is the proper use of the driver core APIs. To make it very clear that the
> >> >name is not valid until registration pass it in to the
> >> >ib_register_device() call rather than messing with ibdev->name directly.
> >> >
> >> >Also the reorganization now checks that dev_name is unique even if it does
> >> >not contain a %.
> >> >
> >> >Signed-off-by: Jason Gunthorpe <[e-mail]>
> >> > drivers/infiniband/hw/hfi1/init.c             |  1 -
> >> > drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
> >> > drivers/infiniband/hw/qib/qib_init.c          |  1 -
> >> > drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
> >> >diff --git a/drivers/infiniband/hw/hfi1/init.c
> >> >b/drivers/infiniband/hw/hfi1/init.c
> >> >index 1e770a1337793e..e87ff7a544eb1a 100644
> >> >+++ b/drivers/infiniband/hw/hfi1/init.c
> >> >@@ -1313,7 +1313,6 @@ static struct hfi1_devdata
> >*hfi1_alloc_devdata(struct
> >> >pci_dev *pdev,
> >> > 			"Could not allocate unit ID: error %d\n", -ret);
> >> > 		goto bail;
> >> > 	}
> >> >-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(),
> >> >dd->unit);
> >>
> >> If you remove this call, the HFI (and probably the QIB) dd_dev_xxx() macros will
> >> not work correctly until we call ib_register().  Since we do a lot of work before
> >> that, this will lose our ability to print out the device name.
> >
> >What prints exactly?
> >
> >From what I can see the hfi driver uses macros like this:
> >
> >#define dd_dev_warn(dd, fmt, ...) \
> >        dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \
>
> You missed the important part of this macro:
>
> #define dd_dev_warn(dd, fmt, ...) \
> 	dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \
> 		 rvt_get_ibdev_name(&(dd)->verbs_dev.rdi), ##__VA_ARGS__)
>
> The unit name is retrieved with the rvt_get_ibdev_name() function.
>
> >Which uses the physical device dev for printing, not the IB device.
> >
> >Generally speaking, drivers are supposed to use the physical device
> >for printing until registration completes, then they should use the ib
> >device.
>
> Yah.  Need to ponder the best way to clean this up.

Convert to use dev_name and set respective init_name.

Thanks

>
> M
>
> >Jason

Jason Gunthorpe Sept. 25, 2018, 2:51 p.m. UTC | #12

On Tue, Sep 25, 2018 at 11:28:24AM +0000, Ruhl, Michael J wrote:

> >> >Signed-off-by: Jason Gunthorpe <[e-mail]>
> >> > drivers/infiniband/hw/hfi1/init.c             |  1 -
> >> > drivers/infiniband/hw/hfi1/verbs.c            |  4 ++-
> >> > drivers/infiniband/hw/qib/qib_init.c          |  1 -
> >> > drivers/infiniband/hw/qib/qib_verbs.c         |  4 ++-
> >> >diff --git a/drivers/infiniband/hw/hfi1/init.c
> >> >b/drivers/infiniband/hw/hfi1/init.c
> >> >index 1e770a1337793e..e87ff7a544eb1a 100644
> >> >+++ b/drivers/infiniband/hw/hfi1/init.c
> >> >@@ -1313,7 +1313,6 @@ static struct hfi1_devdata
> >*hfi1_alloc_devdata(struct
> >> >pci_dev *pdev,
> >> > 			"Could not allocate unit ID: error %d\n", -ret);
> >> > 		goto bail;
> >> > 	}
> >> >-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(),
> >> >dd->unit);
> >>
> >> If you remove this call, the HFI (and probably the QIB) dd_dev_xxx() macros will
> >> not work correctly until we call ib_register().  Since we do a lot of work before
> >> that, this will lose our ability to print out the device name.
> >
> >What prints exactly?
> >
> >From what I can see the hfi driver uses macros like this:
> >
> >#define dd_dev_warn(dd, fmt, ...) \
> >        dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \
> 
> You missed the important part of this macro: 
> 
> #define dd_dev_warn(dd, fmt, ...) \
> 	dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \
> 		 rvt_get_ibdev_name(&(dd)->verbs_dev.rdi), ##__VA_ARGS__)
> 
> The unit name is retrieved with the rvt_get_ibdev_name() function.

So lets just keep setting dev_name early in hfi1 then.

This is really ugly BTW, drivers shouldn't be making so much stuff up
on their own..

Jason

Patch

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 5a680a88aa87b7..faacf95699d794 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -170,10 +170,9 @@  static struct ib_device *__ib_device_get_by_name(const char *name)
 	return NULL;
 }
 
-static int alloc_name(char *name)
+static int alloc_name(struct ib_device *ibdev, const char *name)
 {
 	unsigned long *inuse;
-	char buf[IB_DEVICE_NAME_MAX];
 	struct ib_device *device;
 	int i;
 
@@ -182,24 +181,21 @@  static int alloc_name(char *name)
 		return -ENOMEM;
 
 	list_for_each_entry(device, &device_list, core_list) {
-		if (!sscanf(device->name, name, &i))
+		char buf[IB_DEVICE_NAME_MAX];
+
+		if (sscanf(device->name, name, &i) != 1)
 			continue;
 		if (i < 0 || i >= PAGE_SIZE * 8)
 			continue;
 		snprintf(buf, sizeof buf, name, i);
-		if (!strncmp(buf, device->name, IB_DEVICE_NAME_MAX))
+		if (!strcmp(buf, dev_name(&device->dev)))
 			set_bit(i, inuse);
 	}
 
 	i = find_first_zero_bit(inuse, PAGE_SIZE * 8);
 	free_page((unsigned long) inuse);
-	snprintf(buf, sizeof buf, name, i);
-
-	if (__ib_device_get_by_name(buf))
-		return -ENFILE;
 
-	strlcpy(name, buf, IB_DEVICE_NAME_MAX);
-	return 0;
+	return dev_set_name(&ibdev->dev, name, i);
 }
 
 static void ib_device_release(struct device *device)
@@ -454,9 +450,9 @@  static u32 __dev_new_index(void)
  * callback for each device that is added. @device must be allocated
  * with ib_alloc_device().
  */
-int ib_register_device(struct ib_device *device,
-		       int (*port_callback)(struct ib_device *,
-					    u8, struct kobject *))
+int ib_register_device(struct ib_device *device, const char *name,
+		       int (*port_callback)(struct ib_device *, u8,
+					    struct kobject *))
 {
 	int ret;
 	struct ib_client *client;
@@ -495,11 +491,20 @@  int ib_register_device(struct ib_device *device,
 
 	mutex_lock(&device_mutex);
 
-	if (strchr(device->name, '%')) {
-		ret = alloc_name(device->name);
+	if (strchr(name, '%')) {
+		ret = alloc_name(device, name);
+		if (ret)
+			goto out;
+	} else {
+		ret = dev_set_name(&device->dev, name);
 		if (ret)
 			goto out;
 	}
+	if (__ib_device_get_by_name(dev_name(&device->dev))) {
+		ret = -ENFILE;
+		goto out;
+	}
+	strlcpy(device->name, dev_name(&device->dev), IB_DEVICE_NAME_MAX);
 
 	if (ib_device_check_mandatory(device)) {
 		ret = -EINVAL;
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index 0b04dbff884f48..bc947a863b34c7 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -1311,10 +1311,6 @@  int ib_device_register_sysfs(struct ib_device *device,
 	int ret;
 	int i;
 
-	ret = dev_set_name(class_dev, "%s", device->name);
-	if (ret)
-		return ret;
-
 	device->groups[0] = &dev_attr_group;
 	class_dev->groups = device->groups;
 
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 20b9f31052bf97..73632e5b819f01 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -579,7 +579,6 @@  static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
 	/* ib device init */
 	ibdev->owner = THIS_MODULE;
 	ibdev->node_type = RDMA_NODE_IB_CA;
-	strlcpy(ibdev->name, "bnxt_re%d", IB_DEVICE_NAME_MAX);
 	strlcpy(ibdev->node_desc, BNXT_RE_DESC " HCA",
 		strlen(BNXT_RE_DESC) + 5);
 	ibdev->phys_port_cnt = 1;
@@ -672,7 +671,7 @@  static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
 	ibdev->alloc_hw_stats           = bnxt_re_ib_alloc_hw_stats;
 
 	ibdev->driver_id = RDMA_DRIVER_BNXT_RE;
-	return ib_register_device(ibdev, NULL);
+	return ib_register_device(ibdev, "bnxt_re%d", NULL);
 }
 
 static ssize_t show_rev(struct device *device, struct device_attribute *attr,
diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index 1b9ff21aa1d528..39530cc15f9588 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -1319,7 +1319,6 @@  int iwch_register_device(struct iwch_dev *dev)
 	int i;
 
 	pr_debug("%s iwch_dev %p\n", __func__, dev);
-	strlcpy(dev->ibdev.name, "cxgb3_%d", IB_DEVICE_NAME_MAX);
 	memset(&dev->ibdev.node_guid, 0, sizeof(dev->ibdev.node_guid));
 	memcpy(&dev->ibdev.node_guid, dev->rdev.t3cdev_p->lldev->dev_addr, 6);
 	dev->ibdev.owner = THIS_MODULE;
@@ -1402,7 +1401,7 @@  int iwch_register_device(struct iwch_dev *dev)
 	       sizeof(dev->ibdev.iwcm->ifname));
 
 	dev->ibdev.driver_id = RDMA_DRIVER_CXGB3;
-	ret = ib_register_device(&dev->ibdev, NULL);
+	ret = ib_register_device(&dev->ibdev, "cxgb3_%d", NULL);
 	if (ret)
 		goto bail1;
 
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index 4eda6872e617ac..416f8d1af610ae 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -535,7 +535,6 @@  void c4iw_register_device(struct work_struct *work)
 	struct c4iw_dev *dev = ctx->dev;
 
 	pr_debug("c4iw_dev %p\n", dev);
-	strlcpy(dev->ibdev.name, "cxgb4_%d", IB_DEVICE_NAME_MAX);
 	memset(&dev->ibdev.node_guid, 0, sizeof(dev->ibdev.node_guid));
 	memcpy(&dev->ibdev.node_guid, dev->rdev.lldi.ports[0]->dev_addr, 6);
 	dev->ibdev.owner = THIS_MODULE;
@@ -627,7 +626,7 @@  void c4iw_register_device(struct work_struct *work)
 	       sizeof(dev->ibdev.iwcm->ifname));
 
 	dev->ibdev.driver_id = RDMA_DRIVER_CXGB4;
-	ret = ib_register_device(&dev->ibdev, NULL);
+	ret = ib_register_device(&dev->ibdev, "cxgb4_%d", NULL);
 	if (ret)
 		goto err_kfree_iwcm;
 
diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c
index 1e770a1337793e..e87ff7a544eb1a 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -1313,7 +1313,6 @@  static struct hfi1_devdata *hfi1_alloc_devdata(struct pci_dev *pdev,
 			"Could not allocate unit ID: error %d\n", -ret);
 		goto bail;
 	}
-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(), dd->unit);
 
 	/*
 	 * Initialize all locks for the device. This needs to be as early as
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index 13374c727b142d..0376964a4eba15 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -1843,6 +1843,7 @@  int hfi1_register_ib_device(struct hfi1_devdata *dd)
 	struct ib_device *ibdev = &dev->rdi.ibdev;
 	struct hfi1_pportdata *ppd = dd->pport;
 	struct hfi1_ibport *ibp = &ppd->ibport_data;
+	char name[IB_DEVICE_NAME_MAX];
 	unsigned i;
 	int ret;
 
@@ -1961,7 +1962,8 @@  int hfi1_register_ib_device(struct hfi1_devdata *dd)
 			      i,
 			      ppd->pkeys);
 
-	ret = rvt_register_device(&dd->verbs_dev.rdi, RDMA_DRIVER_HFI1);
+	snprintf(name, sizeof(name), "%s_%d", class_name(), dd->unit);
+	ret = rvt_register_device(&dd->verbs_dev.rdi, name, RDMA_DRIVER_HFI1);
 	if (ret)
 		goto err_verbs_txreq;
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index c5cae9a38c0443..46357f94e6ef84 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -508,7 +508,6 @@  static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
 	spin_lock_init(&iboe->lock);
 
 	ib_dev = &hr_dev->ib_dev;
-	strlcpy(ib_dev->name, "hns_%d", IB_DEVICE_NAME_MAX);
 
 	ib_dev->owner			= THIS_MODULE;
 	ib_dev->node_type		= RDMA_NODE_IB_CA;
@@ -589,7 +588,7 @@  static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
 	ib_dev->disassociate_ucontext	= hns_roce_disassociate_ucontext;
 
 	ib_dev->driver_id = RDMA_DRIVER_HNS;
-	ret = ib_register_device(ib_dev, NULL);
+	ret = ib_register_device(ib_dev, "hns_%d", NULL);
 	if (ret) {
 		dev_err(dev, "ib_register_device failed!\n");
 		return ret;
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index e2e6c74a745225..cb2aef874ca85f 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -2752,7 +2752,6 @@  static struct i40iw_ib_device *i40iw_init_rdma_device(struct i40iw_device *iwdev
 		i40iw_pr_err("iwdev == NULL\n");
 		return NULL;
 	}
-	strlcpy(iwibdev->ibdev.name, "i40iw%d", IB_DEVICE_NAME_MAX);
 	iwibdev->ibdev.owner = THIS_MODULE;
 	iwdev->iwibdev = iwibdev;
 	iwibdev->iwdev = iwdev;
@@ -2897,7 +2896,7 @@  int i40iw_register_rdma_device(struct i40iw_device *iwdev)
 	iwibdev = iwdev->iwibdev;
 
 	iwibdev->ibdev.driver_id = RDMA_DRIVER_I40IW;
-	ret = ib_register_device(&iwibdev->ibdev, NULL);
+	ret = ib_register_device(&iwibdev->ibdev, "i40iw%d", NULL);
 	if (ret)
 		goto error;
 
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index ca0f1ee26091b3..be4dcefa058377 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2634,7 +2634,6 @@  static void *mlx4_ib_add(struct mlx4_dev *dev)
 	ibdev->dev = dev;
 	ibdev->bond_next_port	= 0;
 
-	strlcpy(ibdev->ib_dev.name, "mlx4_%d", IB_DEVICE_NAME_MAX);
 	ibdev->ib_dev.owner		= THIS_MODULE;
 	ibdev->ib_dev.node_type		= RDMA_NODE_IB_CA;
 	ibdev->ib_dev.local_dma_lkey	= dev->caps.reserved_lkey;
@@ -2897,7 +2896,7 @@  static void *mlx4_ib_add(struct mlx4_dev *dev)
 		goto err_steer_free_bitmap;
 
 	ibdev->ib_dev.driver_id = RDMA_DRIVER_MLX4;
-	if (ib_register_device(&ibdev->ib_dev, NULL))
+	if (ib_register_device(&ibdev->ib_dev, "mlx4_%d", NULL))
 		goto err_diag_counters;
 
 	if (mlx4_ib_mad_init(ibdev))
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 2be6a437755819..4e8d497d99bd5d 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -5724,7 +5724,6 @@  void mlx5_ib_stage_init_cleanup(struct mlx5_ib_dev *dev)
 int mlx5_ib_stage_init_init(struct mlx5_ib_dev *dev)
 {
 	struct mlx5_core_dev *mdev = dev->mdev;
-	const char *name;
 	int err;
 	int i;
 
@@ -5757,12 +5756,6 @@  int mlx5_ib_stage_init_init(struct mlx5_ib_dev *dev)
 	if (mlx5_use_mad_ifc(dev))
 		get_ext_port_caps(dev);
 
-	if (!mlx5_lag_is_active(mdev))
-		name = "mlx5_%d";
-	else
-		name = "mlx5_bond_%d";
-
-	strlcpy(dev->ib_dev.name, name, IB_DEVICE_NAME_MAX);
 	dev->ib_dev.owner		= THIS_MODULE;
 	dev->ib_dev.node_type		= RDMA_NODE_IB_CA;
 	dev->ib_dev.local_dma_lkey	= 0 /* not supported for now */;
@@ -6175,7 +6168,13 @@  static int mlx5_ib_stage_populate_specs(struct mlx5_ib_dev *dev)
 
 int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev *dev)
 {
-	return ib_register_device(&dev->ib_dev, NULL);
+	const char *name;
+
+	if (!mlx5_lag_is_active(dev->mdev))
+		name = "mlx5_%d";
+	else
+		name = "mlx5_bond_%d";
+	return ib_register_device(&dev->ib_dev, name, NULL);
 }
 
 void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev)
diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
index 0d3473b4596e16..7bd7e2ad17e4cd 100644
--- a/drivers/infiniband/hw/mthca/mthca_provider.c
+++ b/drivers/infiniband/hw/mthca/mthca_provider.c
@@ -1198,7 +1198,6 @@  int mthca_register_device(struct mthca_dev *dev)
 	if (ret)
 		return ret;
 
-	strlcpy(dev->ib_dev.name, "mthca%d", IB_DEVICE_NAME_MAX);
 	dev->ib_dev.owner                = THIS_MODULE;
 
 	dev->ib_dev.uverbs_abi_ver	 = MTHCA_UVERBS_ABI_VERSION;
@@ -1297,7 +1296,7 @@  int mthca_register_device(struct mthca_dev *dev)
 	mutex_init(&dev->cap_mask_mutex);
 
 	dev->ib_dev.driver_id = RDMA_DRIVER_MTHCA;
-	ret = ib_register_device(&dev->ib_dev, NULL);
+	ret = ib_register_device(&dev->ib_dev, "mthca%d", NULL);
 	if (ret)
 		return ret;
 
diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index 6940c72159610d..2127cd2f4becc1 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -3640,7 +3640,6 @@  struct nes_ib_device *nes_init_ofa_device(struct net_device *netdev)
 	if (nesibdev == NULL) {
 		return NULL;
 	}
-	strlcpy(nesibdev->ibdev.name, "nes%d", IB_DEVICE_NAME_MAX);
 	nesibdev->ibdev.owner = THIS_MODULE;
 
 	nesibdev->ibdev.node_type = RDMA_NODE_RNIC;
@@ -3798,7 +3797,7 @@  int nes_register_ofa_device(struct nes_ib_device *nesibdev)
 	int i, ret;
 
 	nesvnic->nesibdev->ibdev.driver_id = RDMA_DRIVER_NES;
-	ret = ib_register_device(&nesvnic->nesibdev->ibdev, NULL);
+	ret = ib_register_device(&nesvnic->nesibdev->ibdev, "nes%d", NULL);
 	if (ret) {
 		return ret;
 	}
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
index 7832ee3e0c8491..4d3c27613351d8 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
@@ -116,7 +116,6 @@  static void get_dev_fw_str(struct ib_device *device, char *str)
 
 static int ocrdma_register_device(struct ocrdma_dev *dev)
 {
-	strlcpy(dev->ibdev.name, "ocrdma%d", IB_DEVICE_NAME_MAX);
 	ocrdma_get_guid(dev, (u8 *)&dev->ibdev.node_guid);
 	BUILD_BUG_ON(sizeof(OCRDMA_NODE_DESC) > IB_DEVICE_NODE_DESC_MAX);
 	memcpy(dev->ibdev.node_desc, OCRDMA_NODE_DESC,
@@ -214,7 +213,7 @@  static int ocrdma_register_device(struct ocrdma_dev *dev)
 		dev->ibdev.post_srq_recv = ocrdma_post_srq_recv;
 	}
 	dev->ibdev.driver_id = RDMA_DRIVER_OCRDMA;
-	return ib_register_device(&dev->ibdev, NULL);
+	return ib_register_device(&dev->ibdev, "ocrdma%d", NULL);
 }
 
 static int ocrdma_alloc_resources(struct ocrdma_dev *dev)
diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
index a0af6d424aeda5..cd7b8b39a12983 100644
--- a/drivers/infiniband/hw/qedr/main.c
+++ b/drivers/infiniband/hw/qedr/main.c
@@ -170,8 +170,6 @@  static int qedr_register_device(struct qedr_dev *dev)
 {
 	int rc;
 
-	strlcpy(dev->ibdev.name, "qedr%d", IB_DEVICE_NAME_MAX);
-
 	dev->ibdev.node_guid = dev->attr.node_guid;
 	memcpy(dev->ibdev.node_desc, QEDR_NODE_DESC, sizeof(QEDR_NODE_DESC));
 	dev->ibdev.owner = THIS_MODULE;
@@ -264,7 +262,7 @@  static int qedr_register_device(struct qedr_dev *dev)
 	dev->ibdev.get_dev_fw_str = qedr_get_dev_fw_str;
 
 	dev->ibdev.driver_id = RDMA_DRIVER_QEDR;
-	return ib_register_device(&dev->ibdev, NULL);
+	return ib_register_device(&dev->ibdev, "qedr%d", NULL);
 }
 
 /* This function allocates fast-path status block memory */
diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index d7cdc77d630648..af9f4978e50b31 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -1123,7 +1123,6 @@  struct qib_devdata *qib_alloc_devdata(struct pci_dev *pdev, size_t extra)
 			      "Could not allocate unit ID: error %d\n", -ret);
 		goto bail;
 	}
-	rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s%d", "qib", dd->unit);
 
 	dd->int_counter = alloc_percpu(u64);
 	if (!dd->int_counter) {
diff --git a/drivers/infiniband/hw/qib/qib_verbs.c b/drivers/infiniband/hw/qib/qib_verbs.c
index 41babbc0db583c..9d0ddb42eafe97 100644
--- a/drivers/infiniband/hw/qib/qib_verbs.c
+++ b/drivers/infiniband/hw/qib/qib_verbs.c
@@ -1524,6 +1524,7 @@  int qib_register_ib_device(struct qib_devdata *dd)
 	struct qib_ibdev *dev = &dd->verbs_dev;
 	struct ib_device *ibdev = &dev->rdi.ibdev;
 	struct qib_pportdata *ppd = dd->pport;
+	char name[IB_DEVICE_NAME_MAX];
 	unsigned i, ctxt;
 	int ret;
 
@@ -1643,7 +1644,8 @@  int qib_register_ib_device(struct qib_devdata *dd)
 			      dd->rcd[ctxt]->pkeys);
 	}
 
-	ret = rvt_register_device(&dd->verbs_dev.rdi, RDMA_DRIVER_QIB);
+	snprintf(name, sizeof(name), "%s%d", "qib", dd->unit);
+	ret = rvt_register_device(&dd->verbs_dev.rdi, name, RDMA_DRIVER_QIB);
 	if (ret)
 		goto err_tx;
 
diff --git a/drivers/infiniband/hw/usnic/usnic_ib_main.c b/drivers/infiniband/hw/usnic/usnic_ib_main.c
index f0538a460328dd..3b9f12928314fd 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_main.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_main.c
@@ -364,7 +364,6 @@  static void *usnic_ib_device_add(struct pci_dev *dev)
 	us_ibdev->ib_dev.num_comp_vectors = USNIC_IB_NUM_COMP_VECTORS;
 	us_ibdev->ib_dev.dev.parent = &dev->dev;
 	us_ibdev->ib_dev.uverbs_abi_ver = USNIC_UVERBS_ABI_VERSION;
-	strlcpy(us_ibdev->ib_dev.name, "usnic_%d", IB_DEVICE_NAME_MAX);
 
 	us_ibdev->ib_dev.uverbs_cmd_mask =
 		(1ull << IB_USER_VERBS_CMD_GET_CONTEXT) |
@@ -416,7 +415,7 @@  static void *usnic_ib_device_add(struct pci_dev *dev)
 
 
 	us_ibdev->ib_dev.driver_id = RDMA_DRIVER_USNIC;
-	if (ib_register_device(&us_ibdev->ib_dev, NULL))
+	if (ib_register_device(&us_ibdev->ib_dev, "usnic_%d", NULL))
 		goto err_fwd_dealloc;
 
 	usnic_fwd_set_mtu(us_ibdev->ufdev, us_ibdev->netdev->mtu);
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
index a5719899f49ad6..6878107fc637d7 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -162,7 +162,6 @@  static int pvrdma_register_device(struct pvrdma_dev *dev)
 	int ret = -1;
 	int i = 0;
 
-	strlcpy(dev->ib_dev.name, "vmw_pvrdma%d", IB_DEVICE_NAME_MAX);
 	dev->ib_dev.node_guid = dev->dsr->caps.node_guid;
 	dev->sys_image_guid = dev->dsr->caps.sys_image_guid;
 	dev->flags = 0;
@@ -267,7 +266,7 @@  static int pvrdma_register_device(struct pvrdma_dev *dev)
 	dev->ib_dev.driver_id = RDMA_DRIVER_VMW_PVRDMA;
 	spin_lock_init(&dev->srq_tbl_lock);
 
-	ret = ib_register_device(&dev->ib_dev, NULL);
+	ret = ib_register_device(&dev->ib_dev, "vmw_pvrdma%d", NULL);
 	if (ret)
 		goto err_srq_free;
 
diff --git a/drivers/infiniband/sw/rdmavt/vt.c b/drivers/infiniband/sw/rdmavt/vt.c
index 17e4abc067afa3..510ab492d1be57 100644
--- a/drivers/infiniband/sw/rdmavt/vt.c
+++ b/drivers/infiniband/sw/rdmavt/vt.c
@@ -728,7 +728,8 @@  static noinline int check_support(struct rvt_dev_info *rdi, int verb)
  *
  * Return: 0 on success otherwise an errno.
  */
-int rvt_register_device(struct rvt_dev_info *rdi, u32 driver_id)
+int rvt_register_device(struct rvt_dev_info *rdi, const char *name,
+			u32 driver_id)
 {
 	int ret = 0, i;
 
@@ -828,7 +829,8 @@  int rvt_register_device(struct rvt_dev_info *rdi, u32 driver_id)
 
 	rdi->ibdev.driver_id = driver_id;
 	/* We are now good to announce we exist */
-	ret =  ib_register_device(&rdi->ibdev, rdi->driver_f.port_callback);
+	ret = ib_register_device(&rdi->ibdev, name,
+				 rdi->driver_f.port_callback);
 	if (ret) {
 		rvt_pr_err(rdi, "Failed to register driver with ib core.\n");
 		goto bail_mr;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index f5b1e0ad614204..e4da5b671e4a21 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1159,7 +1159,6 @@  int rxe_register_device(struct rxe_dev *rxe)
 	struct ib_device *dev = &rxe->ib_dev;
 	struct crypto_shash *tfm;
 
-	strlcpy(dev->name, "rxe%d", IB_DEVICE_NAME_MAX);
 	strlcpy(dev->node_desc, "rxe", sizeof(dev->node_desc));
 
 	dev->owner = THIS_MODULE;
@@ -1261,7 +1260,7 @@  int rxe_register_device(struct rxe_dev *rxe)
 	rxe->tfm = tfm;
 
 	dev->driver_id = RDMA_DRIVER_RXE;
-	err = ib_register_device(dev, NULL);
+	err = ib_register_device(dev, "rxe%d", NULL);
 	if (err) {
 		pr_warn("%s failed with error %d\n", __func__, err);
 		goto err1;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index e463d3007a356e..2096e55ff09b63 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2634,9 +2634,9 @@  void ib_dealloc_device(struct ib_device *device);
 
 void ib_get_device_fw_str(struct ib_device *device, char *str);
 
-int ib_register_device(struct ib_device *device,
-		       int (*port_callback)(struct ib_device *,
-					    u8, struct kobject *));
+int ib_register_device(struct ib_device *device, const char *name,
+		       int (*port_callback)(struct ib_device *, u8,
+					    struct kobject *));
 void ib_unregister_device(struct ib_device *device);
 
 int ib_register_client   (struct ib_client *client);
diff --git a/include/rdma/rdma_vt.h b/include/rdma/rdma_vt.h
index e32facdd9fd3d0..496f31ac9d8421 100644
--- a/include/rdma/rdma_vt.h
+++ b/include/rdma/rdma_vt.h
@@ -419,19 +419,6 @@  struct rvt_dev_info {
 
 };
 
-/**
- * rvt_set_ibdev_name - Craft an IB device name from client info
- * @rdi: pointer to the client rvt_dev_info structure
- * @name: client specific name
- * @unit: client specific unit number.
- */
-static inline void rvt_set_ibdev_name(struct rvt_dev_info *rdi,
-				      const char *fmt, const char *name,
-				      const int unit)
-{
-	snprintf(rdi->ibdev.name, sizeof(rdi->ibdev.name), fmt, name, unit);
-}
-
 /**
  * rvt_get_ibdev_name - return the IB name
  * @rdi: rdmavt device
@@ -545,7 +532,8 @@  static inline void rvt_mod_retry_timer(struct rvt_qp *qp)
 
 struct rvt_dev_info *rvt_alloc_device(size_t size, int nports);
 void rvt_dealloc_device(struct rvt_dev_info *rdi);
-int rvt_register_device(struct rvt_dev_info *rvd, u32 driver_id);
+int rvt_register_device(struct rvt_dev_info *rvd, const char *name,
+			u32 driver_id);
 void rvt_unregister_device(struct rvt_dev_info *rvd);
 int rvt_check_ah(struct ib_device *ibdev, struct rdma_ah_attr *ah_attr);
 int rvt_init_port(struct rvt_dev_info *rdi, struct rvt_ibport *port,