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 <[email protected]>
---
drivers/md/dm-cache-metadata.c | 11 -----------
drivers/md/dm-cache-metadata.h | 5 -----
drivers/md/dm-cache-target.c | 25 ++++++++-----------------
3 files changed, 8 insertions(+), 33 deletions(-)
diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index a9a1ab284076..cf491c8508a4 100644
--- 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 5f77890207fe..2f107e7c67d0 100644
--- 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 eb9d738704d0..17f37db57b5e 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2506,23 +2506,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);
@@ -2848,6 +2833,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
@@ -3081,7 +3072,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;
}
--
2.49.0
