Skip to content

Commit d483001

Browse files
Li LingfengMike Snitzer
Li Lingfeng
authored and
Mike Snitzer
committed
dm thin metadata: Fix ABBA deadlock by resetting dm_bufio_client
As described in commit 8111964 ("dm thin: Fix ABBA deadlock between shrink_slab and dm_pool_abort_metadata"), ABBA deadlocks will be triggered because shrinker_rwsem currently needs to held by dm_pool_abort_metadata() as a side-effect of thin-pool metadata operation failure. The following three problem scenarios have been noticed: 1) Described by commit 8111964 ("dm thin: Fix ABBA deadlock between shrink_slab and dm_pool_abort_metadata") 2) shrinker_rwsem and throttle->lock P1(drop cache) P2(kworker) drop_caches_sysctl_handler drop_slab shrink_slab down_read(&shrinker_rwsem) - LOCK A do_shrink_slab super_cache_scan prune_icache_sb dispose_list evict ext4_evict_inode ext4_clear_inode ext4_discard_preallocations ext4_mb_load_buddy_gfp ext4_mb_init_cache ext4_wait_block_bitmap __ext4_error ext4_handle_error ext4_commit_super ... dm_submit_bio do_worker throttle_work_update down_write(&t->lock) -- LOCK B process_deferred_bios commit metadata_operation_failed dm_pool_abort_metadata dm_block_manager_create dm_bufio_client_create register_shrinker down_write(&shrinker_rwsem) -- LOCK A thin_map thin_bio_map thin_defer_bio_with_throttle throttle_lock down_read(&t->lock) - LOCK B 3) shrinker_rwsem and wait_on_buffer P1(drop cache) P2(kworker) drop_caches_sysctl_handler drop_slab shrink_slab down_read(&shrinker_rwsem) - LOCK A do_shrink_slab ... ext4_wait_block_bitmap __ext4_error ext4_handle_error jbd2_journal_abort jbd2_journal_update_sb_errno jbd2_write_superblock submit_bh // LOCK B // RELEASE B do_worker throttle_work_update down_write(&t->lock) - LOCK B process_deferred_bios process_bio commit metadata_operation_failed dm_pool_abort_metadata dm_block_manager_create dm_bufio_client_create register_shrinker register_shrinker_prepared down_write(&shrinker_rwsem) - LOCK A bio_endio wait_on_buffer __wait_on_buffer Fix these by resetting dm_bufio_client without holding shrinker_rwsem. Fixes: 8111964 ("dm thin: Fix ABBA deadlock between shrink_slab and dm_pool_abort_metadata") Cc: [email protected] Signed-off-by: Li Lingfeng <[email protected]> Signed-off-by: Mike Snitzer <[email protected]>
1 parent 2a32897 commit d483001

File tree

7 files changed

+46
-34
lines changed

7 files changed

+46
-34
lines changed

‎drivers/md/dm-bufio.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2592,6 +2592,13 @@ void dm_bufio_client_destroy(struct dm_bufio_client *c)
25922592
}
25932593
EXPORT_SYMBOL_GPL(dm_bufio_client_destroy);
25942594

2595+
void dm_bufio_client_reset(struct dm_bufio_client *c)
2596+
{
2597+
drop_buffers(c);
2598+
flush_work(&c->shrink_work);
2599+
}
2600+
EXPORT_SYMBOL_GPL(dm_bufio_client_reset);
2601+
25952602
void dm_bufio_set_sector_offset(struct dm_bufio_client *c, sector_t start)
25962603
{
25972604
c->start = start;

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

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,8 @@ static int __format_metadata(struct dm_pool_metadata *pmd)
603603
r = dm_tm_create_with_sm(pmd->bm, THIN_SUPERBLOCK_LOCATION,
604604
&pmd->tm, &pmd->metadata_sm);
605605
if (r < 0) {
606+
pmd->tm = NULL;
607+
pmd->metadata_sm = NULL;
606608
DMERR("tm_create_with_sm failed");
607609
return r;
608610
}
@@ -611,6 +613,7 @@ static int __format_metadata(struct dm_pool_metadata *pmd)
611613
if (IS_ERR(pmd->data_sm)) {
612614
DMERR("sm_disk_create failed");
613615
r = PTR_ERR(pmd->data_sm);
616+
pmd->data_sm = NULL;
614617
goto bad_cleanup_tm;
615618
}
616619

@@ -641,11 +644,15 @@ static int __format_metadata(struct dm_pool_metadata *pmd)
641644

642645
bad_cleanup_nb_tm:
643646
dm_tm_destroy(pmd->nb_tm);
647+
pmd->nb_tm = NULL;
644648
bad_cleanup_data_sm:
645649
dm_sm_destroy(pmd->data_sm);
650+
pmd->data_sm = NULL;
646651
bad_cleanup_tm:
647652
dm_tm_destroy(pmd->tm);
653+
pmd->tm = NULL;
648654
dm_sm_destroy(pmd->metadata_sm);
655+
pmd->metadata_sm = NULL;
649656

650657
return r;
651658
}
@@ -711,6 +718,8 @@ static int __open_metadata(struct dm_pool_metadata *pmd)
711718
sizeof(disk_super->metadata_space_map_root),
712719
&pmd->tm, &pmd->metadata_sm);
713720
if (r < 0) {
721+
pmd->tm = NULL;
722+
pmd->metadata_sm = NULL;
714723
DMERR("tm_open_with_sm failed");
715724
goto bad_unlock_sblock;
716725
}
@@ -720,6 +729,7 @@ static int __open_metadata(struct dm_pool_metadata *pmd)
720729
if (IS_ERR(pmd->data_sm)) {
721730
DMERR("sm_disk_open failed");
722731
r = PTR_ERR(pmd->data_sm);
732+
pmd->data_sm = NULL;
723733
goto bad_cleanup_tm;
724734
}
725735

@@ -746,9 +756,12 @@ static int __open_metadata(struct dm_pool_metadata *pmd)
746756

747757
bad_cleanup_data_sm:
748758
dm_sm_destroy(pmd->data_sm);
759+
pmd->data_sm = NULL;
749760
bad_cleanup_tm:
750761
dm_tm_destroy(pmd->tm);
762+
pmd->tm = NULL;
751763
dm_sm_destroy(pmd->metadata_sm);
764+
pmd->metadata_sm = NULL;
752765
bad_unlock_sblock:
753766
dm_bm_unlock(sblock);
754767

@@ -795,9 +808,13 @@ static void __destroy_persistent_data_objects(struct dm_pool_metadata *pmd,
795808
bool destroy_bm)
796809
{
797810
dm_sm_destroy(pmd->data_sm);
811+
pmd->data_sm = NULL;
798812
dm_sm_destroy(pmd->metadata_sm);
813+
pmd->metadata_sm = NULL;
799814
dm_tm_destroy(pmd->nb_tm);
815+
pmd->nb_tm = NULL;
800816
dm_tm_destroy(pmd->tm);
817+
pmd->tm = NULL;
801818
if (destroy_bm)
802819
dm_block_manager_destroy(pmd->bm);
803820
}
@@ -1005,8 +1022,7 @@ int dm_pool_metadata_close(struct dm_pool_metadata *pmd)
10051022
__func__, r);
10061023
}
10071024
pmd_write_unlock(pmd);
1008-
if (!pmd->fail_io)
1009-
__destroy_persistent_data_objects(pmd, true);
1025+
__destroy_persistent_data_objects(pmd, true);
10101026

10111027
kfree(pmd);
10121028
return 0;
@@ -1877,53 +1893,29 @@ static void __set_abort_with_changes_flags(struct dm_pool_metadata *pmd)
18771893
int dm_pool_abort_metadata(struct dm_pool_metadata *pmd)
18781894
{
18791895
int r = -EINVAL;
1880-
struct dm_block_manager *old_bm = NULL, *new_bm = NULL;
18811896

18821897
/* fail_io is double-checked with pmd->root_lock held below */
18831898
if (unlikely(pmd->fail_io))
18841899
return r;
18851900

1886-
/*
1887-
* Replacement block manager (new_bm) is created and old_bm destroyed outside of
1888-
* pmd root_lock to avoid ABBA deadlock that would result (due to life-cycle of
1889-
* shrinker associated with the block manager's bufio client vs pmd root_lock).
1890-
* - must take shrinker_mutex without holding pmd->root_lock
1891-
*/
1892-
new_bm = dm_block_manager_create(pmd->bdev, THIN_METADATA_BLOCK_SIZE << SECTOR_SHIFT,
1893-
THIN_MAX_CONCURRENT_LOCKS);
1894-
18951901
pmd_write_lock(pmd);
18961902
if (pmd->fail_io) {
18971903
pmd_write_unlock(pmd);
1898-
goto out;
1904+
return r;
18991905
}
1900-
19011906
__set_abort_with_changes_flags(pmd);
1907+
1908+
/* destroy data_sm/metadata_sm/nb_tm/tm */
19021909
__destroy_persistent_data_objects(pmd, false);
1903-
old_bm = pmd->bm;
1904-
if (IS_ERR(new_bm)) {
1905-
DMERR("could not create block manager during abort");
1906-
pmd->bm = NULL;
1907-
r = PTR_ERR(new_bm);
1908-
goto out_unlock;
1909-
}
19101910

1911-
pmd->bm = new_bm;
1911+
/* reset bm */
1912+
dm_block_manager_reset(pmd->bm);
1913+
1914+
/* rebuild data_sm/metadata_sm/nb_tm/tm */
19121915
r = __open_or_format_metadata(pmd, false);
1913-
if (r) {
1914-
pmd->bm = NULL;
1915-
goto out_unlock;
1916-
}
1917-
new_bm = NULL;
1918-
out_unlock:
19191916
if (r)
19201917
pmd->fail_io = true;
19211918
pmd_write_unlock(pmd);
1922-
dm_block_manager_destroy(old_bm);
1923-
out:
1924-
if (new_bm && !IS_ERR(new_bm))
1925-
dm_block_manager_destroy(new_bm);
1926-
19271919
return r;
19281920
}
19291921

‎drivers/md/persistent-data/dm-block-manager.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,12 @@ void dm_block_manager_destroy(struct dm_block_manager *bm)
421421
}
422422
EXPORT_SYMBOL_GPL(dm_block_manager_destroy);
423423

424+
void dm_block_manager_reset(struct dm_block_manager *bm)
425+
{
426+
dm_bufio_client_reset(bm->bufio);
427+
}
428+
EXPORT_SYMBOL_GPL(dm_block_manager_reset);
429+
424430
unsigned int dm_bm_block_size(struct dm_block_manager *bm)
425431
{
426432
return dm_bufio_get_block_size(bm->bufio);

‎drivers/md/persistent-data/dm-block-manager.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ struct dm_block_manager *dm_block_manager_create(
3636
struct block_device *bdev, unsigned int block_size,
3737
unsigned int max_held_per_thread);
3838
void dm_block_manager_destroy(struct dm_block_manager *bm);
39+
void dm_block_manager_reset(struct dm_block_manager *bm);
3940

4041
unsigned int dm_bm_block_size(struct dm_block_manager *bm);
4142
dm_block_t dm_bm_nr_blocks(struct dm_block_manager *bm);

‎drivers/md/persistent-data/dm-space-map.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ struct dm_space_map {
7777

7878
static inline void dm_sm_destroy(struct dm_space_map *sm)
7979
{
80-
sm->destroy(sm);
80+
if (sm)
81+
sm->destroy(sm);
8182
}
8283

8384
static inline int dm_sm_extend(struct dm_space_map *sm, dm_block_t extra_blocks)

‎drivers/md/persistent-data/dm-transaction-manager.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ EXPORT_SYMBOL_GPL(dm_tm_create_non_blocking_clone);
199199

200200
void dm_tm_destroy(struct dm_transaction_manager *tm)
201201
{
202+
if (!tm)
203+
return;
204+
202205
if (!tm->is_clone)
203206
wipe_shadow_table(tm);
204207

‎include/linux/dm-bufio.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ dm_bufio_client_create(struct block_device *bdev, unsigned int block_size,
3838
*/
3939
void dm_bufio_client_destroy(struct dm_bufio_client *c);
4040

41+
void dm_bufio_client_reset(struct dm_bufio_client *c);
42+
4143
/*
4244
* Set the sector range.
4345
* When this function is called, there must be no I/O in progress on the bufio

0 commit comments

Comments
 (0)