lkml.org 
[lkml]   [2022]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3] dm thin: Fix ABBA deadlock between shrink_slab and dm_pool_abort_metadata
From
Date
> On Wed, Nov 30 2022 at  8:31P -0500
> Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>
>> Following concurrent processes:
>>
>> P1(drop cache) P2(kworker)
[...]
>> 2.31.1
>>
>
> I've picked your patch up and folded into these following changes:
>
> diff --git a/drivers/md/dm-thin-metadata.c b/drivers/md/dm-thin-metadata.c
> index 7729372519b8..1a62226ac34e 100644
> --- a/drivers/md/dm-thin-metadata.c
> +++ b/drivers/md/dm-thin-metadata.c
> @@ -776,12 +776,15 @@ static int __create_persistent_data_objects(struct dm_pool_metadata *pmd, bool f
> return r;
> }
>
> -static void __destroy_persistent_data_objects(struct dm_pool_metadata *pmd)
> +static void __destroy_persistent_data_objects(struct dm_pool_metadata *pmd,
> + bool destroy_bm)
> {
> dm_sm_destroy(pmd->data_sm);
> dm_sm_destroy(pmd->metadata_sm);
> dm_tm_destroy(pmd->nb_tm);
> dm_tm_destroy(pmd->tm);
> + if (destroy_bm)
> + dm_block_manager_destroy(pmd->bm);
> }
>
> static int __begin_transaction(struct dm_pool_metadata *pmd)
> @@ -987,10 +990,8 @@ int dm_pool_metadata_close(struct dm_pool_metadata *pmd)
> __func__, r);
> }
> pmd_write_unlock(pmd);
> - if (!pmd->fail_io) {
> - __destroy_persistent_data_objects(pmd);
> - dm_block_manager_destroy(pmd->bm);
> - }
> + if (!pmd->fail_io)
> + __destroy_persistent_data_objects(pmd, true);
>
> kfree(pmd);
> return 0;
> @@ -1863,9 +1864,16 @@ int dm_pool_abort_metadata(struct dm_pool_metadata *pmd)
> int r = -EINVAL;
> struct dm_block_manager *old_bm = NULL, *new_bm = NULL;
>
> - if (pmd->fail_io)
> - return -EINVAL;
> + /* fail_io is double-checked with pmd->root_lock held below */
> + if (unlikely(pmd->fail_io))
> + return r;
>
> + /*
> + * Replacement block manager (new_bm) is created and old_bm destroyed outside of
> + * pmd root_lock to avoid ABBA deadlock that would result (due to life-cycle of
> + * shrinker associated with the block manager's bufio client vs pmd root_lock).
> + * - must take shrinker_rwsem without holding pmd->root_lock
> + */
> new_bm = dm_block_manager_create(pmd->bdev, THIN_METADATA_BLOCK_SIZE << SECTOR_SHIFT,
> THIN_MAX_CONCURRENT_LOCKS);
>
> @@ -1876,11 +1884,10 @@ int dm_pool_abort_metadata(struct dm_pool_metadata *pmd)
> }
>
> __set_abort_with_changes_flags(pmd);
> - __destroy_persistent_data_objects(pmd);
> + __destroy_persistent_data_objects(pmd, false);
> old_bm = pmd->bm;
> if (IS_ERR(new_bm)) {
> - /* Return back if creating dm_block_manager failed. */
> - DMERR("could not create block manager");
> + DMERR("could not create block manager during abort");
> pmd->bm = NULL;
> r = PTR_ERR(new_bm);
> goto out_unlock;
>

Hi,Mike
This version is great, thanks for reviewing and modifying.

\
 
 \ /
  Last update: 2022-12-01 03:12    [W:0.032 / U:0.696 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site