rbd: always kick acquire on "acquired" and "released" notifications
authorIlya Dryomov <idryomov@gmail.com>
Sat, 3 Jul 2021 09:56:55 +0000 (11:56 +0200)
committerIlya Dryomov <idryomov@gmail.com>
Tue, 20 Jul 2021 15:57:33 +0000 (17:57 +0200)
Skipping the "lock has been released" notification if the lock owner
is not what we expect based on owner_cid can lead to I/O hangs.
One example is our own notifications: because owner_cid is cleared
in rbd_unlock(), when we get our own notification it is processed as
unexpected/duplicate and maybe_kick_acquire() isn't called.  If a peer
that requested the lock then doesn't go through with acquiring it,
I/O requests that came in while the lock was being quiesced would
be stalled until another I/O request is submitted and kicks acquire
from rbd_img_exclusive_lock().

This makes the comment in rbd_release_lock() actually true: prior to
this change the canceled work was being requeued in response to the
"lock has been acquired" notification from rbd_handle_acquired_lock().

Cc: stable@vger.kernel.org # 5.3+
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Tested-by: Robin Geuze <robin.geuze@nl.team.blue>
drivers/block/rbd.c

index 531d390..e77cea6 100644 (file)
@@ -4203,15 +4203,11 @@ static void rbd_handle_acquired_lock(struct rbd_device *rbd_dev, u8 struct_v,
        if (!rbd_cid_equal(&cid, &rbd_empty_cid)) {
                down_write(&rbd_dev->lock_rwsem);
                if (rbd_cid_equal(&cid, &rbd_dev->owner_cid)) {
-                       /*
-                        * we already know that the remote client is
-                        * the owner
-                        */
-                       up_write(&rbd_dev->lock_rwsem);
-                       return;
+                       dout("%s rbd_dev %p cid %llu-%llu == owner_cid\n",
+                            __func__, rbd_dev, cid.gid, cid.handle);
+               } else {
+                       rbd_set_owner_cid(rbd_dev, &cid);
                }
-
-               rbd_set_owner_cid(rbd_dev, &cid);
                downgrade_write(&rbd_dev->lock_rwsem);
        } else {
                down_read(&rbd_dev->lock_rwsem);
@@ -4236,14 +4232,12 @@ static void rbd_handle_released_lock(struct rbd_device *rbd_dev, u8 struct_v,
        if (!rbd_cid_equal(&cid, &rbd_empty_cid)) {
                down_write(&rbd_dev->lock_rwsem);
                if (!rbd_cid_equal(&cid, &rbd_dev->owner_cid)) {
-                       dout("%s rbd_dev %p unexpected owner, cid %llu-%llu != owner_cid %llu-%llu\n",
+                       dout("%s rbd_dev %p cid %llu-%llu != owner_cid %llu-%llu\n",
                             __func__, rbd_dev, cid.gid, cid.handle,
                             rbd_dev->owner_cid.gid, rbd_dev->owner_cid.handle);
-                       up_write(&rbd_dev->lock_rwsem);
-                       return;
+               } else {
+                       rbd_set_owner_cid(rbd_dev, &rbd_empty_cid);
                }
-
-               rbd_set_owner_cid(rbd_dev, &rbd_empty_cid);
                downgrade_write(&rbd_dev->lock_rwsem);
        } else {
                down_read(&rbd_dev->lock_rwsem);