On Thu, Apr 07, 2022 at 07:13:07PM -0500, Mike Christie wrote:
> We can't release the endpoint ID until all references to the endpoint have
> been dropped or it could be allocated while in use. This has us use an idr
> instead of looping over all conns to find a free ID and then free the ID
> when all references have been dropped instead of when the device is only
> deleted.
>
> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
> ---
> drivers/scsi/scsi_transport_iscsi.c | 71 ++++++++++++++---------------
> include/scsi/scsi_transport_iscsi.h | 2 +-
> 2 files changed, 36 insertions(+), 37 deletions(-)
Reviewed-by: Chris Leech <cleech@xxxxxxxxxx>
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index bf39fb5569b6..1fc7c6bfbd67 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -86,6 +86,9 @@ struct iscsi_internal {
> struct transport_container session_cont;
> };
>
> +static DEFINE_IDR(iscsi_ep_idr);
> +static DEFINE_MUTEX(iscsi_ep_idr_mutex);
> +
> static atomic_t iscsi_session_nr; /* sysfs session id for next new session */
>
> static struct workqueue_struct *iscsi_conn_cleanup_workq;
> @@ -168,6 +171,11 @@ struct device_attribute dev_attr_##_prefix##_##_name = \
> static void iscsi_endpoint_release(struct device *dev)
> {
> struct iscsi_endpoint *ep = iscsi_dev_to_endpoint(dev);
> +
> + mutex_lock(&iscsi_ep_idr_mutex);
> + idr_remove(&iscsi_ep_idr, ep->id);
> + mutex_unlock(&iscsi_ep_idr_mutex);
> +
> kfree(ep);
> }
>
> @@ -180,7 +188,7 @@ static ssize_t
> show_ep_handle(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct iscsi_endpoint *ep = iscsi_dev_to_endpoint(dev);
> - return sysfs_emit(buf, "%llu\n", (unsigned long long) ep->id);
> + return sysfs_emit(buf, "%d\n", ep->id);
> }
> static ISCSI_ATTR(ep, handle, S_IRUGO, show_ep_handle, NULL);
>
> @@ -193,48 +201,32 @@ static struct attribute_group iscsi_endpoint_group = {
> .attrs = iscsi_endpoint_attrs,
> };
>
> -#define ISCSI_MAX_EPID -1
> -
> -static int iscsi_match_epid(struct device *dev, const void *data)
> -{
> - struct iscsi_endpoint *ep = iscsi_dev_to_endpoint(dev);
> - const uint64_t *epid = data;
> -
> - return *epid == ep->id;
> -}
> -
> struct iscsi_endpoint *
> iscsi_create_endpoint(int dd_size)
> {
> - struct device *dev;
> struct iscsi_endpoint *ep;
> - uint64_t id;
> - int err;
> -
> - for (id = 1; id < ISCSI_MAX_EPID; id++) {
> - dev = class_find_device(&iscsi_endpoint_class, NULL, &id,
> - iscsi_match_epid);
> - if (!dev)
> - break;
> - else
> - put_device(dev);
> - }
> - if (id == ISCSI_MAX_EPID) {
> - printk(KERN_ERR "Too many connections. Max supported %u\n",
> - ISCSI_MAX_EPID - 1);
> - return NULL;
> - }
> + int err, id;
>
> ep = kzalloc(sizeof(*ep) + dd_size, GFP_KERNEL);
> if (!ep)
> return NULL;
>
> + mutex_lock(&iscsi_ep_idr_mutex);
> + id = idr_alloc(&iscsi_ep_idr, ep, 0, -1, GFP_NOIO);
> + if (id < 0) {
> + mutex_unlock(&iscsi_ep_idr_mutex);
> + printk(KERN_ERR "Could not allocate endpoint ID. Error %d.\n",
> + id);
> + goto free_ep;
> + }
> + mutex_unlock(&iscsi_ep_idr_mutex);
> +
> ep->id = id;
> ep->dev.class = &iscsi_endpoint_class;
> - dev_set_name(&ep->dev, "ep-%llu", (unsigned long long) id);
> + dev_set_name(&ep->dev, "ep-%d", id);
> err = device_register(&ep->dev);
> if (err)
> - goto free_ep;
> + goto free_id;
>
> err = sysfs_create_group(&ep->dev.kobj, &iscsi_endpoint_group);
> if (err)
> @@ -248,6 +240,10 @@ iscsi_create_endpoint(int dd_size)
> device_unregister(&ep->dev);
> return NULL;
>
> +free_id:
> + mutex_lock(&iscsi_ep_idr_mutex);
> + idr_remove(&iscsi_ep_idr, id);
> + mutex_unlock(&iscsi_ep_idr_mutex);
> free_ep:
> kfree(ep);
> return NULL;
> @@ -275,14 +271,17 @@ EXPORT_SYMBOL_GPL(iscsi_put_endpoint);
> */
> struct iscsi_endpoint *iscsi_lookup_endpoint(u64 handle)
> {
> - struct device *dev;
> + struct iscsi_endpoint *ep;
>
> - dev = class_find_device(&iscsi_endpoint_class, NULL, &handle,
> - iscsi_match_epid);
> - if (!dev)
> - return NULL;
> + mutex_lock(&iscsi_ep_idr_mutex);
> + ep = idr_find(&iscsi_ep_idr, handle);
> + if (!ep)
> + goto unlock;
>
> - return iscsi_dev_to_endpoint(dev);
> + get_device(&ep->dev);
> +unlock:
> + mutex_unlock(&iscsi_ep_idr_mutex);
> + return ep;
> }
> EXPORT_SYMBOL_GPL(iscsi_lookup_endpoint);
>
> diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
> index 38e4a67f5922..fdd486047404 100644
> --- a/include/scsi/scsi_transport_iscsi.h
> +++ b/include/scsi/scsi_transport_iscsi.h
> @@ -295,7 +295,7 @@ extern void iscsi_host_for_each_session(struct Scsi_Host *shost,
> struct iscsi_endpoint {
> void *dd_data; /* LLD private data */
> struct device dev;
> - uint64_t id;
> + int id;
> struct iscsi_cls_conn *conn;
> };
>
> --
> 2.25.1
>
