Re: [PATCH] RDMA/device: Add ib_device_get_by_name() and use it in rxe

[Yanjun Zhu <yanjun.zhu@xxxxxxxxxx>]

When apply this patch to v5.0-rc2, the following will appear.

patching file include/rdma/ib_verbs.h
Hunk #1 FAILED at 3944.
1 out of 1 hunk FAILED -- saving rejects to file include/rdma/ib_verbs.h.rej

It seems that "ib_device_put" does not exist in v5.0-rc2

Zhu Yanjun

On 2019/1/12 10:37, Jason Gunthorpe wrote:
> rxe has an open coded version of this that is not as safe as the core
> version can be.
>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> ---
>   drivers/infiniband/core/device.c      | 28 +++++++++++++++++++++++++++
>   drivers/infiniband/sw/rxe/rxe.h       |  1 -
>   drivers/infiniband/sw/rxe/rxe_net.c   | 17 ----------------
>   drivers/infiniband/sw/rxe/rxe_sysfs.c |  8 +++++---
>   include/rdma/ib_verbs.h               |  2 ++
>   5 files changed, 35 insertions(+), 21 deletions(-)
>
> More usage of device_try_get/put()
>
> Requires the advise_mr series sent to for-rc
>
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 9b5c72d3c85a88..1311d6e5f28c03 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -181,6 +181,34 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
>   	return NULL;
>   }
>   
> +/**
> + * ib_device_get_by_name - Find an IB device by name
> + * @name: The name to look for
> + * @driver_id: The driver ID that must match (RDMA_DRIVER_UNKNOWN matches all)
> + *
> + * Find and hold an ib_device by its name. The caller must call
> + * ib_device_put() on the returned pointer.
> + */
> +struct ib_device *ib_device_get_by_name(const char *name,
> +					enum rdma_driver_id driver_id)
> +{
> +	struct ib_device *device;
> +
> +	down_read(&lists_rwsem);
> +	device = __ib_device_get_by_name(name);
> +	if (device && driver_id != RDMA_DRIVER_UNKNOWN &&
> +	    device->driver_id != driver_id)
> +		device = NULL;
> +
> +	if (device) {
> +		if (!ib_device_try_get(device))
> +			device = NULL;
> +	}
> +	up_read(&lists_rwsem);
> +	return device;
> +}
> +EXPORT_SYMBOL(ib_device_get_by_name);
> +
>   int ib_device_rename(struct ib_device *ibdev, const char *name)
>   {
>   	struct ib_device *device;
> diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
> index 5bde2ad964d277..4f8653c5d1ca5e 100644
> --- a/drivers/infiniband/sw/rxe/rxe.h
> +++ b/drivers/infiniband/sw/rxe/rxe.h
> @@ -106,7 +106,6 @@ static inline void rxe_dev_put(struct rxe_dev *rxe)
>   	kref_put(&rxe->ref_cnt, rxe_release);
>   }
>   struct rxe_dev *net_to_rxe(struct net_device *ndev);
> -struct rxe_dev *get_rxe_by_name(const char *name);
>   
>   void rxe_port_up(struct rxe_dev *rxe);
>   void rxe_port_down(struct rxe_dev *rxe);
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 8fd03ae20efc17..6a4f58a8656578 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -65,23 +65,6 @@ struct rxe_dev *net_to_rxe(struct net_device *ndev)
>   	return found;
>   }
>   
> -struct rxe_dev *get_rxe_by_name(const char *name)
> -{
> -	struct rxe_dev *rxe;
> -	struct rxe_dev *found = NULL;
> -
> -	spin_lock_bh(&dev_list_lock);
> -	list_for_each_entry(rxe, &rxe_dev_list, list) {
> -		if (!strcmp(name, dev_name(&rxe->ib_dev.dev))) {
> -			found = rxe;
> -			break;
> -		}
> -	}
> -	spin_unlock_bh(&dev_list_lock);
> -	return found;
> -}
> -
> -
>   static struct rxe_recv_sockets recv_sockets;
>   
>   struct device *rxe_dma_device(struct rxe_dev *rxe)
> diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c
> index 95a15892f7e659..66c39049cb2d18 100644
> --- a/drivers/infiniband/sw/rxe/rxe_sysfs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c
> @@ -100,6 +100,7 @@ static int rxe_param_set_remove(const char *val, const struct kernel_param *kp)
>   {
>   	int len;
>   	char intf[32];
> +	struct ib_device *ib_dev;
>   	struct rxe_dev *rxe;
>   
>   	len = sanitize_arg(val, intf, sizeof(intf));
> @@ -114,15 +115,16 @@ static int rxe_param_set_remove(const char *val, const struct kernel_param *kp)
>   		return 0;
>   	}
>   
> -	rxe = get_rxe_by_name(intf);
> -
> -	if (!rxe) {
> +	ib_dev = ib_device_get_by_name(intf, RDMA_DRIVER_RXE);
> +	if (!ib_dev) {
>   		pr_err("not configured on %s\n", intf);
>   		return -EINVAL;
>   	}
>   
> +	rxe = container_of(ib_dev, struct rxe_dev, ib_dev);
>   	list_del(&rxe->list);
>   	rxe_remove(rxe);
> +	ib_device_put(ib_dev);
>   
>   	return 0;
>   }
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 85e9dab17b9b92..d4d6bd6c0ae121 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -3944,6 +3944,8 @@ static inline bool ib_device_try_get(struct ib_device *dev)
>   }
>   
>   void ib_device_put(struct ib_device *device);
> +struct ib_device *ib_device_get_by_name(const char *name,
> +					enum rdma_driver_id driver_id);
>   struct net_device *ib_get_net_dev_by_params(struct ib_device *dev, u8 port,
>   					    u16 pkey, const union ib_gid *gid,
>   					    const struct sockaddr *addr);