Skip to content

Commit d14fcf3

Browse files
jthornbersnitm
jthornber
authored and
snitm
committed
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 <[email protected]> Signed-off-by: Mike Snitzer <[email protected]> Cc: [email protected]
1 parent 3f06804 commit d14fcf3

File tree

3 files changed

+71
-43
lines changed

3 files changed

+71
-43
lines changed

‎drivers/md/dm-cache-metadata.c‎

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -867,19 +867,40 @@ static int blocks_are_unmapped_or_clean(struct dm_cache_metadata *cmd,
867867
return 0;
868868
}
869869

870-
#define WRITE_LOCK(cmd) \
871-
if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) \
870+
#define WRITE_LOCK(cmd) \
871+
down_write(&cmd->root_lock); \
872+
if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) { \
873+
up_write(&cmd->root_lock); \
872874
return -EINVAL; \
873-
down_write(&cmd->root_lock)
875+
}
874876

875877
#define WRITE_LOCK_VOID(cmd) \
876-
if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) \
878+
down_write(&cmd->root_lock); \
879+
if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) { \
880+
up_write(&cmd->root_lock); \
877881
return; \
878-
down_write(&cmd->root_lock)
882+
}
879883

880884
#define WRITE_UNLOCK(cmd) \
881885
up_write(&cmd->root_lock)
882886

887+
#define READ_LOCK(cmd) \
888+
down_read(&cmd->root_lock); \
889+
if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) { \
890+
up_read(&cmd->root_lock); \
891+
return -EINVAL; \
892+
}
893+
894+
#define READ_LOCK_VOID(cmd) \
895+
down_read(&cmd->root_lock); \
896+
if (cmd->fail_io || dm_bm_is_read_only(cmd->bm)) { \
897+
up_read(&cmd->root_lock); \
898+
return; \
899+
}
900+
901+
#define READ_UNLOCK(cmd) \
902+
up_read(&cmd->root_lock)
903+
883904
int dm_cache_resize(struct dm_cache_metadata *cmd, dm_cblock_t new_cache_size)
884905
{
885906
int r;
@@ -1015,22 +1036,20 @@ int dm_cache_load_discards(struct dm_cache_metadata *cmd,
10151036
{
10161037
int r;
10171038

1018-
down_read(&cmd->root_lock);
1039+
READ_LOCK(cmd);
10191040
r = __load_discards(cmd, fn, context);
1020-
up_read(&cmd->root_lock);
1041+
READ_UNLOCK(cmd);
10211042

10221043
return r;
10231044
}
10241045

1025-
dm_cblock_t dm_cache_size(struct dm_cache_metadata *cmd)
1046+
int dm_cache_size(struct dm_cache_metadata *cmd, dm_cblock_t *result)
10261047
{
1027-
dm_cblock_t r;
1048+
READ_LOCK(cmd);
1049+
*result = cmd->cache_blocks;
1050+
READ_UNLOCK(cmd);
10281051

1029-
down_read(&cmd->root_lock);
1030-
r = cmd->cache_blocks;
1031-
up_read(&cmd->root_lock);
1032-
1033-
return r;
1052+
return 0;
10341053
}
10351054

10361055
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,
11881207
{
11891208
int r;
11901209

1191-
down_read(&cmd->root_lock);
1210+
READ_LOCK(cmd);
11921211
r = __load_mappings(cmd, policy, fn, context);
1193-
up_read(&cmd->root_lock);
1212+
READ_UNLOCK(cmd);
11941213

11951214
return r;
11961215
}
@@ -1215,18 +1234,18 @@ static int __dump_mappings(struct dm_cache_metadata *cmd)
12151234

12161235
void dm_cache_dump(struct dm_cache_metadata *cmd)
12171236
{
1218-
down_read(&cmd->root_lock);
1237+
READ_LOCK_VOID(cmd);
12191238
__dump_mappings(cmd);
1220-
up_read(&cmd->root_lock);
1239+
READ_UNLOCK(cmd);
12211240
}
12221241

12231242
int dm_cache_changed_this_transaction(struct dm_cache_metadata *cmd)
12241243
{
12251244
int r;
12261245

1227-
down_read(&cmd->root_lock);
1246+
READ_LOCK(cmd);
12281247
r = cmd->changed;
1229-
up_read(&cmd->root_lock);
1248+
READ_UNLOCK(cmd);
12301249

12311250
return r;
12321251
}
@@ -1276,9 +1295,9 @@ int dm_cache_set_dirty(struct dm_cache_metadata *cmd,
12761295
void dm_cache_metadata_get_stats(struct dm_cache_metadata *cmd,
12771296
struct dm_cache_statistics *stats)
12781297
{
1279-
down_read(&cmd->root_lock);
1298+
READ_LOCK_VOID(cmd);
12801299
*stats = cmd->stats;
1281-
up_read(&cmd->root_lock);
1300+
READ_UNLOCK(cmd);
12821301
}
12831302

12841303
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,
13121331
{
13131332
int r = -EINVAL;
13141333

1315-
down_read(&cmd->root_lock);
1334+
READ_LOCK(cmd);
13161335
r = dm_sm_get_nr_free(cmd->metadata_sm, result);
1317-
up_read(&cmd->root_lock);
1336+
READ_UNLOCK(cmd);
13181337

13191338
return r;
13201339
}
@@ -1324,9 +1343,9 @@ int dm_cache_get_metadata_dev_size(struct dm_cache_metadata *cmd,
13241343
{
13251344
int r = -EINVAL;
13261345

1327-
down_read(&cmd->root_lock);
1346+
READ_LOCK(cmd);
13281347
r = dm_sm_get_nr_blocks(cmd->metadata_sm, result);
1329-
up_read(&cmd->root_lock);
1348+
READ_UNLOCK(cmd);
13301349

13311350
return r;
13321351
}
@@ -1417,7 +1436,13 @@ int dm_cache_write_hints(struct dm_cache_metadata *cmd, struct dm_cache_policy *
14171436

14181437
int dm_cache_metadata_all_clean(struct dm_cache_metadata *cmd, bool *result)
14191438
{
1420-
return blocks_are_unmapped_or_clean(cmd, 0, cmd->cache_blocks, result);
1439+
int r;
1440+
1441+
READ_LOCK(cmd);
1442+
r = blocks_are_unmapped_or_clean(cmd, 0, cmd->cache_blocks, result);
1443+
READ_UNLOCK(cmd);
1444+
1445+
return r;
14211446
}
14221447

14231448
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)
14401465
struct dm_block *sblock;
14411466
struct cache_disk_superblock *disk_super;
14421467

1443-
/*
1444-
* We ignore fail_io for this function.
1445-
*/
1446-
down_write(&cmd->root_lock);
1468+
WRITE_LOCK(cmd);
14471469
set_bit(NEEDS_CHECK, &cmd->flags);
14481470

14491471
r = superblock_lock(cmd, &sblock);
@@ -1458,19 +1480,17 @@ int dm_cache_metadata_set_needs_check(struct dm_cache_metadata *cmd)
14581480
dm_bm_unlock(sblock);
14591481

14601482
out:
1461-
up_write(&cmd->root_lock);
1483+
WRITE_UNLOCK(cmd);
14621484
return r;
14631485
}
14641486

1465-
bool dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd)
1487+
int dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd, bool *result)
14661488
{
1467-
bool needs_check;
1489+
READ_LOCK(cmd);
1490+
*result = !!test_bit(NEEDS_CHECK, &cmd->flags);
1491+
READ_UNLOCK(cmd);
14681492

1469-
down_read(&cmd->root_lock);
1470-
needs_check = !!test_bit(NEEDS_CHECK, &cmd->flags);
1471-
up_read(&cmd->root_lock);
1472-
1473-
return needs_check;
1493+
return 0;
14741494
}
14751495

14761496
int dm_cache_metadata_abort(struct dm_cache_metadata *cmd)

‎drivers/md/dm-cache-metadata.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ void dm_cache_metadata_close(struct dm_cache_metadata *cmd);
6666
* origin blocks to map to.
6767
*/
6868
int dm_cache_resize(struct dm_cache_metadata *cmd, dm_cblock_t new_cache_size);
69-
dm_cblock_t dm_cache_size(struct dm_cache_metadata *cmd);
69+
int dm_cache_size(struct dm_cache_metadata *cmd, dm_cblock_t *result);
7070

7171
int dm_cache_discard_bitset_resize(struct dm_cache_metadata *cmd,
7272
sector_t discard_block_size,
@@ -137,7 +137,7 @@ int dm_cache_write_hints(struct dm_cache_metadata *cmd, struct dm_cache_policy *
137137
*/
138138
int dm_cache_metadata_all_clean(struct dm_cache_metadata *cmd, bool *result);
139139

140-
bool dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd);
140+
int dm_cache_metadata_needs_check(struct dm_cache_metadata *cmd, bool *result);
141141
int dm_cache_metadata_set_needs_check(struct dm_cache_metadata *cmd);
142142
void dm_cache_metadata_set_read_only(struct dm_cache_metadata *cmd);
143143
void dm_cache_metadata_set_read_write(struct dm_cache_metadata *cmd);

‎drivers/md/dm-cache-target.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -984,9 +984,14 @@ static void notify_mode_switch(struct cache *cache, enum cache_metadata_mode mod
984984

985985
static void set_cache_mode(struct cache *cache, enum cache_metadata_mode new_mode)
986986
{
987-
bool needs_check = dm_cache_metadata_needs_check(cache->cmd);
987+
bool needs_check;
988988
enum cache_metadata_mode old_mode = get_cache_mode(cache);
989989

990+
if (dm_cache_metadata_needs_check(cache->cmd, &needs_check)) {
991+
DMERR("unable to read needs_check flag, setting failure mode");
992+
new_mode = CM_FAIL;
993+
}
994+
990995
if (new_mode == CM_WRITE && needs_check) {
991996
DMERR("%s: unable to switch cache to write mode until repaired.",
992997
cache_device_name(cache));
@@ -3510,6 +3515,7 @@ static void cache_status(struct dm_target *ti, status_type_t type,
35103515
char buf[BDEVNAME_SIZE];
35113516
struct cache *cache = ti->private;
35123517
dm_cblock_t residency;
3518+
bool needs_check;
35133519

35143520
switch (type) {
35153521
case STATUSTYPE_INFO:
@@ -3583,7 +3589,9 @@ static void cache_status(struct dm_target *ti, status_type_t type,
35833589
else
35843590
DMEMIT("rw ");
35853591

3586-
if (dm_cache_metadata_needs_check(cache->cmd))
3592+
r = dm_cache_metadata_needs_check(cache->cmd, &needs_check);
3593+
3594+
if (r || needs_check)
35873595
DMEMIT("needs_check ");
35883596
else
35893597
DMEMIT("- ");

0 commit comments

Comments
 (0)