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>
Thu, 12 Jan 2023 10:58:52 +0000 (11:58 +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 89a7320..0f6f74e 100644 (file)
@@ -551,11 +551,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);
@@ -826,7 +828,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;
                }
@@ -874,7 +876,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);
        }
 }
@@ -1808,14 +1810,52 @@ 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_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;
 }