rbd: make get_lock_owner_info() return a single locker or NULL
authorIlya Dryomov <idryomov@gmail.com>
Fri, 30 Jun 2023 11:52:13 +0000 (13:52 +0200)
committerIlya Dryomov <idryomov@gmail.com>
Wed, 26 Jul 2023 13:08:08 +0000 (15:08 +0200)
Make the "num_lockers can be only 0 or 1" assumption explicit and
simplify the API by getting rid of output parameters in preparation
for calling get_lock_owner_info() twice before blocklisting.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Dongsheng Yang <dongsheng.yang@easystack.cn>
drivers/block/rbd.c

index bd0e075a5d8928cda2beede3e567edf62011f6dd..dca6c1e5f6bc55992dab9f406e616b1640156817 100644 (file)
@@ -3849,10 +3849,17 @@ static void wake_lock_waiters(struct rbd_device *rbd_dev, int result)
        list_splice_tail_init(&rbd_dev->acquiring_list, &rbd_dev->running_list);
 }
 
-static int get_lock_owner_info(struct rbd_device *rbd_dev,
-                              struct ceph_locker **lockers, u32 *num_lockers)
+static void free_locker(struct ceph_locker *locker)
+{
+       if (locker)
+               ceph_free_lockers(locker, 1);
+}
+
+static struct ceph_locker *get_lock_owner_info(struct rbd_device *rbd_dev)
 {
        struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
+       struct ceph_locker *lockers;
+       u32 num_lockers;
        u8 lock_type;
        char *lock_tag;
        int ret;
@@ -3861,39 +3868,45 @@ static int get_lock_owner_info(struct rbd_device *rbd_dev,
 
        ret = ceph_cls_lock_info(osdc, &rbd_dev->header_oid,
                                 &rbd_dev->header_oloc, RBD_LOCK_NAME,
-                                &lock_type, &lock_tag, lockers, num_lockers);
-       if (ret)
-               return ret;
+                                &lock_type, &lock_tag, &lockers, &num_lockers);
+       if (ret) {
+               rbd_warn(rbd_dev, "failed to retrieve lockers: %d", ret);
+               return ERR_PTR(ret);
+       }
 
-       if (*num_lockers == 0) {
+       if (num_lockers == 0) {
                dout("%s rbd_dev %p no lockers detected\n", __func__, rbd_dev);
+               lockers = NULL;
                goto out;
        }
 
        if (strcmp(lock_tag, RBD_LOCK_TAG)) {
                rbd_warn(rbd_dev, "locked by external mechanism, tag %s",
                         lock_tag);
-               ret = -EBUSY;
-               goto out;
+               goto err_busy;
        }
 
        if (lock_type == CEPH_CLS_LOCK_SHARED) {
                rbd_warn(rbd_dev, "shared lock type detected");
-               ret = -EBUSY;
-               goto out;
+               goto err_busy;
        }
 
-       if (strncmp((*lockers)[0].id.cookie, RBD_LOCK_COOKIE_PREFIX,
+       WARN_ON(num_lockers != 1);
+       if (strncmp(lockers[0].id.cookie, RBD_LOCK_COOKIE_PREFIX,
                    strlen(RBD_LOCK_COOKIE_PREFIX))) {
                rbd_warn(rbd_dev, "locked by external mechanism, cookie %s",
-                        (*lockers)[0].id.cookie);
-               ret = -EBUSY;
-               goto out;
+                        lockers[0].id.cookie);
+               goto err_busy;
        }
 
 out:
        kfree(lock_tag);
-       return ret;
+       return lockers;
+
+err_busy:
+       kfree(lock_tag);
+       ceph_free_lockers(lockers, num_lockers);
+       return ERR_PTR(-EBUSY);
 }
 
 static int find_watcher(struct rbd_device *rbd_dev,
@@ -3947,51 +3960,56 @@ out:
 static int rbd_try_lock(struct rbd_device *rbd_dev)
 {
        struct ceph_client *client = rbd_dev->rbd_client->client;
-       struct ceph_locker *lockers;
-       u32 num_lockers;
+       struct ceph_locker *locker;
        int ret;
 
        for (;;) {
+               locker = NULL;
+
                ret = rbd_lock(rbd_dev);
                if (ret != -EBUSY)
-                       return ret;
+                       goto out;
 
                /* determine if the current lock holder is still alive */
-               ret = get_lock_owner_info(rbd_dev, &lockers, &num_lockers);
-               if (ret)
-                       return ret;
-
-               if (num_lockers == 0)
+               locker = get_lock_owner_info(rbd_dev);
+               if (IS_ERR(locker)) {
+                       ret = PTR_ERR(locker);
+                       locker = NULL;
+                       goto out;
+               }
+               if (!locker)
                        goto again;
 
-               ret = find_watcher(rbd_dev, lockers);
+               ret = find_watcher(rbd_dev, locker);
                if (ret)
                        goto out; /* request lock or error */
 
                rbd_warn(rbd_dev, "breaking header lock owned by %s%llu",
-                        ENTITY_NAME(lockers[0].id.name));
+                        ENTITY_NAME(locker->id.name));
 
                ret = ceph_monc_blocklist_add(&client->monc,
-                                             &lockers[0].info.addr);
+                                             &locker->info.addr);
                if (ret) {
-                       rbd_warn(rbd_dev, "blocklist of %s%llu failed: %d",
-                                ENTITY_NAME(lockers[0].id.name), ret);
+                       rbd_warn(rbd_dev, "failed to blocklist %s%llu: %d",
+                                ENTITY_NAME(locker->id.name), ret);
                        goto out;
                }
 
                ret = ceph_cls_break_lock(&client->osdc, &rbd_dev->header_oid,
                                          &rbd_dev->header_oloc, RBD_LOCK_NAME,
-                                         lockers[0].id.cookie,
-                                         &lockers[0].id.name);
-               if (ret && ret != -ENOENT)
+                                         locker->id.cookie, &locker->id.name);
+               if (ret && ret != -ENOENT) {
+                       rbd_warn(rbd_dev, "failed to break header lock: %d",
+                                ret);
                        goto out;
+               }
 
 again:
-               ceph_free_lockers(lockers, num_lockers);
+               free_locker(locker);
        }
 
 out:
-       ceph_free_lockers(lockers, num_lockers);
+       free_locker(locker);
        return ret;
 }