Re: [dm-devel] [PATCH 5/7] dm: track per-add_disk holder relations in DM

From: Christoph Hellwig <hch@lst.de>
To: Yu Kuai <yukuai1@huaweicloud.com>
Cc: Jens Axboe <axboe@kernel.dk>, Mike Snitzer <snitzer@kernel.org>,
	linux-block@vger.kernel.org, dm-devel@redhat.com,
	"yukuai \(C\)" <yukuai3@huawei.com>,
	Christoph Hellwig <hch@lst.de>, Alasdair Kergon <agk@redhat.com>
Subject: Re: [dm-devel] [PATCH 5/7] dm: track per-add_disk holder relations in DM
Date: Wed, 9 Nov 2022 09:26:45 +0100	[thread overview]
Message-ID: <20221109082645.GA14093@lst.de> (raw)
In-Reply-To: <9b5b4c2a-6566-2fb4-d3ae-4904f0889ea0@huaweicloud.com>

On Wed, Nov 09, 2022 at 10:08:14AM +0800, Yu Kuai wrote:
>> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
>> index 2917700b1e15c..7b0d6dc957549 100644
>> --- a/drivers/md/dm.c
>> +++ b/drivers/md/dm.c
>> @@ -751,9 +751,16 @@ static struct table_device *open_table_device(struct mapped_device *md,
>>   		goto out_free_td;
>>   	}
>>   -	r = bd_link_disk_holder(bdev, dm_disk(md));
>> -	if (r)
>> -		goto out_blkdev_put;
>> +	/*
>> +	 * We can be called before the dm disk is added.  In that case we can't
>> +	 * register the holder relation here.  It will be done once add_disk was
>> +	 * called.
>> +	 */
>> +	if (md->disk->slave_dir) {
> If device_add_disk() or del_gendisk() can concurrent with this, It seems
> to me that using 'slave_dir' is not safe.
>
> I'm not quite familiar with dm, can we guarantee that they can't
> concurrent?

I assumed dm would not get itself into territory were creating /
deleting the device could race with adding component devices, but
digging deeper I can't find anything.  This could be done
by holding table_devices_lock around add_disk/del_gendisk, but
I'm not that familar with the dm code.

Mike, can you help out on this?

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel