dm cache: fix dirty mapping checking in passthrough mode switching

As mentioned in commit 9b1cc9f251af ("dm cache: share cache-metadata
object across inactive and active DM tables"), dm-cache assumed table
reload occurs after suspension, while LVM's table preload breaks this
assumption. The dirty mapping check for passthrough mode was designed
around this assumption and is performed during table creation, causing
the check to fail with preload while metadata updates are ongoing. This
risks loading dirty mappings into passthrough mode, resulting in data
loss.

Reproduce steps:

1. Create a writeback cache with zero migration_threshold to produce
   dirty mappings

dmsetup create cmeta --table "0 8192 linear /dev/sdc 0"
dmsetup create cdata --table "0 131072 linear /dev/sdc 8192"
dmsetup create corig --table "0 262144 linear /dev/sdc 262144"
dd if=/dev/zero of=/dev/mapper/cmeta bs=4k count=1 oflag=direct
dmsetup create cache --table "0 262144 cache /dev/mapper/cmeta \
/dev/mapper/cdata /dev/mapper/corig 128 2 metadata2 writeback smq \
2 migration_threshold 0"

2. Preload a table in passthrough mode

dmsetup reload cache --table "0 262144 cache /dev/mapper/cmeta \
/dev/mapper/cdata /dev/mapper/corig 128 2 metadata2 passthrough smq 0"

3. Write to the first cache block to make it dirty

fio --filename=/dev/mapper/cache --name=populate --rw=write --bs=4k \
--direct=1 --size=64k

4. Resume the inactive table. Now it's possible to load the dirty block
   into passthrough mode.

dmsetup resume cache

Fix by moving the checks to the preresume phase to support table
preloading. Also remove the unused function dm_cache_metadata_all_clean.

Fixes: 2ee57d587357 ("dm cache: add passthrough mode")
Signed-off-by: Ming-Hung Tsai <mtsai@redhat.com>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index 57158c02d096ed38759d563bf27e7f1b3fe58ccc..1b86e80c89cca0caafe1bbc331d5c0721f991c4c 100644 (file)
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -1714,17 +1714,6 @@ int dm_cache_write_hints(struct dm_cache_metadata *cmd, struct dm_cache_policy *
        return r;
 }
 
-int dm_cache_metadata_all_clean(struct dm_cache_metadata *cmd, bool *result)
-{
-       int r;
-
-       READ_LOCK(cmd);
-       r = blocks_are_unmapped_or_clean(cmd, 0, cmd->cache_blocks, result);
-       READ_UNLOCK(cmd);
-
-       return r;
-}
-
 void dm_cache_metadata_set_read_only(struct dm_cache_metadata *cmd)
 {
        WRITE_LOCK_VOID(cmd);

diff --git a/drivers/md/dm-cache-metadata.h b/drivers/md/dm-cache-metadata.h
index 5f77890207fedea429de2930070f0728ca506eb8..2f107e7c67d0a856f763ba08344de575e8b6e669 100644 (file)
--- a/drivers/md/dm-cache-metadata.h
+++ b/drivers/md/dm-cache-metadata.h
@@ -135,11 +135,6 @@ int dm_cache_get_metadata_dev_size(struct dm_cache_metadata *cmd,
  */
 int dm_cache_write_hints(struct dm_cache_metadata *cmd, struct dm_cache_policy *p);
 
-/*
- * Query method.  Are all the blocks in the cache clean?
- */
-int dm_cache_metadata_all_clean(struct dm_cache_metadata *cmd, bool *result);
-
 int dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd, bool *result);
 int dm_cache_metadata_set_needs_check(struct dm_cache_metadata *cmd);
 void dm_cache_metadata_set_read_only(struct dm_cache_metadata *cmd);

diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 32d22c7b9a07d61d2d4a94ce1e5dbdadf842735c..e479ac22b97cc6cabd5e6e8f70e43848e0752b32 100644 (file)
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2499,23 +2499,8 @@ static int cache_create(struct cache_args *ca, struct cache **result)
                goto bad;
        }
 
-       if (passthrough_mode(cache)) {
-               bool all_clean;
-
-               r = dm_cache_metadata_all_clean(cache->cmd, &all_clean);
-               if (r) {
-                       *error = "dm_cache_metadata_all_clean() failed";
-                       goto bad;
-               }
-
-               if (!all_clean) {
-                       *error = "Cannot enter passthrough mode unless all blocks are clean";
-                       r = -EINVAL;
-                       goto bad;
-               }
-
+       if (passthrough_mode(cache))
                policy_allow_migrations(cache->policy, false);
-       }
 
        spin_lock_init(&cache->lock);
        bio_list_init(&cache->deferred_bios);
@@ -2842,6 +2827,12 @@ static int load_mapping(void *context, dm_oblock_t oblock, dm_cblock_t cblock,
        struct cache *cache = context;
 
        if (dirty) {
+               if (passthrough_mode(cache)) {
+                       DMERR("%s: cannot enter passthrough mode unless all blocks are clean",
+                             cache_device_name(cache));
+                       return -EBUSY;
+               }
+
                set_bit(from_cblock(cblock), cache->dirty_bitset);
                atomic_inc(&cache->nr_dirty);
        } else
@@ -3075,7 +3066,7 @@ static int cache_preresume(struct dm_target *ti)
                                           load_filtered_mapping, cache);
                if (r) {
                        DMERR("%s: could not load cache mappings", cache_device_name(cache));
-                       if (r != -EFBIG)
+                       if (r != -EFBIG && r != -EBUSY)
                                metadata_operation_failed(cache, "dm_cache_load_mappings", r);
                        return r;
                }