btrfs: raid56: do not unhash a cached rbio that has rbios plugged on it

unlock_stripe() caches the finished rbio with cache_rbio() before it
retakes the bucket and bio_list locks, and RBIO_RMW_LOCKED_BIT is still
set at that point.  A lock_stripe_add() for the same full stripe that
runs in that window finds a cached rbio it can neither steal (still RMW
locked) nor merge with (cached) and plugs itself onto it, which is
correct: unlock_stripe() then sees the plug list and hands the stripe
lock on to the plugged rbio.

But if the rbio is removed from the cache in that same window --
btrfs_raid56_uncache_range() from the commit's unpin, or the cache's own
shrink in cache_rbio() -- __remove_rbio_from_cache() finds an empty
bio_list, takes the rbio off the hash list, drops the hash reference and
hits BUG_ON(!list_empty(&rbio->plug_list)).  Seen on a degraded raid5
under fsstress: the transaction kthread died in that BUG_ON with the
bucket lock held and the rmw workers spun on it until the machine was
reset.

An rbio with rbios plugged on it is busy, like one with bios: leave it
on the hash list and let unlock_stripe() hand the lock on; drop only the
cache's reference here.

Assisted-by: Claude:claude-opus-4-8

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index deacd44a7804dcaacbdf04b78b73360217fbae81..77ffc59262e3efc5e8c356cc06bd46b3164ec248 100644 (file)
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -474,11 +474,21 @@ static void __remove_rbio_from_cache(struct btrfs_raid_bio *rbio)
                 * the rbio from the hash_table, and drop
                 * the corresponding ref
                 */
-               if (bio_list_empty(&rbio->bio_list)) {
+               /*
+                * An rbio with no bios is idle only if nothing is plugged on
+                * it either.  unlock_stripe() caches the rbio before it retakes
+                * the locks, with RBIO_RMW_LOCKED_BIT still set, so a
+                * lock_stripe_add() in that window can neither steal the pages
+                * nor merge and plugs onto the cached rbio instead.  That rbio
+                * is still the stripe's lock owner and unlock_stripe() hands
+                * the lock on to the plugged one; only the cache's reference
+                * goes away here.
+                */
+               if (bio_list_empty(&rbio->bio_list) &&
+                   list_empty(&rbio->plug_list)) {
                        if (!list_empty(&rbio->hash_list)) {
                                list_del_init(&rbio->hash_list);
                                refcount_dec(&rbio->refs);
-                               BUG_ON(!list_empty(&rbio->plug_list));
                        }
                }
        }