dm cache: make sure every metadata function checks fail_io

Otherwise operations may be attempted that will only ever go on to crash
(since the metadata device is either missing or unreliable if 'fail_io'
is set).

Signed-off-by: Joe Thornber <ejt@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: stable@vger.kernel.org

3 files changed, 71 insertions, 43 deletions

diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index f6543f3a970f..27f2ef300f8b 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -867,19 +867,40 @@ static int blocks_are_unmapped_or_clean(struct dm_cache_metadata *cmd,
         return 0;
 }
 
-#define WRITE_LOCK(cmd) \
-        if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) \
+#define WRITE_LOCK(cmd) \
+        down_write(&cmd->root_lock); \
+        if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) { \
+                up_write(&cmd->root_lock); \
                 return -EINVAL; \
-        down_write(&cmd->root_lock)
+        }
 
 #define WRITE_LOCK_VOID(cmd) \
-        if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) \
+        down_write(&cmd->root_lock); \
+        if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) { \
+                up_write(&cmd->root_lock); \
                 return; \
-        down_write(&cmd->root_lock)
+        }
 
 #define WRITE_UNLOCK(cmd) \
         up_write(&cmd->root_lock)
 
+#define READ_LOCK(cmd) \
+        down_read(&cmd->root_lock); \
+        if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) { \
+                up_read(&cmd->root_lock); \
+                return -EINVAL; \
+        }
+
+#define READ_LOCK_VOID(cmd)     \
+        down_read(&cmd->root_lock); \
+        if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) { \
+                up_read(&cmd->root_lock); \
+                return; \
+        }
+
+#define READ_UNLOCK(cmd) \
+        up_read(&cmd->root_lock)
+
 int dm_cache_resize(struct dm_cache_metadata *cmd, dm_cblock_t new_cache_size)
 {
         int r;
@@ -1015,22 +1036,20 @@ int dm_cache_load_discards(struct dm_cache_metadata *cmd,
 {
         int r;
 
-        down_read(&cmd->root_lock);
+        READ_LOCK(cmd);
         r = __load_discards(cmd, fn, context);
-        up_read(&cmd->root_lock);
+        READ_UNLOCK(cmd);
 
         return r;
 }
 
-dm_cblock_t dm_cache_size(struct dm_cache_metadata *cmd)
+int dm_cache_size(struct dm_cache_metadata *cmd, dm_cblock_t *result)
 {
-        dm_cblock_t r;
+        READ_LOCK(cmd);
+        *result = cmd->cache_blocks;
+        READ_UNLOCK(cmd);
 
-        down_read(&cmd->root_lock);
-        r = cmd->cache_blocks;
-        up_read(&cmd->root_lock);
-
-        return r;
+        return 0;
 }
 
 static int __remove(struct dm_cache_metadata *cmd, dm_cblock_t cblock)
@@ -1188,9 +1207,9 @@ int dm_cache_load_mappings(struct dm_cache_metadata *cmd,
 {
         int r;
 
-        down_read(&cmd->root_lock);
+        READ_LOCK(cmd);
         r = __load_mappings(cmd, policy, fn, context);
-        up_read(&cmd->root_lock);
+        READ_UNLOCK(cmd);
 
         return r;
 }
@@ -1215,18 +1234,18 @@ static int __dump_mappings(struct dm_cache_metadata *cmd)
 
 void dm_cache_dump(struct dm_cache_metadata *cmd)
 {
-        down_read(&cmd->root_lock);
+        READ_LOCK_VOID(cmd);
         __dump_mappings(cmd);
-        up_read(&cmd->root_lock);
+        READ_UNLOCK(cmd);
 }
 
 int dm_cache_changed_this_transaction(struct dm_cache_metadata *cmd)
 {
         int r;
 
-        down_read(&cmd->root_lock);
+        READ_LOCK(cmd);
         r = cmd->changed;
-        up_read(&cmd->root_lock);
+        READ_UNLOCK(cmd);
 
         return r;
 }
@@ -1276,9 +1295,9 @@ int dm_cache_set_dirty(struct dm_cache_metadata *cmd,
 void dm_cache_metadata_get_stats(struct dm_cache_metadata *cmd,
                                  struct dm_cache_statistics *stats)
 {
-        down_read(&cmd->root_lock);
+        READ_LOCK_VOID(cmd);
         *stats = cmd->stats;
-        up_read(&cmd->root_lock);
+        READ_UNLOCK(cmd);
 }
 
 void dm_cache_metadata_set_stats(struct dm_cache_metadata *cmd,
@@ -1312,9 +1331,9 @@ int dm_cache_get_free_metadata_block_count(struct dm_cache_metadata *cmd,
 {
         int r = -EINVAL;
 
-        down_read(&cmd->root_lock);
+        READ_LOCK(cmd);
         r = dm_sm_get_nr_free(cmd->metadata_sm, result);
-        up_read(&cmd->root_lock);
+        READ_UNLOCK(cmd);
 
         return r;
 }
@@ -1324,9 +1343,9 @@ int dm_cache_get_metadata_dev_size(struct dm_cache_metadata *cmd,
 {
         int r = -EINVAL;
 
-        down_read(&cmd->root_lock);
+        READ_LOCK(cmd);
         r = dm_sm_get_nr_blocks(cmd->metadata_sm, result);
-        up_read(&cmd->root_lock);
+        READ_UNLOCK(cmd);
 
         return r;
 }
@@ -1417,7 +1436,13 @@ int dm_cache_write_hints(struct dm_cache_metadata *cmd, struct dm_cache_policy *
 
 int dm_cache_metadata_all_clean(struct dm_cache_metadata *cmd, bool *result)
 {
-        return blocks_are_unmapped_or_clean(cmd, 0, cmd->cache_blocks, 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)
@@ -1440,10 +1465,7 @@ int dm_cache_metadata_set_needs_check(struct dm_cache_metadata *cmd)
         struct dm_block *sblock;
         struct cache_disk_superblock *disk_super;
 
-        /*
-         * We ignore fail_io for this function.
-         */
-        down_write(&cmd->root_lock);
+        WRITE_LOCK(cmd);
         set_bit(NEEDS_CHECK, &cmd->flags);
 
         r = superblock_lock(cmd, &sblock);
@@ -1458,19 +1480,17 @@ int dm_cache_metadata_set_needs_check(struct dm_cache_metadata *cmd)
         dm_bm_unlock(sblock);
 
 out:
-        up_write(&cmd->root_lock);
+        WRITE_UNLOCK(cmd);
         return r;
 }
 
-bool dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd)
+int dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd, bool *result)
 {
-        bool needs_check;
+        READ_LOCK(cmd);
+        *result = !!test_bit(NEEDS_CHECK, &cmd->flags);
+        READ_UNLOCK(cmd);
 
-        down_read(&cmd->root_lock);
-        needs_check = !!test_bit(NEEDS_CHECK, &cmd->flags);
-        up_read(&cmd->root_lock);
-
-        return needs_check;
+        return 0;
 }
 
 int dm_cache_metadata_abort(struct dm_cache_metadata *cmd)
diff --git a/drivers/md/dm-cache-metadata.h b/drivers/md/dm-cache-metadata.h
index 2ffee21f318d..8528744195e5 100644
--- a/drivers/md/dm-cache-metadata.h
+++ b/drivers/md/dm-cache-metadata.h
@@ -66,7 +66,7 @@ void dm_cache_metadata_close(struct dm_cache_metadata *cmd);
  * origin blocks to map to.
  */
 int dm_cache_resize(struct dm_cache_metadata *cmd, dm_cblock_t new_cache_size);
-dm_cblock_t dm_cache_size(struct dm_cache_metadata *cmd);
+int dm_cache_size(struct dm_cache_metadata *cmd, dm_cblock_t *result);
 
 int dm_cache_discard_bitset_resize(struct dm_cache_metadata *cmd,
                                    sector_t discard_block_size,
@@ -137,7 +137,7 @@ int dm_cache_write_hints(struct dm_cache_metadata *cmd, struct dm_cache_policy *
  */
 int dm_cache_metadata_all_clean(struct dm_cache_metadata *cmd, bool *result);
 
-bool dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd);
+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);
 void dm_cache_metadata_set_read_write(struct dm_cache_metadata *cmd);
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 2238d6f484fe..43d9e9a39f81 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -984,9 +984,14 @@ static void notify_mode_switch(struct cache *cache, enum cache_metadata_mode mod
 
 static void set_cache_mode(struct cache *cache, enum cache_metadata_mode new_mode)
 {
-        bool needs_check = dm_cache_metadata_needs_check(cache->cmd);
+        bool needs_check;
         enum cache_metadata_mode old_mode = get_cache_mode(cache);
 
+        if (dm_cache_metadata_needs_check(cache->cmd, &needs_check)) {
+                DMERR("unable to read needs_check flag, setting failure mode");
+                new_mode = CM_FAIL;
+        }
+
         if (new_mode == CM_WRITE && needs_check) {
                 DMERR("%s: unable to switch cache to write mode until repaired.",
                       cache_device_name(cache));
@@ -3510,6 +3515,7 @@ static void cache_status(struct dm_target *ti, status_type_t type,
         char buf[BDEVNAME_SIZE];
         struct cache *cache = ti->private;
         dm_cblock_t residency;
+        bool needs_check;
 
         switch (type) {
         case STATUSTYPE_INFO:
@@ -3583,7 +3589,9 @@ static void cache_status(struct dm_target *ti, status_type_t type,
                 else
                         DMEMIT("rw ");
 
-                if (dm_cache_metadata_needs_check(cache->cmd))
+                r = dm_cache_metadata_needs_check(cache->cmd, &needs_check);
+
+                if (r || needs_check)
                         DMEMIT("needs_check ");
                 else
                         DMEMIT("- ");