[dm-devel] [PATCH 5/7] dm: track per-add_disk holder relations in DM
Yu Kuai
yukuai1 at huaweicloud.com
Wed Nov 9 02:08:14 UTC 2022
Hi,
在 2022/10/30 23:31, Christoph Hellwig 写道:
> dm is a bit special in that it opens the underlying devices. Commit
> 89f871af1b26 ("dm: delay registering the gendisk") tried to accomodate
> that by allowing to add the holder to the list before add_gendisk and
> then just add them to sysfs once add_disk is called. But that leads to
> really odd lifetime problems and error handling problems as we can't
> know the state of the kobjects and don't unwind properly. To fix this
> switch to just registering all existing table_devices with the holder
> code right after add_disk, and remove them before calling del_gendisk.
>
> Fixes: 89f871af1b26 ("dm: delay registering the gendisk")
> Reported-by: Yu Kuai <yukuai1 at huaweicloud.com>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
> drivers/md/dm.c | 45 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 37 insertions(+), 8 deletions(-)
>
> 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?
Thanks,
Kuai
> + r = bd_link_disk_holder(bdev, md->disk);
> + if (r)
> + goto out_blkdev_put;
> + }
>
> td->dm_dev.mode = mode;
> td->dm_dev.bdev = bdev;
> @@ -774,7 +781,8 @@ static struct table_device *open_table_device(struct mapped_device *md,
> */
> static void close_table_device(struct table_device *td, struct mapped_device *md)
> {
> - bd_unlink_disk_holder(td->dm_dev.bdev, dm_disk(md));
> + if (md->disk->slave_dir)
> + bd_unlink_disk_holder(td->dm_dev.bdev, md->disk);
> blkdev_put(td->dm_dev.bdev, td->dm_dev.mode | FMODE_EXCL);
> put_dax(td->dm_dev.dax_dev);
> list_del(&td->list);
> @@ -1951,7 +1959,13 @@ static void cleanup_mapped_device(struct mapped_device *md)
> md->disk->private_data = NULL;
> spin_unlock(&_minor_lock);
> if (dm_get_md_type(md) != DM_TYPE_NONE) {
> + struct table_device *td;
> +
> dm_sysfs_exit(md);
> + list_for_each_entry(td, &md->table_devices, list) {
> + bd_unlink_disk_holder(td->dm_dev.bdev,
> + md->disk);
> + }
> del_gendisk(md->disk);
> }
> dm_queue_destroy_crypto_profile(md->queue);
> @@ -2284,6 +2298,7 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
> {
> enum dm_queue_mode type = dm_table_get_type(t);
> struct queue_limits limits;
> + struct table_device *td;
> int r;
>
> switch (type) {
> @@ -2316,13 +2331,27 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
> if (r)
> return r;
>
> - r = dm_sysfs_init(md);
> - if (r) {
> - del_gendisk(md->disk);
> - return r;
> + /*
> + * Register the holder relationship for devices added before the disk
> + * was live.
> + */
> + list_for_each_entry(td, &md->table_devices, list) {
> + r = bd_link_disk_holder(td->dm_dev.bdev, md->disk);
> + if (r)
> + goto out_undo_holders;
> }
> +
> + r = dm_sysfs_init(md);
> + if (r)
> + goto out_undo_holders;
> md->type = type;
> return 0;
> +
> +out_undo_holders:
> + list_for_each_entry_continue_reverse(td, &md->table_devices, list)
> + bd_unlink_disk_holder(td->dm_dev.bdev, md->disk);
> + del_gendisk(md->disk);
> + return r;
> }
>
> struct mapped_device *dm_get_md(dev_t dev)
>
More information about the dm-devel
mailing list