Message ID | 20190112023752.GA24713@ziepe.ca (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | RDMA/device: Add ib_device_get_by_name() and use it in rxe | expand |
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@mellanox.com> > --- > 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);
On Mon, Jan 14, 2019 at 06:00:09PM +0800, Yanjun Zhu wrote: > 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 Yes, this needs the patch I sent earlier Jason
On 2019/1/15 3:27, Jason Gunthorpe wrote: > On Mon, Jan 14, 2019 at 06:00:09PM +0800, Yanjun Zhu wrote: >> 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 > Yes, this needs the patch I sent earlier Sure. I found it in "[PATCH rdma-next v1] RDMA/core: Sync unregistration with netlink commands" +void ib_device_put(struct ib_device *device) +{ + if (refcount_dec_and_test(&device->refcount)) + complete(&device->unreg_completion); +} + Reviewed-by: Zhu Yanjun <yanjun.zhu@oracle.com> > > Jason >
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@mellanox.com> > --- > 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)) In the patch [PATCH for-rc 1/2] RDMA/device: Expose ib_device_try_get((), +/** + * ib_device_try_get: Hold a registration lock + * device: The device to lock + * + * A device under an active registration lock cannot become unregistered. It + * is only possible to obtain a registration lock on a device that is fully + * registered, otherwise this function returns false. + * + * The registration lock is only necessary for actions which require the + * device to still be registered. Uses that only require the device pointer to + * be valid should use get_device(&ibdev->dev) to hold the memory. + * + */ +static inline bool ib_device_try_get(struct ib_device *dev) +{ + return refcount_inc_not_zero(&dev->refcount); +} + > + 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);
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);
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@mellanox.com> --- 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