FAILED: patch "[PATCH] rbd: take header_rwsem in rbd_dev_refresh() only when" failed to apply to 5.10-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 0b207d02bd9ab8dcc31b262ca9f60dbc1822500d
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2023100406-routine-tackle-b16a@gregkh' --subject-prefix 'PATCH 5.10.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 0b207d02bd9ab8dcc31b262ca9f60dbc1822500d Mon Sep 17 00:00:00 2001
From: Ilya Dryomov <[email protected]>
Date: Wed, 20 Sep 2023 19:01:03 +0200
Subject: [PATCH] rbd: take header_rwsem in rbd_dev_refresh() only when
updating
rbd_dev_refresh() has been holding header_rwsem across header and parent info read-in unnecessarily for ages. With commit 870611e4877e ("rbd: get snapshot context after exclusive lock is ensured to be held"), the potential for deadlocks became much more real owning to
a) header_rwsem now nesting inside lock_rwsem and b) rw_semaphores not allowing new readers after a writer is registered.
For example, assuming that I/O request 1, I/O request 2 and header read-in request all target the same OSD:
1. I/O request 1 comes in and gets submitted
2. watch error occurs
3. rbd_watch_errcb() takes lock_rwsem for write, clears owner_cid and releases lock_rwsem
4. after reestablishing the watch, rbd_reregister_watch() calls rbd_dev_refresh() which takes header_rwsem for write and submits a header read-in request
5. I/O request 2 comes in: after taking lock_rwsem for read in __rbd_img_handle_request(), it blocks trying to take header_rwsem for read in rbd_img_object_requests()
6. another watch error occurs
7. rbd_watch_errcb() blocks trying to take lock_rwsem for write
8. I/O request 1 completion is received by the messenger but can't be processed because lock_rwsem won't be granted anymore
9. header read-in request completion can't be received, let alone processed, because the messenger is stranded
Change rbd_dev_refresh() to take header_rwsem only for actually updating rbd_dev->header. Header and parent info read-in don't need any locking.
drivers/block/rbd.c +11 -11
@@ -6986,7 +6986,14 @@ static void rbd_dev_update_header(struct rbd_device *rbd_dev,rbd_assert (rbd_image_format_valid (rbd_dev->image_format));rbd_assert (rbd_dev->header.object_prefix);/* !first_time */ - rbd_dev->header.image_size = header->image_size; +if (rbd_dev->header.image_size != header->image_size) { + rbd_dev->header.image_size = header->image_size; + +if (!rbd_is_snap (rbd_dev)) { + rbd_dev->mapping.size = header->image_size; +rbd_dev_update_size (rbd_dev); + } + }ceph_put_snap_context (rbd_dev->header.snapc); rbd_dev->header.snapc = header->snapc;
@@ -7044,11 +7051,9 @@ static int rbd_dev_refresh(struct rbd_device *rbd_dev) {struct rbd_image_header header = {0 };struct parent_image_info pii = {0 }; -u64 mapping_size;int ret; -down_write (&rbd_dev->header_rwsem); - mapping_size = rbd_dev->mapping.size; +dout ("%s rbd_dev %p\n" , __func__, rbd_dev); ret =rbd_dev_header_info (rbd_dev, &header, false);if (ret)
@@ -7064,18 +7069,13 @@ static int rbd_dev_refresh(struct rbd_device *rbd_dev) goto out; } +down_write (&rbd_dev->header_rwsem);rbd_dev_update_header (rbd_dev, &header);if (rbd_dev->parent)rbd_dev_update_parent (rbd_dev, &pii); - -rbd_assert (!rbd_is_snap (rbd_dev)); - rbd_dev->mapping.size = rbd_dev->header.image_size; +up_write (&rbd_dev->header_rwsem); out: -up_write (&rbd_dev->header_rwsem); -if (!ret && mapping_size != rbd_dev->mapping.size) -rbd_dev_update_size (rbd_dev); -rbd_parent_info_cleanup (&pii);rbd_image_header_cleanup (&header);return ret;