diff mbox

[01/11] rbd: handle locking inside __rbd_client_find()

Message ID 5037AC8B.6070608@inktank.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Elder Aug. 24, 2012, 4:32 p.m. UTC
There is only one caller of __rbd_client_find(), and it somewhat
clumsily gets the appropriate lock and gets a reference to the
existing ceph_client structure if it's found.

Instead, have that function handle its own locking, and acquire the
reference if found while it holds the lock.  Drop the underscores
from the name because there's no need to signify anything special
about this function.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

Comments

Yehuda Sadeh Aug. 30, 2012, 4:09 p.m. UTC | #1
Reviewed-by: Yehuda Sadeh <yehuda@inktank.com>

On Fri, Aug 24, 2012 at 9:32 AM, Alex Elder <elder@inktank.com> wrote:
>
> There is only one caller of __rbd_client_find(), and it somewhat
> clumsily gets the appropriate lock and gets a reference to the
> existing ceph_client structure if it's found.
>
> Instead, have that function handle its own locking, and acquire the
> reference if found while it holds the lock.  Drop the underscores
> from the name because there's no need to signify anything special
> about this function.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>  drivers/block/rbd.c |   29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 8e6e29e..81b5344 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -322,19 +322,28 @@ out_opt:
>  }
>
>  /*
> - * Find a ceph client with specific addr and configuration.
> + * Find a ceph client with specific addr and configuration.  If
> + * found, bump its reference count.
>   */
> -static struct rbd_client *__rbd_client_find(struct ceph_options
> *ceph_opts)
> +static struct rbd_client *rbd_client_find(struct ceph_options *ceph_opts)
>  {
>         struct rbd_client *client_node;
> +       bool found = false;
>
>         if (ceph_opts->flags & CEPH_OPT_NOSHARE)
>                 return NULL;
>
> -       list_for_each_entry(client_node, &rbd_client_list, node)
> -               if (!ceph_compare_options(ceph_opts, client_node->client))
> -                       return client_node;
> -       return NULL;
> +       spin_lock(&rbd_client_list_lock);
> +       list_for_each_entry(client_node, &rbd_client_list, node) {
> +               if (!ceph_compare_options(ceph_opts, client_node->client))
> {
> +                       kref_get(&client_node->kref);
> +                       found = true;
> +                       break;
> +               }
> +       }
> +       spin_unlock(&rbd_client_list_lock);
> +
> +       return found ? client_node : NULL;
>  }
>
>  /*
> @@ -416,22 +425,16 @@ static struct rbd_client *rbd_get_client(const
> char *mon_addr,
>                 return ERR_CAST(ceph_opts);
>         }
>
> -       spin_lock(&rbd_client_list_lock);
> -       rbdc = __rbd_client_find(ceph_opts);
> +       rbdc = rbd_client_find(ceph_opts);
>         if (rbdc) {
>                 /* using an existing client */
> -               kref_get(&rbdc->kref);
> -               spin_unlock(&rbd_client_list_lock);
> -
>                 ceph_destroy_options(ceph_opts);
>                 kfree(rbd_opts);
>
>                 return rbdc;
>         }
> -       spin_unlock(&rbd_client_list_lock);
>
>         rbdc = rbd_client_create(ceph_opts, rbd_opts);
> -
>         if (IS_ERR(rbdc))
>                 kfree(rbd_opts);
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 8e6e29e..81b5344 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -322,19 +322,28 @@  out_opt:
 }

 /*
- * Find a ceph client with specific addr and configuration.
+ * Find a ceph client with specific addr and configuration.  If
+ * found, bump its reference count.
  */
-static struct rbd_client *__rbd_client_find(struct ceph_options *ceph_opts)
+static struct rbd_client *rbd_client_find(struct ceph_options *ceph_opts)
 {
 	struct rbd_client *client_node;
+	bool found = false;

 	if (ceph_opts->flags & CEPH_OPT_NOSHARE)
 		return NULL;

-	list_for_each_entry(client_node, &rbd_client_list, node)
-		if (!ceph_compare_options(ceph_opts, client_node->client))
-			return client_node;
-	return NULL;
+	spin_lock(&rbd_client_list_lock);
+	list_for_each_entry(client_node, &rbd_client_list, node) {
+		if (!ceph_compare_options(ceph_opts, client_node->client)) {
+			kref_get(&client_node->kref);
+			found = true;
+			break;
+		}
+	}
+	spin_unlock(&rbd_client_list_lock);
+
+	return found ? client_node : NULL;
 }

 /*
@@ -416,22 +425,16 @@  static struct rbd_client *rbd_get_client(const
char *mon_addr,
 		return ERR_CAST(ceph_opts);
 	}

-	spin_lock(&rbd_client_list_lock);
-	rbdc = __rbd_client_find(ceph_opts);
+	rbdc = rbd_client_find(ceph_opts);
 	if (rbdc) {
 		/* using an existing client */
-		kref_get(&rbdc->kref);
-		spin_unlock(&rbd_client_list_lock);
-
 		ceph_destroy_options(ceph_opts);
 		kfree(rbd_opts);

 		return rbdc;
 	}
-	spin_unlock(&rbd_client_list_lock);

 	rbdc = rbd_client_create(ceph_opts, rbd_opts);
-
 	if (IS_ERR(rbdc))
 		kfree(rbd_opts);