dm cache: Fix ABBA deadlock between shrink_slab and dm_cache_metadata_abort
authorMike Snitzer <snitzer@kernel.org>
Wed, 30 Nov 2022 18:26:32 +0000 (13:26 -0500)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 7 Jan 2023 11:07:36 +0000 (12:07 +0100)
commit 352b837a5541690d4f843819028cf2b8be83d424 upstream.

Same ABBA deadlock pattern fixed in commit 4b60f452ec51 ("dm thin: Fix
ABBA deadlock between shrink_slab and dm_pool_abort_metadata") to
DM-cache's metadata.

Reported-by: Zhihao Cheng <chengzhihao1@huawei.com>
Cc: stable@vger.kernel.org
Fixes: 028ae9f76f29 ("dm cache: add fail io mode and needs_check flag")
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/md/dm-cache-metadata.c

index a9208ab127080c9e7a9cd968d669d7d0dec7e7e8..16925d16906b03056f39922f694d2a90a238a9c5 100644 (file)
@@ -522,11 +522,13 @@ static int __create_persistent_data_objects(struct dm_cache_metadata *cmd,
        return r;
 }
 
-static void __destroy_persistent_data_objects(struct dm_cache_metadata *cmd)
+static void __destroy_persistent_data_objects(struct dm_cache_metadata *cmd,
+                                             bool destroy_bm)
 {
        dm_sm_destroy(cmd->metadata_sm);
        dm_tm_destroy(cmd->tm);
-       dm_block_manager_destroy(cmd->bm);
+       if (destroy_bm)
+               dm_block_manager_destroy(cmd->bm);
 }
 
 typedef unsigned long (*flags_mutator)(unsigned long);
@@ -780,7 +782,7 @@ static struct dm_cache_metadata *lookup_or_open(struct block_device *bdev,
                cmd2 = lookup(bdev);
                if (cmd2) {
                        mutex_unlock(&table_lock);
-                       __destroy_persistent_data_objects(cmd);
+                       __destroy_persistent_data_objects(cmd, true);
                        kfree(cmd);
                        return cmd2;
                }
@@ -827,7 +829,7 @@ void dm_cache_metadata_close(struct dm_cache_metadata *cmd)
                mutex_unlock(&table_lock);
 
                if (!cmd->fail_io)
-                       __destroy_persistent_data_objects(cmd);
+                       __destroy_persistent_data_objects(cmd, true);
                kfree(cmd);
        }
 }
@@ -1551,14 +1553,53 @@ int dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd, bool *result)
 
 int dm_cache_metadata_abort(struct dm_cache_metadata *cmd)
 {
-       int r;
+       int r = -EINVAL;
+       struct dm_block_manager *old_bm = NULL, *new_bm = NULL;
+
+       /* fail_io is double-checked with cmd->root_lock held below */
+       if (unlikely(cmd->fail_io))
+               return r;
+
+       /*
+        * Replacement block manager (new_bm) is created and old_bm destroyed outside of
+        * cmd root_lock to avoid ABBA deadlock that would result (due to life-cycle of
+        * shrinker associated with the block manager's bufio client vs cmd root_lock).
+        * - must take shrinker_rwsem without holding cmd->root_lock
+        */
+       new_bm = dm_block_manager_create(cmd->bdev, DM_CACHE_METADATA_BLOCK_SIZE << SECTOR_SHIFT,
+                                        CACHE_METADATA_CACHE_SIZE,
+                                        CACHE_MAX_CONCURRENT_LOCKS);
 
        WRITE_LOCK(cmd);
-       __destroy_persistent_data_objects(cmd);
-       r = __create_persistent_data_objects(cmd, false);
+       if (cmd->fail_io) {
+               WRITE_UNLOCK(cmd);
+               goto out;
+       }
+
+       __destroy_persistent_data_objects(cmd, false);
+       old_bm = cmd->bm;
+       if (IS_ERR(new_bm)) {
+               DMERR("could not create block manager during abort");
+               cmd->bm = NULL;
+               r = PTR_ERR(new_bm);
+               goto out_unlock;
+       }
+
+       cmd->bm = new_bm;
+       r = __open_or_format_metadata(cmd, false);
+       if (r) {
+               cmd->bm = NULL;
+               goto out_unlock;
+       }
+       new_bm = NULL;
+out_unlock:
        if (r)
                cmd->fail_io = true;
        WRITE_UNLOCK(cmd);
+       dm_block_manager_destroy(old_bm);
+out:
+       if (new_bm && !IS_ERR(new_bm))
+               dm_block_manager_destroy(new_bm);
 
        return r;
 }