dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [dm-devel] use regular gendisk registration in device mapper
@ 2021-07-25  5:54 Christoph Hellwig
  2021-07-25  5:54 ` [dm-devel] [PATCH 1/8] block: make the block holder code optional Christoph Hellwig
                   ` (8 more replies)
  0 siblings, 9 replies; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

Hi all,

The device mapper code currently has a somewhat odd gendisk registration
scheme where it calls add_disk early, but uses a special flag to skip the
"queue registration", which is a major part of add_disk.  This series
improves the block layer holder tracking to work on an entirely
unregistered disk and thus allows device mapper to use the normal scheme
of calling add_disk when it is ready to accept I/O.

Note that this leads to a user visible change - the sysfs attributes on
the disk and the dm directory hanging off it are not only visible once
the initial table is loaded.  This did not make a different to my testing
using dmsetup and the lvm2 tools.

Diffstat:
 block/Kconfig             |    4 +
 block/Makefile            |    1 
 block/elevator.c          |    1 
 block/genhd.c             |   42 +++++------
 block/holder.c            |  167 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/Kconfig        |    2 
 drivers/md/bcache/Kconfig |    1 
 drivers/md/dm-ioctl.c     |    4 -
 drivers/md/dm-rq.c        |    1 
 drivers/md/dm.c           |   32 +++-----
 fs/block_dev.c            |  145 ---------------------------------------
 include/linux/blk_types.h |    3 
 include/linux/genhd.h     |   19 ++---
 13 files changed, 219 insertions(+), 203 deletions(-)

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 1/8] block: make the block holder code optional
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
@ 2021-07-25  5:54 ` Christoph Hellwig
  2021-07-29 16:25   ` Mike Snitzer
  2021-07-25  5:54 ` [dm-devel] [PATCH 2/8] block: remove the extra kobject reference in bd_link_disk_holder Christoph Hellwig
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

Move the block holder code into a separate file as it is not in any way
related to the other block_dev.c code, and add a new selectable config
option for it so that we don't have to build it without any remapped
drivers selected.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/Kconfig             |   4 ++
 block/Makefile            |   1 +
 block/holder.c            | 139 ++++++++++++++++++++++++++++++++++++
 drivers/md/Kconfig        |   2 +
 drivers/md/bcache/Kconfig |   1 +
 fs/block_dev.c            | 144 +-------------------------------------
 include/linux/blk_types.h |   2 +-
 include/linux/genhd.h     |   4 +-
 8 files changed, 151 insertions(+), 146 deletions(-)
 create mode 100644 block/holder.c

diff --git a/block/Kconfig b/block/Kconfig
index fd732aede922..a24d7263d1fc 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -251,4 +251,8 @@ config BLK_MQ_RDMA
 config BLK_PM
 	def_bool BLOCK && PM
 
+# do not use in new code
+config BLOCK_HOLDER_DEPRECATED
+	bool
+
 source "block/Kconfig.iosched"
diff --git a/block/Makefile b/block/Makefile
index bfbe4e13ca1e..6fc6216634ed 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -42,3 +42,4 @@ obj-$(CONFIG_BLK_SED_OPAL)	+= sed-opal.o
 obj-$(CONFIG_BLK_PM)		+= blk-pm.o
 obj-$(CONFIG_BLK_INLINE_ENCRYPTION)	+= keyslot-manager.o blk-crypto.o
 obj-$(CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK)	+= blk-crypto-fallback.o
+obj-$(CONFIG_BLOCK_HOLDER_DEPRECATED)	+= holder.o
diff --git a/block/holder.c b/block/holder.c
new file mode 100644
index 000000000000..904a1dcd5c12
--- /dev/null
+++ b/block/holder.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/genhd.h>
+
+struct bd_holder_disk {
+	struct list_head	list;
+	struct gendisk		*disk;
+	int			refcnt;
+};
+
+static struct bd_holder_disk *bd_find_holder_disk(struct block_device *bdev,
+						  struct gendisk *disk)
+{
+	struct bd_holder_disk *holder;
+
+	list_for_each_entry(holder, &bdev->bd_holder_disks, list)
+		if (holder->disk == disk)
+			return holder;
+	return NULL;
+}
+
+static int add_symlink(struct kobject *from, struct kobject *to)
+{
+	return sysfs_create_link(from, to, kobject_name(to));
+}
+
+static void del_symlink(struct kobject *from, struct kobject *to)
+{
+	sysfs_remove_link(from, kobject_name(to));
+}
+
+/**
+ * bd_link_disk_holder - create symlinks between holding disk and slave bdev
+ * @bdev: the claimed slave bdev
+ * @disk: the holding disk
+ *
+ * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
+ *
+ * This functions creates the following sysfs symlinks.
+ *
+ * - from "slaves" directory of the holder @disk to the claimed @bdev
+ * - from "holders" directory of the @bdev to the holder @disk
+ *
+ * For example, if /dev/dm-0 maps to /dev/sda and disk for dm-0 is
+ * passed to bd_link_disk_holder(), then:
+ *
+ *   /sys/block/dm-0/slaves/sda --> /sys/block/sda
+ *   /sys/block/sda/holders/dm-0 --> /sys/block/dm-0
+ *
+ * The caller must have claimed @bdev before calling this function and
+ * ensure that both @bdev and @disk are valid during the creation and
+ * lifetime of these symlinks.
+ *
+ * CONTEXT:
+ * Might sleep.
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
+{
+	struct bd_holder_disk *holder;
+	int ret = 0;
+
+	mutex_lock(&bdev->bd_disk->open_mutex);
+
+	WARN_ON_ONCE(!bdev->bd_holder);
+
+	/* FIXME: remove the following once add_disk() handles errors */
+	if (WARN_ON(!disk->slave_dir || !bdev->bd_holder_dir))
+		goto out_unlock;
+
+	holder = bd_find_holder_disk(bdev, disk);
+	if (holder) {
+		holder->refcnt++;
+		goto out_unlock;
+	}
+
+	holder = kzalloc(sizeof(*holder), GFP_KERNEL);
+	if (!holder) {
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
+
+	INIT_LIST_HEAD(&holder->list);
+	holder->disk = disk;
+	holder->refcnt = 1;
+
+	ret = add_symlink(disk->slave_dir, bdev_kobj(bdev));
+	if (ret)
+		goto out_free;
+
+	ret = add_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
+	if (ret)
+		goto out_del;
+	/*
+	 * bdev could be deleted beneath us which would implicitly destroy
+	 * the holder directory.  Hold on to it.
+	 */
+	kobject_get(bdev->bd_holder_dir);
+
+	list_add(&holder->list, &bdev->bd_holder_disks);
+	goto out_unlock;
+
+out_del:
+	del_symlink(disk->slave_dir, bdev_kobj(bdev));
+out_free:
+	kfree(holder);
+out_unlock:
+	mutex_unlock(&bdev->bd_disk->open_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(bd_link_disk_holder);
+
+/**
+ * bd_unlink_disk_holder - destroy symlinks created by bd_link_disk_holder()
+ * @bdev: the calimed slave bdev
+ * @disk: the holding disk
+ *
+ * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
+ *
+ * CONTEXT:
+ * Might sleep.
+ */
+void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
+{
+	struct bd_holder_disk *holder;
+
+	mutex_lock(&bdev->bd_disk->open_mutex);
+	holder = bd_find_holder_disk(bdev, disk);
+	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
+		del_symlink(disk->slave_dir, bdev_kobj(bdev));
+		del_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
+		kobject_put(bdev->bd_holder_dir);
+		list_del_init(&holder->list);
+		kfree(holder);
+	}
+	mutex_unlock(&bdev->bd_disk->open_mutex);
+}
+EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 0602e82a9516..f821dae101a9 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -15,6 +15,7 @@ if MD
 
 config BLK_DEV_MD
 	tristate "RAID support"
+	select BLOCK_HOLDER_DEPRECATED if SYSFS
 	help
 	  This driver lets you combine several hard disk partitions into one
 	  logical block device. This can be used to simply append one
@@ -201,6 +202,7 @@ config BLK_DEV_DM_BUILTIN
 
 config BLK_DEV_DM
 	tristate "Device mapper support"
+	select BLOCK_HOLDER_DEPRECATED if SYSFS
 	select BLK_DEV_DM_BUILTIN
 	depends on DAX || DAX=n
 	help
diff --git a/drivers/md/bcache/Kconfig b/drivers/md/bcache/Kconfig
index d1ca4d059c20..cf3e8096942a 100644
--- a/drivers/md/bcache/Kconfig
+++ b/drivers/md/bcache/Kconfig
@@ -2,6 +2,7 @@
 
 config BCACHE
 	tristate "Block device as cache"
+	select BLOCK_HOLDER_DEPRECATED if SYSFS
 	select CRC64
 	help
 	Allows a block device to be used as cache for other devices; uses
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 0c424a0cadaa..7825d152634e 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -900,7 +900,7 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	bdev->bd_disk = disk;
 	bdev->bd_partno = partno;
 	bdev->bd_inode = inode;
-#ifdef CONFIG_SYSFS
+#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 	INIT_LIST_HEAD(&bdev->bd_holder_disks);
 #endif
 	bdev->bd_stats = alloc_percpu(struct disk_stats);
@@ -1092,148 +1092,6 @@ void bd_abort_claiming(struct block_device *bdev, void *holder)
 }
 EXPORT_SYMBOL(bd_abort_claiming);
 
-#ifdef CONFIG_SYSFS
-struct bd_holder_disk {
-	struct list_head	list;
-	struct gendisk		*disk;
-	int			refcnt;
-};
-
-static struct bd_holder_disk *bd_find_holder_disk(struct block_device *bdev,
-						  struct gendisk *disk)
-{
-	struct bd_holder_disk *holder;
-
-	list_for_each_entry(holder, &bdev->bd_holder_disks, list)
-		if (holder->disk == disk)
-			return holder;
-	return NULL;
-}
-
-static int add_symlink(struct kobject *from, struct kobject *to)
-{
-	return sysfs_create_link(from, to, kobject_name(to));
-}
-
-static void del_symlink(struct kobject *from, struct kobject *to)
-{
-	sysfs_remove_link(from, kobject_name(to));
-}
-
-/**
- * bd_link_disk_holder - create symlinks between holding disk and slave bdev
- * @bdev: the claimed slave bdev
- * @disk: the holding disk
- *
- * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
- *
- * This functions creates the following sysfs symlinks.
- *
- * - from "slaves" directory of the holder @disk to the claimed @bdev
- * - from "holders" directory of the @bdev to the holder @disk
- *
- * For example, if /dev/dm-0 maps to /dev/sda and disk for dm-0 is
- * passed to bd_link_disk_holder(), then:
- *
- *   /sys/block/dm-0/slaves/sda --> /sys/block/sda
- *   /sys/block/sda/holders/dm-0 --> /sys/block/dm-0
- *
- * The caller must have claimed @bdev before calling this function and
- * ensure that both @bdev and @disk are valid during the creation and
- * lifetime of these symlinks.
- *
- * CONTEXT:
- * Might sleep.
- *
- * RETURNS:
- * 0 on success, -errno on failure.
- */
-int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
-{
-	struct bd_holder_disk *holder;
-	int ret = 0;
-
-	mutex_lock(&bdev->bd_disk->open_mutex);
-
-	WARN_ON_ONCE(!bdev->bd_holder);
-
-	/* FIXME: remove the following once add_disk() handles errors */
-	if (WARN_ON(!disk->slave_dir || !bdev->bd_holder_dir))
-		goto out_unlock;
-
-	holder = bd_find_holder_disk(bdev, disk);
-	if (holder) {
-		holder->refcnt++;
-		goto out_unlock;
-	}
-
-	holder = kzalloc(sizeof(*holder), GFP_KERNEL);
-	if (!holder) {
-		ret = -ENOMEM;
-		goto out_unlock;
-	}
-
-	INIT_LIST_HEAD(&holder->list);
-	holder->disk = disk;
-	holder->refcnt = 1;
-
-	ret = add_symlink(disk->slave_dir, bdev_kobj(bdev));
-	if (ret)
-		goto out_free;
-
-	ret = add_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
-	if (ret)
-		goto out_del;
-	/*
-	 * bdev could be deleted beneath us which would implicitly destroy
-	 * the holder directory.  Hold on to it.
-	 */
-	kobject_get(bdev->bd_holder_dir);
-
-	list_add(&holder->list, &bdev->bd_holder_disks);
-	goto out_unlock;
-
-out_del:
-	del_symlink(disk->slave_dir, bdev_kobj(bdev));
-out_free:
-	kfree(holder);
-out_unlock:
-	mutex_unlock(&bdev->bd_disk->open_mutex);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(bd_link_disk_holder);
-
-/**
- * bd_unlink_disk_holder - destroy symlinks created by bd_link_disk_holder()
- * @bdev: the calimed slave bdev
- * @disk: the holding disk
- *
- * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
- *
- * CONTEXT:
- * Might sleep.
- */
-void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
-{
-	struct bd_holder_disk *holder;
-
-	mutex_lock(&bdev->bd_disk->open_mutex);
-
-	holder = bd_find_holder_disk(bdev, disk);
-
-	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
-		del_symlink(disk->slave_dir, bdev_kobj(bdev));
-		del_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
-		kobject_put(bdev->bd_holder_dir);
-		list_del_init(&holder->list);
-		kfree(holder);
-	}
-
-	mutex_unlock(&bdev->bd_disk->open_mutex);
-}
-EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
-#endif
-
 static void blkdev_flush_mapping(struct block_device *bdev)
 {
 	WARN_ON_ONCE(bdev->bd_holders);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 290f9061b29a..7a4e139d24ef 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -34,7 +34,7 @@ struct block_device {
 	void *			bd_holder;
 	int			bd_holders;
 	bool			bd_write_holder;
-#ifdef CONFIG_SYSFS
+#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 	struct list_head	bd_holder_disks;
 #endif
 	struct kobject		*bd_holder_dir;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 13b34177cc85..6831d74f2002 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -316,7 +316,7 @@ void set_capacity(struct gendisk *disk, sector_t size);
 int blkdev_ioctl(struct block_device *, fmode_t, unsigned, unsigned long);
 long compat_blkdev_ioctl(struct file *, unsigned, unsigned long);
 
-#ifdef CONFIG_SYSFS
+#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk);
 void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk);
 #else
@@ -329,7 +329,7 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev,
 					 struct gendisk *disk)
 {
 }
-#endif /* CONFIG_SYSFS */
+#endif /* CONFIG_BLOCK_HOLDER_DEPRECATED */
 
 dev_t part_devt(struct gendisk *disk, u8 partno);
 dev_t blk_lookup_devt(const char *name, int partno);
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 2/8] block: remove the extra kobject reference in bd_link_disk_holder
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
  2021-07-25  5:54 ` [dm-devel] [PATCH 1/8] block: make the block holder code optional Christoph Hellwig
@ 2021-07-25  5:54 ` Christoph Hellwig
  2021-07-29 16:27   ` Mike Snitzer
  2021-07-25  5:54 ` [dm-devel] [PATCH 3/8] block: look up holders by bdev Christoph Hellwig
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

With the new block_device life time rules there is no way for the
bdev to go away as long as there is a holder, so remove these.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/holder.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/block/holder.c b/block/holder.c
index 904a1dcd5c12..960654a71342 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -92,11 +92,6 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
 	ret = add_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
 	if (ret)
 		goto out_del;
-	/*
-	 * bdev could be deleted beneath us which would implicitly destroy
-	 * the holder directory.  Hold on to it.
-	 */
-	kobject_get(bdev->bd_holder_dir);
 
 	list_add(&holder->list, &bdev->bd_holder_disks);
 	goto out_unlock;
@@ -130,7 +125,6 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
 	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
 		del_symlink(disk->slave_dir, bdev_kobj(bdev));
 		del_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
-		kobject_put(bdev->bd_holder_dir);
 		list_del_init(&holder->list);
 		kfree(holder);
 	}
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 3/8] block: look up holders by bdev
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
  2021-07-25  5:54 ` [dm-devel] [PATCH 1/8] block: make the block holder code optional Christoph Hellwig
  2021-07-25  5:54 ` [dm-devel] [PATCH 2/8] block: remove the extra kobject reference in bd_link_disk_holder Christoph Hellwig
@ 2021-07-25  5:54 ` Christoph Hellwig
  2021-07-25  5:54 ` [dm-devel] [PATCH 4/8] block: support delayed holder registration Christoph Hellwig
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

Invert they way the holder relations are tracked.  This very
slightly reduces the memory overhead for partitioned devices.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c             |  3 +++
 block/holder.c            | 18 +++++++++---------
 fs/block_dev.c            |  3 ---
 include/linux/blk_types.h |  3 ---
 include/linux/genhd.h     |  4 +++-
 5 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index af4d2ab4a633..e2708a4a7a47 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1263,6 +1263,9 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
 	disk_to_dev(disk)->class = &block_class;
 	disk_to_dev(disk)->type = &disk_type;
 	device_initialize(disk_to_dev(disk));
+#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
+	INIT_LIST_HEAD(&disk->slave_bdevs);
+#endif
 	return disk;
 
 out_destroy_part_tbl:
diff --git a/block/holder.c b/block/holder.c
index 960654a71342..11e65d99a9fb 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -3,7 +3,7 @@
 
 struct bd_holder_disk {
 	struct list_head	list;
-	struct gendisk		*disk;
+	struct block_device	*bdev;
 	int			refcnt;
 };
 
@@ -12,8 +12,8 @@ static struct bd_holder_disk *bd_find_holder_disk(struct block_device *bdev,
 {
 	struct bd_holder_disk *holder;
 
-	list_for_each_entry(holder, &bdev->bd_holder_disks, list)
-		if (holder->disk == disk)
+	list_for_each_entry(holder, &disk->slave_bdevs, list)
+		if (holder->bdev == bdev)
 			return holder;
 	return NULL;
 }
@@ -61,7 +61,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
 	struct bd_holder_disk *holder;
 	int ret = 0;
 
-	mutex_lock(&bdev->bd_disk->open_mutex);
+	mutex_lock(&disk->open_mutex);
 
 	WARN_ON_ONCE(!bdev->bd_holder);
 
@@ -82,7 +82,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
 	}
 
 	INIT_LIST_HEAD(&holder->list);
-	holder->disk = disk;
+	holder->bdev = bdev;
 	holder->refcnt = 1;
 
 	ret = add_symlink(disk->slave_dir, bdev_kobj(bdev));
@@ -93,7 +93,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
 	if (ret)
 		goto out_del;
 
-	list_add(&holder->list, &bdev->bd_holder_disks);
+	list_add(&holder->list, &disk->slave_bdevs);
 	goto out_unlock;
 
 out_del:
@@ -101,7 +101,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
 out_free:
 	kfree(holder);
 out_unlock:
-	mutex_unlock(&bdev->bd_disk->open_mutex);
+	mutex_unlock(&disk->open_mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(bd_link_disk_holder);
@@ -120,7 +120,7 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
 {
 	struct bd_holder_disk *holder;
 
-	mutex_lock(&bdev->bd_disk->open_mutex);
+	mutex_lock(&disk->open_mutex);
 	holder = bd_find_holder_disk(bdev, disk);
 	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
 		del_symlink(disk->slave_dir, bdev_kobj(bdev));
@@ -128,6 +128,6 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
 		list_del_init(&holder->list);
 		kfree(holder);
 	}
-	mutex_unlock(&bdev->bd_disk->open_mutex);
+	mutex_unlock(&disk->open_mutex);
 }
 EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7825d152634e..22646906ddaa 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -900,9 +900,6 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	bdev->bd_disk = disk;
 	bdev->bd_partno = partno;
 	bdev->bd_inode = inode;
-#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
-	INIT_LIST_HEAD(&bdev->bd_holder_disks);
-#endif
 	bdev->bd_stats = alloc_percpu(struct disk_stats);
 	if (!bdev->bd_stats) {
 		iput(inode);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 7a4e139d24ef..e92735655684 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -34,9 +34,6 @@ struct block_device {
 	void *			bd_holder;
 	int			bd_holders;
 	bool			bd_write_holder;
-#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
-	struct list_head	bd_holder_disks;
-#endif
 	struct kobject		*bd_holder_dir;
 	u8			bd_partno;
 	spinlock_t		bd_size_lock; /* for bd_inode->i_size updates */
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 6831d74f2002..26c8557e2714 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -159,7 +159,9 @@ struct gendisk {
 	unsigned open_partitions;	/* number of open partitions */
 
 	struct kobject *slave_dir;
-
+#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
+	struct list_head slave_bdevs;
+#endif
 	struct timer_rand_state *random;
 	atomic_t sync_io;		/* RAID */
 	struct disk_events *ev;
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 4/8] block: support delayed holder registration
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-07-25  5:54 ` [dm-devel] [PATCH 3/8] block: look up holders by bdev Christoph Hellwig
@ 2021-07-25  5:54 ` Christoph Hellwig
  2021-07-27 16:06   ` Mike Snitzer
  2021-07-29 16:32   ` Mike Snitzer
  2021-07-25  5:54 ` [dm-devel] [PATCH 5/8] dm: cleanup cleanup_mapped_device Christoph Hellwig
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

device mapper needs to register holders before it is ready to do I/O.
Currently it does so by registering the disk early, which has all kinds
of bad side effects.  Support registering holders on an initialized but
not registered disk instead by delaying the sysfs registration until the
disk is registered.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c         | 10 +++++++
 block/holder.c        | 68 ++++++++++++++++++++++++++++++++-----------
 include/linux/genhd.h |  5 ++++
 3 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index e2708a4a7a47..e3d93b868ec5 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -429,6 +429,16 @@ static void register_disk(struct device *parent, struct gendisk *disk,
 		kobject_create_and_add("holders", &ddev->kobj);
 	disk->slave_dir = kobject_create_and_add("slaves", &ddev->kobj);
 
+	/*
+	 * XXX: this is a mess, can't wait for real error handling in add_disk.
+	 * Make sure ->slave_dir is NULL if we failed some of the registration
+	 * so that the cleanup in bd_unlink_disk_holder works properly.
+	 */
+	if (bd_register_pending_holders(disk) < 0) {
+		kobject_put(disk->slave_dir);
+		disk->slave_dir = NULL;
+	}
+
 	if (disk->flags & GENHD_FL_HIDDEN)
 		return;
 
diff --git a/block/holder.c b/block/holder.c
index 11e65d99a9fb..4568cc4f6827 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -28,6 +28,19 @@ static void del_symlink(struct kobject *from, struct kobject *to)
 	sysfs_remove_link(from, kobject_name(to));
 }
 
+static int __link_disk_holder(struct block_device *bdev, struct gendisk *disk)
+{
+	int ret;
+
+	ret = add_symlink(disk->slave_dir, bdev_kobj(bdev));
+	if (ret)
+		return ret;
+	ret = add_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
+	if (ret)
+		del_symlink(disk->slave_dir, bdev_kobj(bdev));
+	return ret;
+}
+
 /**
  * bd_link_disk_holder - create symlinks between holding disk and slave bdev
  * @bdev: the claimed slave bdev
@@ -66,7 +79,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
 	WARN_ON_ONCE(!bdev->bd_holder);
 
 	/* FIXME: remove the following once add_disk() handles errors */
-	if (WARN_ON(!disk->slave_dir || !bdev->bd_holder_dir))
+	if (WARN_ON(!bdev->bd_holder_dir))
 		goto out_unlock;
 
 	holder = bd_find_holder_disk(bdev, disk);
@@ -84,28 +97,28 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
 	INIT_LIST_HEAD(&holder->list);
 	holder->bdev = bdev;
 	holder->refcnt = 1;
-
-	ret = add_symlink(disk->slave_dir, bdev_kobj(bdev));
-	if (ret)
-		goto out_free;
-
-	ret = add_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
-	if (ret)
-		goto out_del;
+	if (disk->slave_dir) {
+		ret = __link_disk_holder(bdev, disk);
+		if (ret) {
+			kfree(holder);
+			goto out_unlock;
+		}
+	}
 
 	list_add(&holder->list, &disk->slave_bdevs);
-	goto out_unlock;
-
-out_del:
-	del_symlink(disk->slave_dir, bdev_kobj(bdev));
-out_free:
-	kfree(holder);
 out_unlock:
 	mutex_unlock(&disk->open_mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(bd_link_disk_holder);
 
+static void __unlink_disk_holder(struct block_device *bdev,
+		struct gendisk *disk)
+{
+	del_symlink(disk->slave_dir, bdev_kobj(bdev));
+	del_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
+}
+
 /**
  * bd_unlink_disk_holder - destroy symlinks created by bd_link_disk_holder()
  * @bdev: the calimed slave bdev
@@ -123,11 +136,32 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
 	mutex_lock(&disk->open_mutex);
 	holder = bd_find_holder_disk(bdev, disk);
 	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
-		del_symlink(disk->slave_dir, bdev_kobj(bdev));
-		del_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
+		if (disk->slave_dir)
+			__unlink_disk_holder(bdev, disk);
 		list_del_init(&holder->list);
 		kfree(holder);
 	}
 	mutex_unlock(&disk->open_mutex);
 }
 EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
+
+int bd_register_pending_holders(struct gendisk *disk)
+{
+	struct bd_holder_disk *holder;
+	int ret;
+
+	mutex_lock(&disk->open_mutex);
+	list_for_each_entry(holder, &disk->slave_bdevs, list) {
+		ret = __link_disk_holder(holder->bdev, disk);
+		if (ret)
+			goto out_undo;
+	}
+	mutex_unlock(&disk->open_mutex);
+	return 0;
+
+out_undo:
+	list_for_each_entry_continue_reverse(holder, &disk->slave_bdevs, list)
+		__unlink_disk_holder(holder->bdev, disk);
+	mutex_unlock(&disk->open_mutex);
+	return ret;
+}
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 26c8557e2714..dd95d53c75fa 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -321,6 +321,7 @@ long compat_blkdev_ioctl(struct file *, unsigned, unsigned long);
 #ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk);
 void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk);
+int bd_register_pending_holders(struct gendisk *disk);
 #else
 static inline int bd_link_disk_holder(struct block_device *bdev,
 				      struct gendisk *disk)
@@ -331,6 +332,10 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev,
 					 struct gendisk *disk)
 {
 }
+static inline int bd_register_pending_holders(struct gendisk *disk)
+{
+	return 0;
+}
 #endif /* CONFIG_BLOCK_HOLDER_DEPRECATED */
 
 dev_t part_devt(struct gendisk *disk, u8 partno);
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 5/8] dm: cleanup cleanup_mapped_device
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
                   ` (3 preceding siblings ...)
  2021-07-25  5:54 ` [dm-devel] [PATCH 4/8] block: support delayed holder registration Christoph Hellwig
@ 2021-07-25  5:54 ` Christoph Hellwig
  2021-07-29 16:33   ` Mike Snitzer
  2021-07-25  5:54 ` [dm-devel] [PATCH 6/8] dm: move setting md->type into dm_setup_md_queue Christoph Hellwig
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

md->queue is not always set when md->disk is set, so simplify the
conditionas a bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 2c5f9e585211..7971ec8ce677 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1694,13 +1694,9 @@ static void cleanup_mapped_device(struct mapped_device *md)
 		md->disk->private_data = NULL;
 		spin_unlock(&_minor_lock);
 		del_gendisk(md->disk);
-	}
-
-	if (md->queue)
 		dm_queue_destroy_keyslot_manager(md->queue);
-
-	if (md->disk)
 		blk_cleanup_disk(md->disk);
+	}
 
 	cleanup_srcu_struct(&md->io_barrier);
 
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 6/8] dm: move setting md->type into dm_setup_md_queue
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
                   ` (4 preceding siblings ...)
  2021-07-25  5:54 ` [dm-devel] [PATCH 5/8] dm: cleanup cleanup_mapped_device Christoph Hellwig
@ 2021-07-25  5:54 ` Christoph Hellwig
  2021-07-29 16:34   ` Mike Snitzer
  2021-07-25  5:54 ` [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

Move setting md->type from both callers into dm_setup_md_queue.
This ensures that md->type is only set to a valid value after the queue
has been fully setup, something we'll rely on future changes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-ioctl.c | 4 ----
 drivers/md/dm.c       | 5 +++--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 2209cbcd84db..2575074a2204 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1436,9 +1436,6 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si
 	}
 
 	if (dm_get_md_type(md) == DM_TYPE_NONE) {
-		/* Initial table load: acquire type of table. */
-		dm_set_md_type(md, dm_table_get_type(t));
-
 		/* setup md->queue to reflect md's type (may block) */
 		r = dm_setup_md_queue(md, t);
 		if (r) {
@@ -2187,7 +2184,6 @@ int __init dm_early_create(struct dm_ioctl *dmi,
 	if (r)
 		goto err_destroy_table;
 
-	md->type = dm_table_get_type(t);
 	/* setup md->queue to reflect md's type (may block) */
 	r = dm_setup_md_queue(md, t);
 	if (r) {
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 7971ec8ce677..f003bd5b93ce 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2052,9 +2052,9 @@ EXPORT_SYMBOL_GPL(dm_get_queue_limits);
  */
 int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
 {
-	int r;
+	enum dm_queue_mode type = dm_table_get_type(t);
 	struct queue_limits limits;
-	enum dm_queue_mode type = dm_get_md_type(md);
+	int r;
 
 	switch (type) {
 	case DM_TYPE_REQUEST_BASED:
@@ -2081,6 +2081,7 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
 	r = dm_table_set_restrictions(t, md->queue, &limits);
 	if (r)
 		return r;
+	md->type = type;
 
 	blk_register_queue(md->disk);
 
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
                   ` (5 preceding siblings ...)
  2021-07-25  5:54 ` [dm-devel] [PATCH 6/8] dm: move setting md->type into dm_setup_md_queue Christoph Hellwig
@ 2021-07-25  5:54 ` Christoph Hellwig
  2021-07-29 16:36   ` Mike Snitzer
  2021-07-25  5:54 ` [dm-devel] [PATCH 8/8] block: remove support for delayed queue registrations Christoph Hellwig
  2021-07-27 15:58 ` [dm-devel] use regular gendisk registration in device mapper Mike Snitzer
  8 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

device mapper is currently the only outlier that tries to call
register_disk after add_disk, leading to fairly inconsistent state
of these block layer data structures.  Instead change device-mapper
to just register the gendisk later now that the holder mechanism
can cope with that.

Note that this introduces a user visible change: the dm kobject is
now only visible after the initial table has been loaded.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-rq.c |  1 -
 drivers/md/dm.c    | 23 +++++++++++------------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 0dbd48cbdff9..5b95eea517d1 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -559,7 +559,6 @@ int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t)
 	err = blk_mq_init_allocated_queue(md->tag_set, md->queue);
 	if (err)
 		goto out_tag_set;
-	elevator_init_mq(md->queue);
 	return 0;
 
 out_tag_set:
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index f003bd5b93ce..7981b7287628 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1693,7 +1693,10 @@ static void cleanup_mapped_device(struct mapped_device *md)
 		spin_lock(&_minor_lock);
 		md->disk->private_data = NULL;
 		spin_unlock(&_minor_lock);
-		del_gendisk(md->disk);
+		if (dm_get_md_type(md) != DM_TYPE_NONE) {
+			dm_sysfs_exit(md);
+			del_gendisk(md->disk);
+		}
 		dm_queue_destroy_keyslot_manager(md->queue);
 		blk_cleanup_disk(md->disk);
 	}
@@ -1788,7 +1791,6 @@ static struct mapped_device *alloc_dev(int minor)
 			goto bad;
 	}
 
-	add_disk_no_queue_reg(md->disk);
 	format_dev_t(md->name, MKDEV(_major, minor));
 
 	md->wq = alloc_workqueue("kdmflush", WQ_MEM_RECLAIM, 0);
@@ -1989,19 +1991,12 @@ static struct dm_table *__unbind(struct mapped_device *md)
  */
 int dm_create(int minor, struct mapped_device **result)
 {
-	int r;
 	struct mapped_device *md;
 
 	md = alloc_dev(minor);
 	if (!md)
 		return -ENXIO;
 
-	r = dm_sysfs_init(md);
-	if (r) {
-		free_dev(md);
-		return r;
-	}
-
 	*result = md;
 	return 0;
 }
@@ -2081,10 +2076,15 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
 	r = dm_table_set_restrictions(t, md->queue, &limits);
 	if (r)
 		return r;
-	md->type = type;
 
-	blk_register_queue(md->disk);
+	add_disk(md->disk);
 
+	r = dm_sysfs_init(md);
+	if (r) {
+		del_gendisk(md->disk);
+		return r;
+	}
+	md->type = type;
 	return 0;
 }
 
@@ -2190,7 +2190,6 @@ static void __dm_destroy(struct mapped_device *md, bool wait)
 		DMWARN("%s: Forcibly removing mapped_device still in use! (%d users)",
 		       dm_device_name(md), atomic_read(&md->holders));
 
-	dm_sysfs_exit(md);
 	dm_table_destroy(__unbind(md));
 	free_dev(md);
 }
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 8/8] block: remove support for delayed queue registrations
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
                   ` (6 preceding siblings ...)
  2021-07-25  5:54 ` [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig
@ 2021-07-25  5:54 ` Christoph Hellwig
  2021-07-29 16:37   ` Mike Snitzer
  2021-07-27 15:58 ` [dm-devel] use regular gendisk registration in device mapper Mike Snitzer
  8 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-25  5:54 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

Now that device mapper has been changed to register the disk once
it is fully ready all this code is unused.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/elevator.c      |  1 -
 block/genhd.c         | 29 +++++++----------------------
 include/linux/genhd.h |  6 ------
 3 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index 52ada14cfe45..706d5a64508d 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -702,7 +702,6 @@ void elevator_init_mq(struct request_queue *q)
 		elevator_put(e);
 	}
 }
-EXPORT_SYMBOL_GPL(elevator_init_mq); /* only for dm-rq */
 
 /*
  * switch to new_e io scheduler. be careful not to introduce deadlocks -
diff --git a/block/genhd.c b/block/genhd.c
index e3d93b868ec5..3cd9f165a5a7 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -457,20 +457,20 @@ static void register_disk(struct device *parent, struct gendisk *disk,
 }
 
 /**
- * __device_add_disk - add disk information to kernel list
+ * device_add_disk - add disk information to kernel list
  * @parent: parent device for the disk
  * @disk: per-device partitioning information
  * @groups: Additional per-device sysfs groups
- * @register_queue: register the queue if set to true
  *
  * This function registers the partitioning information in @disk
  * with the kernel.
  *
  * FIXME: error handling
  */
-static void __device_add_disk(struct device *parent, struct gendisk *disk,
-			      const struct attribute_group **groups,
-			      bool register_queue)
+
+void device_add_disk(struct device *parent, struct gendisk *disk,
+		     const struct attribute_group **groups)
+
 {
 	int ret;
 
@@ -480,8 +480,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
 	 * elevator if one is needed, that is, for devices requesting queue
 	 * registration.
 	 */
-	if (register_queue)
-		elevator_init_mq(disk->queue);
+	elevator_init_mq(disk->queue);
 
 	/*
 	 * If the driver provides an explicit major number it also must provide
@@ -535,8 +534,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
 		bdev_add(disk->part0, dev->devt);
 	}
 	register_disk(parent, disk, groups);
-	if (register_queue)
-		blk_register_queue(disk);
+	blk_register_queue(disk);
 
 	/*
 	 * Take an extra ref on queue which will be put on disk_release()
@@ -550,21 +548,8 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
 	disk_add_events(disk);
 	blk_integrity_add(disk);
 }
-
-void device_add_disk(struct device *parent, struct gendisk *disk,
-		     const struct attribute_group **groups)
-
-{
-	__device_add_disk(parent, disk, groups, true);
-}
 EXPORT_SYMBOL(device_add_disk);
 
-void device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk)
-{
-	__device_add_disk(parent, disk, NULL, false);
-}
-EXPORT_SYMBOL(device_add_disk_no_queue_reg);
-
 /**
  * del_gendisk - remove the gendisk
  * @disk: the struct gendisk to remove
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index dd95d53c75fa..fbc4bf269f63 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -218,12 +218,6 @@ static inline void add_disk(struct gendisk *disk)
 {
 	device_add_disk(NULL, disk, NULL);
 }
-extern void device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk);
-static inline void add_disk_no_queue_reg(struct gendisk *disk)
-{
-	device_add_disk_no_queue_reg(NULL, disk);
-}
-
 extern void del_gendisk(struct gendisk *gp);
 
 void set_disk_ro(struct gendisk *disk, bool read_only);
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
                   ` (7 preceding siblings ...)
  2021-07-25  5:54 ` [dm-devel] [PATCH 8/8] block: remove support for delayed queue registrations Christoph Hellwig
@ 2021-07-27 15:58 ` Mike Snitzer
  2021-07-27 16:02   ` Christoph Hellwig
  8 siblings, 1 reply; 39+ messages in thread
From: Mike Snitzer @ 2021-07-27 15:58 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> Hi all,
> 
> The device mapper code currently has a somewhat odd gendisk registration
> scheme where it calls add_disk early, but uses a special flag to skip the
> "queue registration", which is a major part of add_disk.  This series
> improves the block layer holder tracking to work on an entirely
> unregistered disk and thus allows device mapper to use the normal scheme
> of calling add_disk when it is ready to accept I/O.
> 
> Note that this leads to a user visible change - the sysfs attributes on
> the disk and the dm directory hanging off it are not only visible once
> the initial table is loaded.

s/not/now/

> This did not make a different to my testing
> using dmsetup and the lvm2 tools.

I'll try these changes running through the lvm2 testsuite.

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-27 15:58 ` [dm-devel] use regular gendisk registration in device mapper Mike Snitzer
@ 2021-07-27 16:02   ` Christoph Hellwig
  2021-07-27 16:18     ` Mike Snitzer
  0 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-27 16:02 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: Jens Axboe, linux-block, dm-devel, Christoph Hellwig

On Tue, Jul 27, 2021 at 11:58:46AM -0400, Mike Snitzer wrote:
> > This did not make a different to my testing
> > using dmsetup and the lvm2 tools.
> 
> I'll try these changes running through the lvm2 testsuite.

Btw, is ther documentation on how to run it somewhere?  I noticed
tests, old-tests and unit-tests directories, but no obvious way
to run them.

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 4/8] block: support delayed holder registration
  2021-07-25  5:54 ` [dm-devel] [PATCH 4/8] block: support delayed holder registration Christoph Hellwig
@ 2021-07-27 16:06   ` Mike Snitzer
  2021-07-28  6:54     ` Christoph Hellwig
  2021-07-29 16:32   ` Mike Snitzer
  1 sibling, 1 reply; 39+ messages in thread
From: Mike Snitzer @ 2021-07-27 16:06 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> device mapper needs to register holders before it is ready to do I/O.
> Currently it does so by registering the disk early, which has all kinds
> of bad side effects.  Support registering holders on an initialized but
> not registered disk instead by delaying the sysfs registration until the
> disk is registered.

This header starts to shine some light on what is motivating this
series by touching on "all kinds of bad side effects" being fixed.
Any chance you could elaborate what you've noticed/found/hit?

Mike

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-27 16:02   ` Christoph Hellwig
@ 2021-07-27 16:18     ` Mike Snitzer
  2021-07-27 20:38       ` Milan Broz
  2021-07-27 22:52       ` Mike Snitzer
  0 siblings, 2 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-27 16:18 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Tue, Jul 27 2021 at 12:02P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> On Tue, Jul 27, 2021 at 11:58:46AM -0400, Mike Snitzer wrote:
> > > This did not make a different to my testing
> > > using dmsetup and the lvm2 tools.
> > 
> > I'll try these changes running through the lvm2 testsuite.
> 
> Btw, is ther documentation on how to run it somewhere?  I noticed
> tests, old-tests and unit-tests directories, but no obvious way
> to run them.

I haven't tracked how it has changed in a while, but I always run:
make check_local

(but to do that you first need to ./configure how your distro does
it... so that all targets are enabled, etc. Then: make).

Will revisit this shortly and let you know if my process needed to
change at all due to lvm2 changes.

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-27 16:18     ` Mike Snitzer
@ 2021-07-27 20:38       ` Milan Broz
  2021-07-28  7:06         ` Christoph Hellwig
  2021-07-28 16:17         ` Mike Snitzer
  2021-07-27 22:52       ` Mike Snitzer
  1 sibling, 2 replies; 39+ messages in thread
From: Milan Broz @ 2021-07-27 20:38 UTC (permalink / raw)
  To: Mike Snitzer, Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On 27/07/2021 18:18, Mike Snitzer wrote:
> On Tue, Jul 27 2021 at 12:02P -0400,
> Christoph Hellwig <hch@lst.de> wrote:
> 
>> On Tue, Jul 27, 2021 at 11:58:46AM -0400, Mike Snitzer wrote:
>>>> This did not make a different to my testing
>>>> using dmsetup and the lvm2 tools.
>>>
>>> I'll try these changes running through the lvm2 testsuite.
>>
>> Btw, is ther documentation on how to run it somewhere?  I noticed
>> tests, old-tests and unit-tests directories, but no obvious way
>> to run them.
> 
> I haven't tracked how it has changed in a while, but I always run:
> make check_local
> 
> (but to do that you first need to ./configure how your distro does
> it... so that all targets are enabled, etc. Then: make).
> 
> Will revisit this shortly and let you know if my process needed to
> change at all due to lvm2 changes.

BTW it would be also nice to run cryptsetup testsuite as root - we do a lot
of DM operations there (and we depend on sysfs on some places).

You can just run configure, make and then make check.

Thanks,
Milan

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-27 16:18     ` Mike Snitzer
  2021-07-27 20:38       ` Milan Broz
@ 2021-07-27 22:52       ` Mike Snitzer
  1 sibling, 0 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-27 22:52 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Tue, Jul 27 2021 at 12:18P -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Tue, Jul 27 2021 at 12:02P -0400,
> Christoph Hellwig <hch@lst.de> wrote:
> 
> > On Tue, Jul 27, 2021 at 11:58:46AM -0400, Mike Snitzer wrote:
> > > > This did not make a different to my testing
> > > > using dmsetup and the lvm2 tools.
> > > 
> > > I'll try these changes running through the lvm2 testsuite.
> > 
> > Btw, is ther documentation on how to run it somewhere?  I noticed
> > tests, old-tests and unit-tests directories, but no obvious way
> > to run them.
> 
> I haven't tracked how it has changed in a while, but I always run:
> make check_local
> 
> (but to do that you first need to ./configure how your distro does
> it... so that all targets are enabled, etc. Then: make).
> 
> Will revisit this shortly and let you know if my process needed to
> change at all due to lvm2 changes.

Yeap, same process as I described above.

Same 6 tests fail in the lvm2 testsuite with or without your changes,
so nothing to do with your changes.

I'll review your patches closer tomorrow (first pass it all looked
pretty good).

Mike

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 4/8] block: support delayed holder registration
  2021-07-27 16:06   ` Mike Snitzer
@ 2021-07-28  6:54     ` Christoph Hellwig
  0 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-28  6:54 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: Jens Axboe, linux-block, dm-devel, Christoph Hellwig

On Tue, Jul 27, 2021 at 12:06:04PM -0400, Mike Snitzer wrote:
> This header starts to shine some light on what is motivating this
> series by touching on "all kinds of bad side effects" being fixed.
> Any chance you could elaborate what you've noticed/found/hit?

The proble mis that it leaves the queue in a weird half state.  The
normal states for a gendisk are:

 1) allocated		(after *alloc_disk)
 2) registered		(after add_disk*)
 3) unregistered	(after del_gendisk)

the delayed queue registration adds a weird half state where it is
sort of registered, except for in sysfs and the elevator.  I have
some pretty big changes between how the disk and queue interact
that tripped over it, but even right now code has to be very careful
in the takedown path to deal with the half-initialized disks.

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-27 20:38       ` Milan Broz
@ 2021-07-28  7:06         ` Christoph Hellwig
  2021-07-28  8:37           ` Milan Broz
  2021-07-28 16:17         ` Mike Snitzer
  1 sibling, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-28  7:06 UTC (permalink / raw)
  To: Milan Broz
  Cc: Jens Axboe, linux-block, dm-devel, Christoph Hellwig, Mike Snitzer

On Tue, Jul 27, 2021 at 10:38:16PM +0200, Milan Broz wrote:
> BTW it would be also nice to run cryptsetup testsuite as root - we do a lot
> of DM operations there (and we depend on sysfs on some places).

It already doesn't seem very happy in current mainline for me:

=======================
13 of 17 tests failed
(12 tests were not run)
=======================

but this series doesn't seem to change anything.

A lot of the not run tests seem to be due to broken assumptions
that some code must be modular.  E.g. my kernel has scsi_debug built
in, but it complains like this:

modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/module'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc3+

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-28  7:06         ` Christoph Hellwig
@ 2021-07-28  8:37           ` Milan Broz
  2021-07-28 11:24             ` Christoph Hellwig
  0 siblings, 1 reply; 39+ messages in thread
From: Milan Broz @ 2021-07-28  8:37 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel, Mike Snitzer

On 28/07/2021 09:06, Christoph Hellwig wrote:
> On Tue, Jul 27, 2021 at 10:38:16PM +0200, Milan Broz wrote:
>> BTW it would be also nice to run cryptsetup testsuite as root - we do a lot
>> of DM operations there (and we depend on sysfs on some places).
> 
> It already doesn't seem very happy in current mainline for me:
> 
> =======================
> 13 of 17 tests failed
> (12 tests were not run)
> =======================
> 
> but this series doesn't seem to change anything.
> 
> A lot of the not run tests seem to be due to broken assumptions
> that some code must be modular.  E.g. my kernel has scsi_debug built
> in, but it complains like this:
> 
> modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/module'
> modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc3+

Hi,

there should not be many assumptions, but yes, we depend on modular scsi_debug in some tests because we simulate
very specific hw attributes. So you have one emulated device compiled-in?

Or there is another way how to configure scsi_debug if compiled-in? (we use module parameters, I think it is
the same was how util-linux testsute works with scsi_debug).

Anyway, this is a bug, tests should be skipped (the same way if scsi_debug is not available).

I forgot to say - there is a list of packages that should be installed for make check mentioned
in README.md - I guess this was the reason some other tests were skipped.

(BTW could you send me output of the failed test run? I run it over Linus' tree and ti works so it is perhaps another
assumption that should be fixed.)

Thanks,
Milan

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-28  8:37           ` Milan Broz
@ 2021-07-28 11:24             ` Christoph Hellwig
  2021-07-29 15:01               ` Milan Broz
  0 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2021-07-28 11:24 UTC (permalink / raw)
  To: Milan Broz
  Cc: Jens Axboe, linux-block, dm-devel, Christoph Hellwig, Mike Snitzer

On Wed, Jul 28, 2021 at 10:37:41AM +0200, Milan Broz wrote:
> very specific hw attributes. So you have one emulated device compiled-in?

Yes.

> Or there is another way how to configure scsi_debug if compiled-in? (we use module parameters, I think it is
> the same was how util-linux testsute works with scsi_debug).

Can can add hosts using the add_host sysfs file.  I thought that was the
way to go generally, never thought of reloading the module just to
add/delete hosts.

> (BTW could you send me output of the failed test run? I run it over Linus' tree and ti works so it is perhaps another
> assumption that should be fixed.)

Output with everything from the README installed (a lot less failures now):


Making check in po
make[1]: Entering directory '/root/cryptsetup/po'
make[1]: Nothing to be done for 'check'.
make[1]: Leaving directory '/root/cryptsetup/po'
Making check in tests
make[1]: Entering directory '/root/cryptsetup/tests'
make  check-am
make[2]: Entering directory '/root/cryptsetup/tests'
make  api-test api-test-2 differ vectors-test unit-utils-io all-symbols-test
make[3]: Entering directory '/root/cryptsetup/tests'
make[3]: 'api-test' is up to date.
make[3]: 'api-test-2' is up to date.
make[3]: 'differ' is up to date.
make[3]: 'vectors-test' is up to date.
make[3]: 'unit-utils-io' is up to date.
make[3]: 'all-symbols-test' is up to date.
make[3]: Leaving directory '/root/cryptsetup/tests'
make  check-TESTS
make[3]: Entering directory '/root/cryptsetup/tests'
Cryptsetup test environment (Wed Jul 28 10:59:13 UTC 2021)
Linux testvm 5.14.0-rc2+ #53 SMP PREEMPT Wed Jul 28 12:57:30 CEST 2021 x86_64 GNU/Linux
Debian GNU/Linux 10 (buster) (Debian GNU/Linux) 10 (buster)
Memory
              total        used        free      shared  buff/cache   available
Mem:          3.8Gi       173Mi       3.5Gi       9.0Mi       157Mi       3.6Gi
Swap:            0B          0B          0B
../cryptsetup 2.4.0-rc0
../veritysetup 2.4.0-rc0
../integritysetup 2.4.0-rc0
../cryptsetup-reencrypt 2.4.0-rc0
Cryptsetup defaults:
Default compiled-in metadata format is LUKS2 (for luksFormat action).

LUKS2 external token plugin support is compiled-in.
LUKS2 external token plugin path: /usr/lib/cryptsetup.

Default compiled-in key and passphrase parameters:
	Maximum keyfile size: 8192kB, Maximum interactive passphrase length 512 (characters)
Default PBKDF for LUKS1: pbkdf2, iteration time: 2000 (ms)
Default PBKDF for LUKS2: argon2id
	Iteration time: 2000, Memory required: 1048576kB, Parallel threads: 4

Default compiled-in device cipher parameters:
	loop-AES: aes, Key 256 bits
	plain: aes-cbc-essiv:sha256, Key: 256 bits, Password hashing: ripemd160
	LUKS: aes-xts-plain64, Key: 256 bits, LUKS header hashing: sha256, RNG: /dev/urandom
	LUKS: Default keysize with XTS mode (two internal keys) will be doubled.
Library version:   1.02.155 (2018-12-18)
Driver version:    4.45.0
Device mapper targets:
thin-pool        v1.22.0
thin             v1.22.0
zero             v1.1.0
mirror           v1.14.0
snapshot-merge   v1.5.0
snapshot-origin  v1.9.0
snapshot         v1.16.0
multipath        v1.14.0
crypt            v1.23.0
striped          v1.6.0
linear           v1.4.0
error            v1.5.0
PASS: 00modules-test
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module dm-crypt not found in directory /lib/modules/5.14.0-rc2+
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module dm-verity not found in directory /lib/modules/5.14.0-rc2+
NonFIPSAlg: Crypto is properly initialised in format
AddDevicePlain: A plain device API creation
HashDevicePlain: A plain device API hash
AddDeviceLuks: Format and use LUKS device
LuksHeaderLoad: Header load
LuksHeaderRestore: LUKS header restore
LuksHeaderBackup: LUKS header backup
ResizeDeviceLuks: LUKS device resize
UseLuksDevice: Use pre-formated LUKS device
SuspendDevice: Suspend/Resume
UseTempVolumes: Format and use temporary encrypted device
CallbacksTest: API callbacks
VerityTest: DM verity
WARNING: kernel dm-verity not supported, skipping test.
TcryptTest: Tcrypt API
WARNING: algif_skcipher interface not present, skipping test.
IntegrityTest: Integrity API
WARNING: cannot format integrity device, skipping test.
PASS: api-test
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module dm-crypt not found in directory /lib/modules/5.14.0-rc2+
Cannot set test devices.
SKIP: api-test-2
[1] Current state
PASS: compat-test-args
CASE: Image in file tests (root capabilities not required)
[1] format
[2] open
[3] add key
[4] change key
[5] remove key
[6] kill slot
[7] header backup
[8] header restore
[9] luksDump
[10] uuid
CASE: [1] open - compat image - acceptance check
CASE: [2] open - compat image - denial check
CASE: [3] format
CASE: [4] format using hash sha512
CASE: [5] open
CASE: [6] add key
CASE: [7] unsuccessful delete
CASE: [8] successful delete
CASE: [9] add key test for key files
CASE: [10] delete key test with key1 as remaining key
CASE: [11] delete last key
CASE: [12] parameter variation test
CASE: [13] open/close - stacked devices
CASE: [14] format/open - passphrase on stdin & new line
CASE: [15] UUID - use and report provided UUID
CASE: [16] luksFormat
CASE: [17] AddKey volume key, passphrase and keyfile
CASE: [18] RemoveKey passphrase and keyfile
CASE: [19] create & status & resize
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
This kernel seems to not support proper scsi_debug module, test skipped.
SKIP: compat-test
CASE: [0] Detect LUKS2 environment
CASE: [1] Data offset
CASE: [2] Sector size and old payload alignment
CASE: [3] format
CASE: [4] format using hash sha512
CASE: [5] open
CASE: [6] add key
CASE: [7] unsuccessful delete
CASE: [8] successful delete
CASE: [9] add key test for key files
CASE: [10] delete key test with key1 as remaining key
CASE: [11] delete last key
CASE: [12] parameter variation test
CASE: [13] open/close - stacked devices
CASE: [14] format/open - passphrase on stdin & new line
CASE: [15] UUID - use and report provided UUID
CASE: [16] luksFormat
CASE: [17] AddKey volume key, passphrase and keyfile
CASE: [18] RemoveKey passphrase and keyfile
CASE: [19] create & status & resize
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
This kernel seems to not support proper scsi_debug module, test skipped.
SKIP: compat-test2
Open loop-AES key_v1 / AES-128 / offset 0 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-128 / offset 8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-128 / offset @8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-128 / offset 8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-128 / offset @8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-128 / offset 0 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-128 / offset 8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-128 / offset @8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-128 / offset 8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-128 / offset @8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-128 / offset 0 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-128 / offset 8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-128 / offset @8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-128 / offset 8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-128 / offset @8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-256 / offset 0 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-256 / offset 8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-256 / offset @8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-256 / offset 8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v1 / AES-256 / offset @8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-256 / offset 0 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-256 / offset 8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-256 / offset @8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-256 / offset 8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v2 / AES-256 / offset @8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-256 / offset 0 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-256 / offset 8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-256 / offset @8192 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-256 / offset 8388608 [keyfile:OK][stdin:OK]
Open loop-AES key_v3 / AES-256 / offset @8388608 [keyfile:OK][stdin:OK]
PASS: loopaes-test
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
SKIP: align-test
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
SKIP: align-test2
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
SKIP: discards-test
aes                            PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-plain                      PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
null                           PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
cipher_null                    PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
cipher_null-ecb                PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-ecb                        PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
twofish-ecb                    PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-ecb                    PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
aes-cbc-null                   PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-cbc-benbi                  PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-cbc-plain                  PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-cbc-plain64                PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-cbc-essiv:sha256           PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-lrw-null                   PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
aes-lrw-benbi                  PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
aes-lrw-plain                  PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
aes-lrw-plain64                PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
aes-lrw-essiv:sha256           PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
aes-xts-null                   PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-xts-benbi                  PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-xts-plain                  PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-xts-plain64                PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
aes-xts-essiv:sha256           PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] LUKS2:[table OK][status OK] CHECKSUM:[OK]
twofish-cbc-null               PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-cbc-benbi              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-cbc-plain              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-cbc-plain64            PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-cbc-essiv:sha256       PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-lrw-null               PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-lrw-benbi              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-lrw-plain              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-lrw-plain64            PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-lrw-essiv:sha256       PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-xts-null               PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-xts-benbi              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-xts-plain              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-xts-plain64            PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
twofish-xts-essiv:sha256       PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-cbc-null               PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-cbc-benbi              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-cbc-plain              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-cbc-plain64            PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-cbc-essiv:sha256       PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-lrw-null               PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-lrw-benbi              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-lrw-plain              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-lrw-plain64            PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-lrw-essiv:sha256       PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-xts-null               PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-xts-benbi              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-xts-plain              PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-xts-plain64            PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
serpent-xts-essiv:sha256       PLAIN:[table OK][status OK] LUKS1:[table OK][status OK] CHECKSUM:[OK]
xchacha12,aes-adiantum-plain64 [n/a]
xchacha20,aes-adiantum-plain64 [n/a]
PASS: mode-test
HASH: ripemd160 KSIZE: 0 / pwd [OK]
HASH: ripemd160 KSIZE: 256 / pwd [OK]
HASH: ripemd160 KSIZE: 128 / pwd [OK]
HASH: sha1 KSIZE: 256 / pwd [OK]
HASH: sha1 KSIZE: 128 / pwd [OK]
HASH: sha256 KSIZE: 256 / pwd [OK]
HASH: sha256 KSIZE: 128 / pwd [OK]
HASH: sha256 KSIZE: 0 / std- [OK]
HASH: sha256 KSIZE: 256 / std- [OK]
HASH: sha256 KSIZE: 128 / std- [OK]
HASH: sha256 KSIZE: 256 / stdin [OK]
HASH: sha256 KSIZE: 0 / stdin [OK]
HASH: ripemd160 KSIZE: 256 / file [OK]
HASH: sha256 KSIZE: 256 / file [OK]
HASH: unknown* KSIZE: 256 / file [OK]
HASH: sha256:20 KSIZE: 256 / pwd [OK]
HASH: sha256:32 KSIZE: 256 / pwd [OK]
HASH: sha256: KSIZE: 256 / failpwd [OK]
HASH: sha256:xx KSIZE: 256 / failpwd [OK]
HASH: ripemd160 KSIZE: 256 / file [OK]
HASH: sha256 KSIZE: 256 / file [OK]
HASH: sha256 KSIZE: 128 / file [OK]
HASH: sha256 KSIZE: 512 / file [OK]
HASH: plain KSIZE: 128 / cat [OK]
HASH: plain KSIZE: 128 / cat [OK]
HASH: plain KSIZE: 128 / cat [OK]
HASH: plain KSIZE: 128 / cat- [OK]
HASH: plain KSIZE: 128 / cat- [OK]
HASH: sha256 KSIZE: 128 / cat- [OK]
HASH: sha256 KSIZE: 128 / cat- [OK]
HASH: sha256 KSIZE: 128 / cat- [OK]
HASH: sha256 KSIZE: 128 / cat- [OK]
HASH: sha256 KSIZE: 128 / cat- [OK]
HASH: sha256 KSIZE: 128 / cat- [OK]
HASH: plain KSIZE: 256 / pwd [OK]
HASH: plain:2 KSIZE: 256 / pwd [OK]
HASH: plain:9 KSIZE: 256 / failpwd [OK]
HASH: sha256 KSIZE: 128 / cat [OK]
HASH: sha256:14 KSIZE: 128 / cat [OK]
HASH: sha256 KSIZE: 128 / pwd [OK]
HASH: sha256 KSIZE: 128 / pwd [OK]
HASH: sha256 KSIZE: 128 / pwd [OK]
HASH: sha1 KSIZE: 256 / pwd [OK]
HASH: sha224 KSIZE: 256 / pwd [OK]
HASH: sha256 KSIZE: 256 / pwd [OK]
HASH: sha384 KSIZE: 256 / pwd [OK]
HASH: sha512 KSIZE: 256 / pwd [OK]
HASH: ripemd160 KSIZE: 256 / pwd [OK]
HASH: whirlpool KSIZE: 256 / pwd [OK]
HASH: sha3-224 KSIZE: 256 / pwd [OK]
HASH: sha3-256 KSIZE: 256 / pwd [OK]
HASH: sha3-384 KSIZE: 256 / pwd [OK]
HASH: sha3-512 KSIZE: 256 / pwd [OK]
HASH: sm3 KSIZE: 256 / pwd [OK]
HASH: stribog512 KSIZE: 256 / pwd [N/A] (1, SKIPPED)
PASS: password-hash-test
REQUIRED KDF TEST
pbkdf2-sha256 [OK]
pbkdf2-sha512 [OK]
pbkdf2-ripemd160 [OK]
pbkdf2-whirlpool [OK]
pbkdf2-stribog512 [N/A]
REQUIRED CIPHERS TEST
aes-cbc [N/A]
aes-lrw [N/A]
aes-xts [N/A]
twofish-ecb [N/A]
twofish-cbc [N/A]
twofish-lrw [N/A]
twofish-xts [N/A]
serpent-ecb [N/A]
serpent-cbc [N/A]
serpent-lrw [N/A]
serpent-xts [N/A]
blowfish-cbc [N/A]
des3_ede-cbc [N/A]
cast5-cbc [N/A]
camellia-xts [N/A]
kuznyechik-xts [N/A]
No remaining images.
Test skipped.
SKIP: tcrypt-compat-test
REQUIRED KDF TEST
REQUIRED CIPHERS TEST
#     Algorithm | Key |  Encryption |  Decryption
Cipher aes-xts (with 256 bits key) is not available.
Required kernel crypto interface not available.
Ensure you have algif_skcipher kernel module loaded.
Test skipped.
SKIP: luks1-compat-test
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module dm-crypt not found in directory /lib/modules/5.14.0-rc2+
FAIL dm-crypt failed to load
FAILED backtrace:
51 ./device-test
122 main ./device-test
FAIL: device-test
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module dm-crypt not found in directory /lib/modules/5.14.0-rc2+
dm-crypt failed to load
FAILED backtrace:
81 ./keyring-test
FAIL: keyring-test
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module dm-crypt not found in directory /lib/modules/5.14.0-rc2+
dm-crypt failed to load
FAILED backtrace:
117 ./keyring-compat-test
FAIL: keyring-compat-test
[0] Generating test headers
generate-luks2-area-in-json-hdr-space-json0.img.sh...done
generate-luks2-argon2-leftover-params.img.sh...done
generate-luks2-correct-full-json0.img.sh...done
generate-luks2-corrupted-hdr0-with-correct-chks.img.sh...done
generate-luks2-corrupted-hdr1-with-correct-chks.img.sh...done
generate-luks2-invalid-checksum-both-hdrs.img.sh...done
generate-luks2-invalid-checksum-hdr0.img.sh...done
generate-luks2-invalid-checksum-hdr1.img.sh...done
generate-luks2-invalid-json-size-c0.img.sh...done
generate-luks2-invalid-json-size-c1.img.sh...done
generate-luks2-invalid-json-size-c2.img.sh...done
generate-luks2-invalid-keyslots-size-c0.img.sh...done
generate-luks2-invalid-keyslots-size-c1.img.sh...done
generate-luks2-invalid-keyslots-size-c2.img.sh...done
generate-luks2-invalid-object-type-json0.img.sh...done
generate-luks2-invalid-opening-char-json0.img.sh...done
generate-luks2-keyslot-missing-digest.img.sh...done
generate-luks2-keyslot-too-many-digests.img.sh...done
generate-luks2-metadata-size-128k-secondary.img.sh...done
generate-luks2-metadata-size-128k.img.sh...done
generate-luks2-metadata-size-16k-secondary.img.sh...done
generate-luks2-metadata-size-1m-secondary.img.sh...done
generate-luks2-metadata-size-1m.img.sh...done
generate-luks2-metadata-size-256k-secondary.img.sh...done
generate-luks2-metadata-size-256k.img.sh...done
generate-luks2-metadata-size-2m-secondary.img.sh...done
generate-luks2-metadata-size-2m.img.sh...done
generate-luks2-metadata-size-32k-secondary.img.sh...done
generate-luks2-metadata-size-32k.img.sh...done
generate-luks2-metadata-size-4m-secondary.img.sh...done
generate-luks2-metadata-size-4m.img.sh...done
generate-luks2-metadata-size-512k-secondary.img.sh...done
generate-luks2-metadata-size-512k.img.sh...done
generate-luks2-metadata-size-64k-inv-area-c0.img.sh...done
generate-luks2-metadata-size-64k-inv-area-c1.img.sh...done
generate-luks2-metadata-size-64k-inv-keyslots-size-c0.img.sh...done
generate-luks2-metadata-size-64k-secondary.img.sh...done
generate-luks2-metadata-size-64k.img.sh...done
generate-luks2-missing-keyslot-referenced-in-digest.img.sh...done
generate-luks2-missing-keyslot-referenced-in-token.img.sh...done
generate-luks2-missing-segment-referenced-in-digest.img.sh...done
generate-luks2-missing-trailing-null-byte-json0.img.sh...done
generate-luks2-non-null-byte-beyond-json0.img.sh...done
generate-luks2-non-null-bytes-beyond-json0.img.sh...done
generate-luks2-overlapping-areas-c0-json0.img.sh...done
generate-luks2-overlapping-areas-c1-json0.img.sh...done
generate-luks2-overlapping-areas-c2-json0.img.sh...done
generate-luks2-pbkdf2-leftover-params-0.img.sh...done
generate-luks2-pbkdf2-leftover-params-1.img.sh...done
generate-luks2-segment-crypt-missing-encryption.img.sh...done
generate-luks2-segment-crypt-missing-ivoffset.img.sh...done
generate-luks2-segment-crypt-missing-sectorsize.img.sh...done
generate-luks2-segment-crypt-wrong-encryption.img.sh...done
generate-luks2-segment-crypt-wrong-ivoffset.img.sh...done
generate-luks2-segment-crypt-wrong-sectorsize-0.img.sh...done
generate-luks2-segment-crypt-wrong-sectorsize-1.img.sh...done
generate-luks2-segment-crypt-wrong-sectorsize-2.img.sh...done
generate-luks2-segment-missing-offset.img.sh...done
generate-luks2-segment-missing-size.img.sh...done
generate-luks2-segment-missing-type.img.sh...done
generate-luks2-segment-two.img.sh...done
generate-luks2-segment-unknown-type.img.sh...done
generate-luks2-segment-wrong-backup-key-0.img.sh...done
generate-luks2-segment-wrong-backup-key-1.img.sh...done
generate-luks2-segment-wrong-flags-element.img.sh...done
generate-luks2-segment-wrong-flags.img.sh...done
generate-luks2-segment-wrong-offset.img.sh...done
generate-luks2-segment-wrong-size-0.img.sh...done
generate-luks2-segment-wrong-size-1.img.sh...done
generate-luks2-segment-wrong-size-2.img.sh...done
generate-luks2-segment-wrong-type.img.sh...done
generate-luks2-uint64-max-segment-size.img.sh...done
generate-luks2-uint64-overflow-segment-size.img.sh...done
generate-luks2-uint64-signed-segment-size.img.sh...done
[1] Test basic auto-recovery
Test image: luks2-invalid-checksum-hdr0.img...OK
Test image: luks2-invalid-checksum-hdr1.img...OK
Test image: luks2-invalid-checksum-both-hdrs.img...OK
[2] Test ability to auto-correct mallformed json area
Test image: luks2-corrupted-hdr0-with-correct-chks.img...OK
Test image: luks2-corrupted-hdr1-with-correct-chks.img...OK
Test image: luks2-correct-full-json0.img...OK
Test image: luks2-argon2-leftover-params.img...OK
Test image: luks2-pbkdf2-leftover-params-0.img...OK
Test image: luks2-pbkdf2-leftover-params-1.img...OK
[3] Test LUKS2 json area restrictions
Test image: luks2-non-null-byte-beyond-json0.img...OK
Test image: luks2-non-null-bytes-beyond-json0.img...OK
Test image: luks2-missing-trailing-null-byte-json0.img...OK
Test image: luks2-invalid-opening-char-json0.img...OK
Test image: luks2-invalid-object-type-json0.img...OK
Test image: luks2-overlapping-areas-c0-json0.img...OK
Test image: luks2-overlapping-areas-c1-json0.img...OK
Test image: luks2-overlapping-areas-c2-json0.img...OK
Test image: luks2-area-in-json-hdr-space-json0.img...OK
Test image: luks2-missing-keyslot-referenced-in-digest.img...OK
Test image: luks2-missing-segment-referenced-in-digest.img...OK
Test image: luks2-missing-keyslot-referenced-in-token.img...OK
Test image: luks2-keyslot-missing-digest.img...OK
Test image: luks2-keyslot-too-many-digests.img...OK
[4] Test integers value limits
Test image: luks2-uint64-max-segment-size.img...OK
Test image: luks2-uint64-overflow-segment-size.img...OK
Test image: luks2-uint64-signed-segment-size.img...OK
[5] Test segments validation
Test image: luks2-segment-missing-type.img...OK
Test image: luks2-segment-wrong-type.img...OK
Test image: luks2-segment-missing-offset.img...OK
Test image: luks2-segment-wrong-offset.img...OK
Test image: luks2-segment-missing-size.img...OK
Test image: luks2-segment-wrong-size-0.img...OK
Test image: luks2-segment-wrong-size-1.img...OK
Test image: luks2-segment-wrong-size-2.img...OK
Test image: luks2-segment-crypt-missing-encryption.img...OK
Test image: luks2-segment-crypt-wrong-encryption.img...OK
Test image: luks2-segment-crypt-missing-ivoffset.img...OK
Test image: luks2-segment-crypt-wrong-ivoffset.img...OK
Test image: luks2-segment-crypt-missing-sectorsize.img...OK
Test image: luks2-segment-crypt-wrong-sectorsize-0.img...OK
Test image: luks2-segment-crypt-wrong-sectorsize-1.img...OK
Test image: luks2-segment-crypt-wrong-sectorsize-2.img...OK
Test image: luks2-segment-unknown-type.img...OK
Test image: luks2-segment-two.img...OK
Test image: luks2-segment-wrong-flags.img...OK
Test image: luks2-segment-wrong-flags-element.img...OK
Test image: luks2-segment-wrong-backup-key-0.img...OK
Test image: luks2-segment-wrong-backup-key-1.img...OK
[6] Test metadata size and keyslots size (config section)
Test image: luks2-invalid-keyslots-size-c0.img...OK
Test image: luks2-invalid-keyslots-size-c1.img...OK
Test image: luks2-invalid-keyslots-size-c2.img...OK
Test image: luks2-invalid-json-size-c0.img...OK
Test image: luks2-invalid-json-size-c1.img...OK
Test image: luks2-invalid-json-size-c2.img...OK
Test image: luks2-metadata-size-32k.img...OK
Test image: luks2-metadata-size-64k.img...OK
Test image: luks2-metadata-size-64k-inv-area-c0.img...OK
Test image: luks2-metadata-size-64k-inv-area-c1.img...OK
Test image: luks2-metadata-size-64k-inv-keyslots-size-c0.img...OK
Test image: luks2-metadata-size-128k.img...OK
Test image: luks2-metadata-size-256k.img...OK
Test image: luks2-metadata-size-512k.img...OK
Test image: luks2-metadata-size-1m.img...OK
Test image: luks2-metadata-size-2m.img...OK
Test image: luks2-metadata-size-4m.img...OK
Test image: luks2-metadata-size-16k-secondary.img...OK
Test image: luks2-metadata-size-32k-secondary.img...OK
Test image: luks2-metadata-size-64k-secondary.img...OK
Test image: luks2-metadata-size-128k-secondary.img...OK
Test image: luks2-metadata-size-256k-secondary.img...OK
Test image: luks2-metadata-size-512k-secondary.img...OK
Test image: luks2-metadata-size-1m-secondary.img...OK
Test image: luks2-metadata-size-2m-secondary.img...OK
Test image: luks2-metadata-size-4m-secondary.img...OK
PASS: luks2-validation-test
Cannot find dm-integrity target, test skipped.
SKIP: luks2-integrity-test
Test vectors using OpenSSL 1.1.1d  10 Sep 2019 crypto backend.
PBKDF vector 00 argon2i [OK]
PBKDF vector 01 argon2id [OK]
PBKDF vector 02 pbkdf2 [OK]
PBKDF vector 03 pbkdf2 [OK]
PBKDF vector 04 pbkdf2 [OK]
PBKDF vector 05 pbkdf2 [OK]
PBKDF vector 06 pbkdf2 [OK]
PBKDF vector 07 pbkdf2 [OK]
PBKDF vector 08 pbkdf2 [OK]
PBKDF vector 09 pbkdf2 [OK]
PBKDF vector 10 pbkdf2 [OK]
PBKDF vector 11 pbkdf2 [OK]
PBKDF vector 12 pbkdf2 [OK]
PBKDF vector 13 pbkdf2 [OK]
PBKDF vector 14 pbkdf2 [OK]
PBKDF vector 15 pbkdf2 [OK]
PBKDF vector 16 pbkdf2 [OK]
PBKDF vector 17 pbkdf2 [OK]
PBKDF vector 18 pbkdf2 [OK]
Hash vector 00: [crc32][sha1][sha256][sha512][ripemd160][whirlpool][blake2b-512][blake2s-256]
Hash vector 01: [crc32][sha1][sha256][sha512][ripemd160][whirlpool][blake2b-512][blake2s-256]
Hash vector 02: [crc32][sha1][sha256][sha512][ripemd160][whirlpool][blake2b-512][blake2s-256]
Hash vector 03: [crc32][sha1][sha256][sha512][ripemd160][whirlpool][blake2b-512][blake2s-256]
Hash vector 04: [crc32][sha1][sha256][sha512][ripemd160][whirlpool][blake2b-512][blake2s-256]
Hash vector 05: [crc32][sha1][sha256][sha512][ripemd160][whirlpool][blake2b-512][blake2s-256]
Hash vector 06: [crc32][sha1][sha256][sha512][ripemd160][whirlpool][blake2b-512][blake2s-256]
HMAC vector 00: [sha1][sha256][sha512]
HMAC vector 01: [sha1][sha256][sha512]
HMAC vector 02: [sha1][sha256][sha512]
HMAC vector 03: [sha1][sha256][sha512]
HMAC vector 04: [sha1][sha256][sha512]
HMAC vector 05: [sha1][sha256][sha512]
CIPHER vector 00: [aes-ecb,128bits][serpent-ecb N/A]
CIPHER vector 01: [aes-cbc,128bits][serpent-cbc N/A]
CIPHER vector 02: [aes-ecb,256bits][serpent-ecb N/A]
CIPHER vector 03: [aes-cbc,256bits][serpent-cbc N/A]
CIPHER vector 04: [aes-xts,256bits][serpent-xts N/A]
CIPHER vector 05: [aes-xts,512bits][serpent-xts N/A]
CIPHER vector 06: [xchacha12,aes-adiantum N/A][xchacha20,aes-adiantum N/A]
IV vector 00: [aes-cbc-null][512][1024][1024L][2048][2048L][4096][4096L]
IV vector 01: [aes-cbc-plain][512][1024][1024L][2048][2048L][4096][4096L]
IV vector 02: [aes-cbc-plain64][512][1024][1024L][2048][2048L][4096][4096L]
IV vector 03: [aes-cbc-plain64be][512][1024][1024L][2048][2048L][4096][4096L]
IV vector 04: [aes-cbc-essiv:sha256][512][1024][1024L][2048][2048L][4096][4096L]
IV vector 05: [aes-cbc-benbi][512][1024][1024L][2048][2048L][4096][4096L]
IV vector 06: [aes-cbc-eboiv][512][1024][1024L][2048][2048L][4096][4096L]
PASS: vectors-test
System PAGE_SIZE=4096
Run tests in local filesystem
# Create classic 512B drive
# (logical_block_size=512, physical_block_size=512)
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
This kernel seems to not support proper scsi_debug module, test skipped.
SKIP: blockwise-compat
HEADER CHECK
 bitlk-images/bitlk-aes-cbc-128-4k.img [OK]
 bitlk-images/bitlk-aes-cbc-128.img [OK]
 bitlk-images/bitlk-aes-cbc-256.img [OK]
 bitlk-images/bitlk-aes-cbc-elephant-128.img [OK]
 bitlk-images/bitlk-aes-cbc-elephant-256.img [OK]
 bitlk-images/bitlk-aes-xts-128-4k.img [OK]
 bitlk-images/bitlk-aes-xts-128-eow.img [OK]
 bitlk-images/bitlk-aes-xts-128-new-entry.img [OK]
 bitlk-images/bitlk-aes-xts-128-smart-card.img [OK]
 bitlk-images/bitlk-aes-xts-128-startup-key.img [OK]
 bitlk-images/bitlk-aes-xts-128.img [OK]
 bitlk-images/bitlk-aes-xts-256.img [OK]
 bitlk-images/bitlk-clearkey-aes-cbc-128.img [OK]
 bitlk-images/bitlk-togo-aes-cbc-128.img [OK]
 bitlk-images/bitlk-togo-aes-xts-128.img [OK]
ACTIVATION FS UUID CHECK
 bitlk-images/bitlk-aes-cbc-128-4k.img [OK]
 bitlk-images/bitlk-aes-cbc-128-4k.img [OK]
 bitlk-images/bitlk-aes-cbc-128-4k.img [OK]
 bitlk-images/bitlk-aes-cbc-128.img [OK]
 bitlk-images/bitlk-aes-cbc-128.img [OK]
 bitlk-images/bitlk-aes-cbc-128.img [OK]
 bitlk-images/bitlk-aes-cbc-256.img [OK]
 bitlk-images/bitlk-aes-cbc-256.img [OK]
 bitlk-images/bitlk-aes-cbc-256.img [OK]
 bitlk-images/bitlk-aes-cbc-elephant-128.img [OK]
 bitlk-images/bitlk-aes-cbc-elephant-128.img [OK]
 bitlk-images/bitlk-aes-cbc-elephant-128.img [OK]
 bitlk-images/bitlk-aes-cbc-elephant-256.img [OK]
 bitlk-images/bitlk-aes-cbc-elephant-256.img [OK]
 bitlk-images/bitlk-aes-cbc-elephant-256.img [OK]
 bitlk-images/bitlk-aes-xts-128-4k.img [OK]
 bitlk-images/bitlk-aes-xts-128-4k.img [OK]
 bitlk-images/bitlk-aes-xts-128-4k.img [OK]
 bitlk-images/bitlk-aes-xts-128-eow.img [N/A]
 bitlk-images/bitlk-aes-xts-128-eow.img [N/A]
 bitlk-images/bitlk-aes-xts-128-eow.img [N/A]
 bitlk-images/bitlk-aes-xts-128-new-entry.img [OK]
 bitlk-images/bitlk-aes-xts-128-new-entry.img [OK]
 bitlk-images/bitlk-aes-xts-128-new-entry.img [OK]
 bitlk-images/bitlk-aes-xts-128-smart-card.img [OK]
 bitlk-images/bitlk-aes-xts-128-smart-card.img [OK]
 bitlk-images/bitlk-aes-xts-128-startup-key.img [OK]
 bitlk-images/bitlk-aes-xts-128-startup-key.img [OK]
 bitlk-images/bitlk-aes-xts-128-startup-key.img [OK]
 bitlk-images/bitlk-aes-xts-128.img [OK]
 bitlk-images/bitlk-aes-xts-128.img [OK]
 bitlk-images/bitlk-aes-xts-128.img [OK]
 bitlk-images/bitlk-aes-xts-256.img [OK]
 bitlk-images/bitlk-aes-xts-256.img [OK]
 bitlk-images/bitlk-aes-xts-256.img [OK]
 bitlk-images/bitlk-clearkey-aes-cbc-128.img [N/A]
 bitlk-images/bitlk-clearkey-aes-cbc-128.img [N/A]
 bitlk-images/bitlk-clearkey-aes-cbc-128.img [N/A]
 bitlk-images/bitlk-togo-aes-cbc-128.img [OK]
 bitlk-images/bitlk-togo-aes-cbc-128.img [OK]
 bitlk-images/bitlk-togo-aes-cbc-128.img [OK]
 bitlk-images/bitlk-togo-aes-xts-128.img [OK]
 bitlk-images/bitlk-togo-aes-xts-128.img [OK]
 bitlk-images/bitlk-togo-aes-xts-128.img [OK]
PASS: bitlk-compat-test
Checking dlopen(../.libs/libcryptsetup.so)...OK
Performed 120 symbol checks in total
.PASS: run-all-symbols
Cannot find dm-verity target, test skipped.
SKIP: verity-compat-test
[1] Reencryption
[2] Reencryption with data shift
[3] Reencryption with keyfile
[4] Encryption of not yet encrypted device
[5] Reencryption using specific keyslot
[6] Reencryption using all active keyslots
[7] Reencryption of block devices with different block size
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
This kernel seems to not support proper scsi_debug module, test skipped.
SKIP: reencryption-compat-test
[1] Reencryption
[2] Reencryption with data shift
[3] Reencryption with keyfile
[4] Encryption of not yet encrypted device
[5] Reencryption using specific keyslot
[6] Reencryption using all active keyslots
[7] Reencryption of block devices with different block size
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
This kernel seems to not support proper scsi_debug module, test skipped.
SKIP: reencryption-compat-test2
modprobe: ERROR: ../libkmod/libkmod.c:586 kmod_search_moddep() could not open moddep file '/lib/modules/5.14.0-rc2+/modules.dep.bin'
modprobe: FATAL: Module scsi_debug not found in directory /lib/modules/5.14.0-rc2+
SKIP: luks2-reencryption-test
Cannot find dm-integrity target, test skipped.
SKIP: integrity-compat-test
WARNING: Variable RUN_SSH_PLUGIN_TEST must be defined, test skipped.
SKIP: ssh-plugin-test
=======================
3 of 13 tests failed
(16 tests were not run)
=======================
make[3]: *** [Makefile:799: check-TESTS] Error 1
make[3]: Leaving directory '/root/cryptsetup/tests'
make[2]: *** [Makefile:926: check-am] Error 2
make[2]: Leaving directory '/root/cryptsetup/tests'
make[1]: *** [Makefile:928: check] Error 2
make[1]: Leaving directory '/root/cryptsetup/tests'
make: *** [Makefile:2595: check-recursive] Error 1

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-27 20:38       ` Milan Broz
  2021-07-28  7:06         ` Christoph Hellwig
@ 2021-07-28 16:17         ` Mike Snitzer
  2021-07-29  7:50           ` Milan Broz
  1 sibling, 1 reply; 39+ messages in thread
From: Mike Snitzer @ 2021-07-28 16:17 UTC (permalink / raw)
  To: Milan Broz; +Cc: Jens Axboe, linux-block, dm-devel, Christoph Hellwig

On Tue, Jul 27 2021 at  4:38P -0400,
Milan Broz <gmazyland@gmail.com> wrote:

> On 27/07/2021 18:18, Mike Snitzer wrote:
> > On Tue, Jul 27 2021 at 12:02P -0400,
> > Christoph Hellwig <hch@lst.de> wrote:
> > 
> >> On Tue, Jul 27, 2021 at 11:58:46AM -0400, Mike Snitzer wrote:
> >>>> This did not make a different to my testing
> >>>> using dmsetup and the lvm2 tools.
> >>>
> >>> I'll try these changes running through the lvm2 testsuite.
> >>
> >> Btw, is ther documentation on how to run it somewhere?  I noticed
> >> tests, old-tests and unit-tests directories, but no obvious way
> >> to run them.
> > 
> > I haven't tracked how it has changed in a while, but I always run:
> > make check_local
> > 
> > (but to do that you first need to ./configure how your distro does
> > it... so that all targets are enabled, etc. Then: make).
> > 
> > Will revisit this shortly and let you know if my process needed to
> > change at all due to lvm2 changes.
> 
> BTW it would be also nice to run cryptsetup testsuite as root - we do a lot
> of DM operations there (and we depend on sysfs on some places).
> 
> You can just run configure, make and then make check.

Once I installed all deps, I got all but one passing with Christoph's changes:

Block_size: 512, Data_size: 256000B, FEC_roots: 9, Corrupted_bytes: 4 [no-superblock][one_device_test]Usage: lt-veritysetup [-?Vv] [-?|--help] [--usage] [-V|--version]
        [--cancel-deferred] [--check-at-most-once] [--data-block-size=bytes]
        [--data-blocks=blocks] [--debug] [--deferred] [--fec-device=path]
        [--fec-offset=bytes] [--fec-roots=bytes] [--format=number]
        [-h|--hash string] [--hash-block-size=bytes] [--hash-offset=bytes]
        [--ignore-corruption] [--ignore-zero-blocks] [--no-superblock]
        [--panic-on-corruption] [--restart-on-corruption]
        [--root-hash-file=STRING] [--root-hash-signature=STRING]
        [-s|--salt hex string] [--uuid=STRING] [-v|--verbose]
        [OPTION...] <action> <action-specific>
-s=e48da609055204e89ae53b655ca2216dd983cf3cb829f34f63a297d106d53e2d: unknown option
[N/A, test skipped]
FEC repair failed
FAILED backtrace:
500 ./verity-compat-test
FAIL: verity-compat-test

Seems like a test bug.

Mike

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-28 16:17         ` Mike Snitzer
@ 2021-07-29  7:50           ` Milan Broz
  0 siblings, 0 replies; 39+ messages in thread
From: Milan Broz @ 2021-07-29  7:50 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: Jens Axboe, linux-block, dm-devel, Christoph Hellwig

On 28/07/2021 18:17, Mike Snitzer wrote:
> On Tue, Jul 27 2021 at  4:38P -0400,
...
 
> Once I installed all deps, I got all but one passing with Christoph's changes:
> 
> Block_size: 512, Data_size: 256000B, FEC_roots: 9, Corrupted_bytes: 4 [no-superblock][one_device_test]Usage: lt-veritysetup [-?Vv] [-?|--help] [--usage] [-V|--version]
>         [--cancel-deferred] [--check-at-most-once] [--data-block-size=bytes]
>         [--data-blocks=blocks] [--debug] [--deferred] [--fec-device=path]
>         [--fec-offset=bytes] [--fec-roots=bytes] [--format=number]
>         [-h|--hash string] [--hash-block-size=bytes] [--hash-offset=bytes]
>         [--ignore-corruption] [--ignore-zero-blocks] [--no-superblock]
>         [--panic-on-corruption] [--restart-on-corruption]
>         [--root-hash-file=STRING] [--root-hash-signature=STRING]
>         [-s|--salt hex string] [--uuid=STRING] [-v|--verbose]
>         [OPTION...] <action> <action-specific>
> -s=e48da609055204e89ae53b655ca2216dd983cf3cb829f34f63a297d106d53e2d: unknown option
> [N/A, test skipped]
> FEC repair failed
> FAILED backtrace:
> 500 ./verity-compat-test
> FAIL: verity-compat-test
> 
> Seems like a test bug.

This is a bug in RHEL7 libpopt where -s=XXX is invalid syntax (works in recent distros), fixed in testsuite just by using the long option.
(Released RHEL7 kernel does not support verity FEC so it never hits this code without recompiling own kernel.)

Thanks for the report!

Milan

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] use regular gendisk registration in device mapper
  2021-07-28 11:24             ` Christoph Hellwig
@ 2021-07-29 15:01               ` Milan Broz
  0 siblings, 0 replies; 39+ messages in thread
From: Milan Broz @ 2021-07-29 15:01 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel, Mike Snitzer

On 28/07/2021 13:24, Christoph Hellwig wrote:
> On Wed, Jul 28, 2021 at 10:37:41AM +0200, Milan Broz wrote:
>> very specific hw attributes. So you have one emulated device compiled-in?
> 
> Yes.
> 
>> Or there is another way how to configure scsi_debug if compiled-in? (we use module parameters, I think it is
>> the same was how util-linux testsute works with scsi_debug).
> 
> Can can add hosts using the add_host sysfs file.  I thought that was the
> way to go generally, never thought of reloading the module just to
> add/delete hosts.

Heh, I just thought the opposite -that using kernel parameters is the way how to use it :-)

> 
>> (BTW could you send me output of the failed test run? I run it over Linus' tree and ti works so it is perhaps another
>> assumption that should be fixed.)
> 
> Output with everything from the README installed (a lot less failures now):

We cannot run some tests without scsi_debug as module, so I at least added detection for compiled-in scsi_debug
and some module error noise removal. (There is still a lot of operations tested without this.)

For the kernel dependencies:
For cryptsetup project and testsuite is good to have also enabled userspace crypto API interface (CONFIG_CRYPTO_USER)
and keyring (CONFIG_KEYS) but we should be able to run without it.
(The rest is specific crypto algs used in test images, but these are skipped if not available.)

Thanks,
Milan

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 1/8] block: make the block holder code optional
  2021-07-25  5:54 ` [dm-devel] [PATCH 1/8] block: make the block holder code optional Christoph Hellwig
@ 2021-07-29 16:25   ` Mike Snitzer
  0 siblings, 0 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-29 16:25 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> Move the block holder code into a separate file as it is not in any way
> related to the other block_dev.c code, and add a new selectable config
> option for it so that we don't have to build it without any remapped
> drivers selected.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/Kconfig             |   4 ++
>  block/Makefile            |   1 +
>  block/holder.c            | 139 ++++++++++++++++++++++++++++++++++++
>  drivers/md/Kconfig        |   2 +
>  drivers/md/bcache/Kconfig |   1 +
>  fs/block_dev.c            | 144 +-------------------------------------
>  include/linux/blk_types.h |   2 +-
>  include/linux/genhd.h     |   4 +-
>  8 files changed, 151 insertions(+), 146 deletions(-)
>  create mode 100644 block/holder.c
> 
> diff --git a/block/Kconfig b/block/Kconfig
> index fd732aede922..a24d7263d1fc 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -251,4 +251,8 @@ config BLK_MQ_RDMA
>  config BLK_PM
>  	def_bool BLOCK && PM
>  
> +# do not use in new code
> +config BLOCK_HOLDER_DEPRECATED
> +	bool
> +

What is it that new code that does IO remapping and device stacking
_should_ be using!?  Seems the whole "do not use" and "DEPRECATED" is
a misnomer.

But those nits aside, code looks fine mechnically:

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

>  source "block/Kconfig.iosched"
> diff --git a/block/Makefile b/block/Makefile
> index bfbe4e13ca1e..6fc6216634ed 100644
> --- a/block/Makefile
> +++ b/block/Makefile
> @@ -42,3 +42,4 @@ obj-$(CONFIG_BLK_SED_OPAL)	+= sed-opal.o
>  obj-$(CONFIG_BLK_PM)		+= blk-pm.o
>  obj-$(CONFIG_BLK_INLINE_ENCRYPTION)	+= keyslot-manager.o blk-crypto.o
>  obj-$(CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK)	+= blk-crypto-fallback.o
> +obj-$(CONFIG_BLOCK_HOLDER_DEPRECATED)	+= holder.o
> diff --git a/block/holder.c b/block/holder.c
> new file mode 100644
> index 000000000000..904a1dcd5c12
> --- /dev/null
> +++ b/block/holder.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/genhd.h>
> +
> +struct bd_holder_disk {
> +	struct list_head	list;
> +	struct gendisk		*disk;
> +	int			refcnt;
> +};
> +
> +static struct bd_holder_disk *bd_find_holder_disk(struct block_device *bdev,
> +						  struct gendisk *disk)
> +{
> +	struct bd_holder_disk *holder;
> +
> +	list_for_each_entry(holder, &bdev->bd_holder_disks, list)
> +		if (holder->disk == disk)
> +			return holder;
> +	return NULL;
> +}
> +
> +static int add_symlink(struct kobject *from, struct kobject *to)
> +{
> +	return sysfs_create_link(from, to, kobject_name(to));
> +}
> +
> +static void del_symlink(struct kobject *from, struct kobject *to)
> +{
> +	sysfs_remove_link(from, kobject_name(to));
> +}
> +
> +/**
> + * bd_link_disk_holder - create symlinks between holding disk and slave bdev
> + * @bdev: the claimed slave bdev
> + * @disk: the holding disk
> + *
> + * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
> + *
> + * This functions creates the following sysfs symlinks.
> + *
> + * - from "slaves" directory of the holder @disk to the claimed @bdev
> + * - from "holders" directory of the @bdev to the holder @disk
> + *
> + * For example, if /dev/dm-0 maps to /dev/sda and disk for dm-0 is
> + * passed to bd_link_disk_holder(), then:
> + *
> + *   /sys/block/dm-0/slaves/sda --> /sys/block/sda
> + *   /sys/block/sda/holders/dm-0 --> /sys/block/dm-0
> + *
> + * The caller must have claimed @bdev before calling this function and
> + * ensure that both @bdev and @disk are valid during the creation and
> + * lifetime of these symlinks.
> + *
> + * CONTEXT:
> + * Might sleep.
> + *
> + * RETURNS:
> + * 0 on success, -errno on failure.
> + */
> +int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
> +{
> +	struct bd_holder_disk *holder;
> +	int ret = 0;
> +
> +	mutex_lock(&bdev->bd_disk->open_mutex);
> +
> +	WARN_ON_ONCE(!bdev->bd_holder);
> +
> +	/* FIXME: remove the following once add_disk() handles errors */
> +	if (WARN_ON(!disk->slave_dir || !bdev->bd_holder_dir))
> +		goto out_unlock;
> +
> +	holder = bd_find_holder_disk(bdev, disk);
> +	if (holder) {
> +		holder->refcnt++;
> +		goto out_unlock;
> +	}
> +
> +	holder = kzalloc(sizeof(*holder), GFP_KERNEL);
> +	if (!holder) {
> +		ret = -ENOMEM;
> +		goto out_unlock;
> +	}
> +
> +	INIT_LIST_HEAD(&holder->list);
> +	holder->disk = disk;
> +	holder->refcnt = 1;
> +
> +	ret = add_symlink(disk->slave_dir, bdev_kobj(bdev));
> +	if (ret)
> +		goto out_free;
> +
> +	ret = add_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
> +	if (ret)
> +		goto out_del;
> +	/*
> +	 * bdev could be deleted beneath us which would implicitly destroy
> +	 * the holder directory.  Hold on to it.
> +	 */
> +	kobject_get(bdev->bd_holder_dir);
> +
> +	list_add(&holder->list, &bdev->bd_holder_disks);
> +	goto out_unlock;
> +
> +out_del:
> +	del_symlink(disk->slave_dir, bdev_kobj(bdev));
> +out_free:
> +	kfree(holder);
> +out_unlock:
> +	mutex_unlock(&bdev->bd_disk->open_mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(bd_link_disk_holder);
> +
> +/**
> + * bd_unlink_disk_holder - destroy symlinks created by bd_link_disk_holder()
> + * @bdev: the calimed slave bdev
> + * @disk: the holding disk
> + *
> + * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
> + *
> + * CONTEXT:
> + * Might sleep.
> + */
> +void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
> +{
> +	struct bd_holder_disk *holder;
> +
> +	mutex_lock(&bdev->bd_disk->open_mutex);
> +	holder = bd_find_holder_disk(bdev, disk);
> +	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
> +		del_symlink(disk->slave_dir, bdev_kobj(bdev));
> +		del_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
> +		kobject_put(bdev->bd_holder_dir);
> +		list_del_init(&holder->list);
> +		kfree(holder);
> +	}
> +	mutex_unlock(&bdev->bd_disk->open_mutex);
> +}
> +EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 0602e82a9516..f821dae101a9 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -15,6 +15,7 @@ if MD
>  
>  config BLK_DEV_MD
>  	tristate "RAID support"
> +	select BLOCK_HOLDER_DEPRECATED if SYSFS
>  	help
>  	  This driver lets you combine several hard disk partitions into one
>  	  logical block device. This can be used to simply append one
> @@ -201,6 +202,7 @@ config BLK_DEV_DM_BUILTIN
>  
>  config BLK_DEV_DM
>  	tristate "Device mapper support"
> +	select BLOCK_HOLDER_DEPRECATED if SYSFS
>  	select BLK_DEV_DM_BUILTIN
>  	depends on DAX || DAX=n
>  	help
> diff --git a/drivers/md/bcache/Kconfig b/drivers/md/bcache/Kconfig
> index d1ca4d059c20..cf3e8096942a 100644
> --- a/drivers/md/bcache/Kconfig
> +++ b/drivers/md/bcache/Kconfig
> @@ -2,6 +2,7 @@
>  
>  config BCACHE
>  	tristate "Block device as cache"
> +	select BLOCK_HOLDER_DEPRECATED if SYSFS
>  	select CRC64
>  	help
>  	Allows a block device to be used as cache for other devices; uses
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 0c424a0cadaa..7825d152634e 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -900,7 +900,7 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
>  	bdev->bd_disk = disk;
>  	bdev->bd_partno = partno;
>  	bdev->bd_inode = inode;
> -#ifdef CONFIG_SYSFS
> +#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
>  	INIT_LIST_HEAD(&bdev->bd_holder_disks);
>  #endif
>  	bdev->bd_stats = alloc_percpu(struct disk_stats);
> @@ -1092,148 +1092,6 @@ void bd_abort_claiming(struct block_device *bdev, void *holder)
>  }
>  EXPORT_SYMBOL(bd_abort_claiming);
>  
> -#ifdef CONFIG_SYSFS
> -struct bd_holder_disk {
> -	struct list_head	list;
> -	struct gendisk		*disk;
> -	int			refcnt;
> -};
> -
> -static struct bd_holder_disk *bd_find_holder_disk(struct block_device *bdev,
> -						  struct gendisk *disk)
> -{
> -	struct bd_holder_disk *holder;
> -
> -	list_for_each_entry(holder, &bdev->bd_holder_disks, list)
> -		if (holder->disk == disk)
> -			return holder;
> -	return NULL;
> -}
> -
> -static int add_symlink(struct kobject *from, struct kobject *to)
> -{
> -	return sysfs_create_link(from, to, kobject_name(to));
> -}
> -
> -static void del_symlink(struct kobject *from, struct kobject *to)
> -{
> -	sysfs_remove_link(from, kobject_name(to));
> -}
> -
> -/**
> - * bd_link_disk_holder - create symlinks between holding disk and slave bdev
> - * @bdev: the claimed slave bdev
> - * @disk: the holding disk
> - *
> - * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
> - *
> - * This functions creates the following sysfs symlinks.
> - *
> - * - from "slaves" directory of the holder @disk to the claimed @bdev
> - * - from "holders" directory of the @bdev to the holder @disk
> - *
> - * For example, if /dev/dm-0 maps to /dev/sda and disk for dm-0 is
> - * passed to bd_link_disk_holder(), then:
> - *
> - *   /sys/block/dm-0/slaves/sda --> /sys/block/sda
> - *   /sys/block/sda/holders/dm-0 --> /sys/block/dm-0
> - *
> - * The caller must have claimed @bdev before calling this function and
> - * ensure that both @bdev and @disk are valid during the creation and
> - * lifetime of these symlinks.
> - *
> - * CONTEXT:
> - * Might sleep.
> - *
> - * RETURNS:
> - * 0 on success, -errno on failure.
> - */
> -int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
> -{
> -	struct bd_holder_disk *holder;
> -	int ret = 0;
> -
> -	mutex_lock(&bdev->bd_disk->open_mutex);
> -
> -	WARN_ON_ONCE(!bdev->bd_holder);
> -
> -	/* FIXME: remove the following once add_disk() handles errors */
> -	if (WARN_ON(!disk->slave_dir || !bdev->bd_holder_dir))
> -		goto out_unlock;
> -
> -	holder = bd_find_holder_disk(bdev, disk);
> -	if (holder) {
> -		holder->refcnt++;
> -		goto out_unlock;
> -	}
> -
> -	holder = kzalloc(sizeof(*holder), GFP_KERNEL);
> -	if (!holder) {
> -		ret = -ENOMEM;
> -		goto out_unlock;
> -	}
> -
> -	INIT_LIST_HEAD(&holder->list);
> -	holder->disk = disk;
> -	holder->refcnt = 1;
> -
> -	ret = add_symlink(disk->slave_dir, bdev_kobj(bdev));
> -	if (ret)
> -		goto out_free;
> -
> -	ret = add_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
> -	if (ret)
> -		goto out_del;
> -	/*
> -	 * bdev could be deleted beneath us which would implicitly destroy
> -	 * the holder directory.  Hold on to it.
> -	 */
> -	kobject_get(bdev->bd_holder_dir);
> -
> -	list_add(&holder->list, &bdev->bd_holder_disks);
> -	goto out_unlock;
> -
> -out_del:
> -	del_symlink(disk->slave_dir, bdev_kobj(bdev));
> -out_free:
> -	kfree(holder);
> -out_unlock:
> -	mutex_unlock(&bdev->bd_disk->open_mutex);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(bd_link_disk_holder);
> -
> -/**
> - * bd_unlink_disk_holder - destroy symlinks created by bd_link_disk_holder()
> - * @bdev: the calimed slave bdev
> - * @disk: the holding disk
> - *
> - * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
> - *
> - * CONTEXT:
> - * Might sleep.
> - */
> -void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
> -{
> -	struct bd_holder_disk *holder;
> -
> -	mutex_lock(&bdev->bd_disk->open_mutex);
> -
> -	holder = bd_find_holder_disk(bdev, disk);
> -
> -	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
> -		del_symlink(disk->slave_dir, bdev_kobj(bdev));
> -		del_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
> -		kobject_put(bdev->bd_holder_dir);
> -		list_del_init(&holder->list);
> -		kfree(holder);
> -	}
> -
> -	mutex_unlock(&bdev->bd_disk->open_mutex);
> -}
> -EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
> -#endif
> -
>  static void blkdev_flush_mapping(struct block_device *bdev)
>  {
>  	WARN_ON_ONCE(bdev->bd_holders);
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 290f9061b29a..7a4e139d24ef 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -34,7 +34,7 @@ struct block_device {
>  	void *			bd_holder;
>  	int			bd_holders;
>  	bool			bd_write_holder;
> -#ifdef CONFIG_SYSFS
> +#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
>  	struct list_head	bd_holder_disks;
>  #endif
>  	struct kobject		*bd_holder_dir;
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 13b34177cc85..6831d74f2002 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -316,7 +316,7 @@ void set_capacity(struct gendisk *disk, sector_t size);
>  int blkdev_ioctl(struct block_device *, fmode_t, unsigned, unsigned long);
>  long compat_blkdev_ioctl(struct file *, unsigned, unsigned long);
>  
> -#ifdef CONFIG_SYSFS
> +#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
>  int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk);
>  void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk);
>  #else
> @@ -329,7 +329,7 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev,
>  					 struct gendisk *disk)
>  {
>  }
> -#endif /* CONFIG_SYSFS */
> +#endif /* CONFIG_BLOCK_HOLDER_DEPRECATED */
>  
>  dev_t part_devt(struct gendisk *disk, u8 partno);
>  dev_t blk_lookup_devt(const char *name, int partno);
> -- 
> 2.30.2
> 

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 2/8] block: remove the extra kobject reference in bd_link_disk_holder
  2021-07-25  5:54 ` [dm-devel] [PATCH 2/8] block: remove the extra kobject reference in bd_link_disk_holder Christoph Hellwig
@ 2021-07-29 16:27   ` Mike Snitzer
  0 siblings, 0 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-29 16:27 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> With the new block_device life time rules there is no way for the
> bdev to go away as long as there is a holder, so remove these.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Might be useful to reference the primary commit that introduced "the
new block_device life time rules" just so readers can inform
themselves.

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

> ---
>  block/holder.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/block/holder.c b/block/holder.c
> index 904a1dcd5c12..960654a71342 100644
> --- a/block/holder.c
> +++ b/block/holder.c
> @@ -92,11 +92,6 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
>  	ret = add_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
>  	if (ret)
>  		goto out_del;
> -	/*
> -	 * bdev could be deleted beneath us which would implicitly destroy
> -	 * the holder directory.  Hold on to it.
> -	 */
> -	kobject_get(bdev->bd_holder_dir);
>  
>  	list_add(&holder->list, &bdev->bd_holder_disks);
>  	goto out_unlock;
> @@ -130,7 +125,6 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
>  	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
>  		del_symlink(disk->slave_dir, bdev_kobj(bdev));
>  		del_symlink(bdev->bd_holder_dir, &disk_to_dev(disk)->kobj);
> -		kobject_put(bdev->bd_holder_dir);
>  		list_del_init(&holder->list);
>  		kfree(holder);
>  	}
> -- 
> 2.30.2
> 

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 4/8] block: support delayed holder registration
  2021-07-25  5:54 ` [dm-devel] [PATCH 4/8] block: support delayed holder registration Christoph Hellwig
  2021-07-27 16:06   ` Mike Snitzer
@ 2021-07-29 16:32   ` Mike Snitzer
  1 sibling, 0 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-29 16:32 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> device mapper needs to register holders before it is ready to do I/O.
> Currently it does so by registering the disk early, which has all kinds
> of bad side effects.  Support registering holders on an initialized but
> not registered disk instead by delaying the sysfs registration until the
> disk is registered.

Maybe expand "bad side effects" in header to include what you detailed here?:
https://listman.redhat.com/archives/dm-devel/2021-July/msg00130.html

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 5/8] dm: cleanup cleanup_mapped_device
  2021-07-25  5:54 ` [dm-devel] [PATCH 5/8] dm: cleanup cleanup_mapped_device Christoph Hellwig
@ 2021-07-29 16:33   ` Mike Snitzer
  0 siblings, 0 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-29 16:33 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> md->queue is not always set when md->disk is set, so simplify the
> conditionas a bit.

s/not/now/
s/conditionas/conditionals/

>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

> ---
>  drivers/md/dm.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 2c5f9e585211..7971ec8ce677 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1694,13 +1694,9 @@ static void cleanup_mapped_device(struct mapped_device *md)
>  		md->disk->private_data = NULL;
>  		spin_unlock(&_minor_lock);
>  		del_gendisk(md->disk);
> -	}
> -
> -	if (md->queue)
>  		dm_queue_destroy_keyslot_manager(md->queue);
> -
> -	if (md->disk)
>  		blk_cleanup_disk(md->disk);
> +	}
>  
>  	cleanup_srcu_struct(&md->io_barrier);
>  
> -- 
> 2.30.2
> 

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 6/8] dm: move setting md->type into dm_setup_md_queue
  2021-07-25  5:54 ` [dm-devel] [PATCH 6/8] dm: move setting md->type into dm_setup_md_queue Christoph Hellwig
@ 2021-07-29 16:34   ` Mike Snitzer
  0 siblings, 0 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-29 16:34 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> Move setting md->type from both callers into dm_setup_md_queue.
> This ensures that md->type is only set to a valid value after the queue
> has been fully setup, something we'll rely on future changes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

> ---
>  drivers/md/dm-ioctl.c | 4 ----
>  drivers/md/dm.c       | 5 +++--
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 2209cbcd84db..2575074a2204 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1436,9 +1436,6 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si
>  	}
>  
>  	if (dm_get_md_type(md) == DM_TYPE_NONE) {
> -		/* Initial table load: acquire type of table. */
> -		dm_set_md_type(md, dm_table_get_type(t));
> -
>  		/* setup md->queue to reflect md's type (may block) */
>  		r = dm_setup_md_queue(md, t);
>  		if (r) {
> @@ -2187,7 +2184,6 @@ int __init dm_early_create(struct dm_ioctl *dmi,
>  	if (r)
>  		goto err_destroy_table;
>  
> -	md->type = dm_table_get_type(t);
>  	/* setup md->queue to reflect md's type (may block) */
>  	r = dm_setup_md_queue(md, t);
>  	if (r) {
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 7971ec8ce677..f003bd5b93ce 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -2052,9 +2052,9 @@ EXPORT_SYMBOL_GPL(dm_get_queue_limits);
>   */
>  int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
>  {
> -	int r;
> +	enum dm_queue_mode type = dm_table_get_type(t);
>  	struct queue_limits limits;
> -	enum dm_queue_mode type = dm_get_md_type(md);
> +	int r;
>  
>  	switch (type) {
>  	case DM_TYPE_REQUEST_BASED:
> @@ -2081,6 +2081,7 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
>  	r = dm_table_set_restrictions(t, md->queue, &limits);
>  	if (r)
>  		return r;
> +	md->type = type;
>  
>  	blk_register_queue(md->disk);
>  
> -- 
> 2.30.2
> 

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2021-07-25  5:54 ` [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig
@ 2021-07-29 16:36   ` Mike Snitzer
  0 siblings, 0 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-29 16:36 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> device mapper is currently the only outlier that tries to call
> register_disk after add_disk, leading to fairly inconsistent state
> of these block layer data structures.  Instead change device-mapper
> to just register the gendisk later now that the holder mechanism
> can cope with that.
> 
> Note that this introduces a user visible change: the dm kobject is
> now only visible after the initial table has been loaded.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

Jens, feel free to pick this series up once you're comfortable with it.

Thanks,
Mike


> ---
>  drivers/md/dm-rq.c |  1 -
>  drivers/md/dm.c    | 23 +++++++++++------------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
> index 0dbd48cbdff9..5b95eea517d1 100644
> --- a/drivers/md/dm-rq.c
> +++ b/drivers/md/dm-rq.c
> @@ -559,7 +559,6 @@ int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t)
>  	err = blk_mq_init_allocated_queue(md->tag_set, md->queue);
>  	if (err)
>  		goto out_tag_set;
> -	elevator_init_mq(md->queue);
>  	return 0;
>  
>  out_tag_set:
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index f003bd5b93ce..7981b7287628 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1693,7 +1693,10 @@ static void cleanup_mapped_device(struct mapped_device *md)
>  		spin_lock(&_minor_lock);
>  		md->disk->private_data = NULL;
>  		spin_unlock(&_minor_lock);
> -		del_gendisk(md->disk);
> +		if (dm_get_md_type(md) != DM_TYPE_NONE) {
> +			dm_sysfs_exit(md);
> +			del_gendisk(md->disk);
> +		}
>  		dm_queue_destroy_keyslot_manager(md->queue);
>  		blk_cleanup_disk(md->disk);
>  	}
> @@ -1788,7 +1791,6 @@ static struct mapped_device *alloc_dev(int minor)
>  			goto bad;
>  	}
>  
> -	add_disk_no_queue_reg(md->disk);
>  	format_dev_t(md->name, MKDEV(_major, minor));
>  
>  	md->wq = alloc_workqueue("kdmflush", WQ_MEM_RECLAIM, 0);
> @@ -1989,19 +1991,12 @@ static struct dm_table *__unbind(struct mapped_device *md)
>   */
>  int dm_create(int minor, struct mapped_device **result)
>  {
> -	int r;
>  	struct mapped_device *md;
>  
>  	md = alloc_dev(minor);
>  	if (!md)
>  		return -ENXIO;
>  
> -	r = dm_sysfs_init(md);
> -	if (r) {
> -		free_dev(md);
> -		return r;
> -	}
> -
>  	*result = md;
>  	return 0;
>  }
> @@ -2081,10 +2076,15 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
>  	r = dm_table_set_restrictions(t, md->queue, &limits);
>  	if (r)
>  		return r;
> -	md->type = type;
>  
> -	blk_register_queue(md->disk);
> +	add_disk(md->disk);
>  
> +	r = dm_sysfs_init(md);
> +	if (r) {
> +		del_gendisk(md->disk);
> +		return r;
> +	}
> +	md->type = type;
>  	return 0;
>  }
>  
> @@ -2190,7 +2190,6 @@ static void __dm_destroy(struct mapped_device *md, bool wait)
>  		DMWARN("%s: Forcibly removing mapped_device still in use! (%d users)",
>  		       dm_device_name(md), atomic_read(&md->holders));
>  
> -	dm_sysfs_exit(md);
>  	dm_table_destroy(__unbind(md));
>  	free_dev(md);
>  }
> -- 
> 2.30.2
> 

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 8/8] block: remove support for delayed queue registrations
  2021-07-25  5:54 ` [dm-devel] [PATCH 8/8] block: remove support for delayed queue registrations Christoph Hellwig
@ 2021-07-29 16:37   ` Mike Snitzer
  0 siblings, 0 replies; 39+ messages in thread
From: Mike Snitzer @ 2021-07-29 16:37 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel

On Sun, Jul 25 2021 at  1:54P -0400,
Christoph Hellwig <hch@lst.de> wrote:

> Now that device mapper has been changed to register the disk once
> it is fully ready all this code is unused.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Mike Snitzer <snitzer@redhat.com>


> ---
>  block/elevator.c      |  1 -
>  block/genhd.c         | 29 +++++++----------------------
>  include/linux/genhd.h |  6 ------
>  3 files changed, 7 insertions(+), 29 deletions(-)
> 
> diff --git a/block/elevator.c b/block/elevator.c
> index 52ada14cfe45..706d5a64508d 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -702,7 +702,6 @@ void elevator_init_mq(struct request_queue *q)
>  		elevator_put(e);
>  	}
>  }
> -EXPORT_SYMBOL_GPL(elevator_init_mq); /* only for dm-rq */
>  
>  /*
>   * switch to new_e io scheduler. be careful not to introduce deadlocks -
> diff --git a/block/genhd.c b/block/genhd.c
> index e3d93b868ec5..3cd9f165a5a7 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -457,20 +457,20 @@ static void register_disk(struct device *parent, struct gendisk *disk,
>  }
>  
>  /**
> - * __device_add_disk - add disk information to kernel list
> + * device_add_disk - add disk information to kernel list
>   * @parent: parent device for the disk
>   * @disk: per-device partitioning information
>   * @groups: Additional per-device sysfs groups
> - * @register_queue: register the queue if set to true
>   *
>   * This function registers the partitioning information in @disk
>   * with the kernel.
>   *
>   * FIXME: error handling
>   */
> -static void __device_add_disk(struct device *parent, struct gendisk *disk,
> -			      const struct attribute_group **groups,
> -			      bool register_queue)
> +
> +void device_add_disk(struct device *parent, struct gendisk *disk,
> +		     const struct attribute_group **groups)
> +
>  {
>  	int ret;
>  
> @@ -480,8 +480,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
>  	 * elevator if one is needed, that is, for devices requesting queue
>  	 * registration.
>  	 */
> -	if (register_queue)
> -		elevator_init_mq(disk->queue);
> +	elevator_init_mq(disk->queue);
>  
>  	/*
>  	 * If the driver provides an explicit major number it also must provide
> @@ -535,8 +534,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
>  		bdev_add(disk->part0, dev->devt);
>  	}
>  	register_disk(parent, disk, groups);
> -	if (register_queue)
> -		blk_register_queue(disk);
> +	blk_register_queue(disk);
>  
>  	/*
>  	 * Take an extra ref on queue which will be put on disk_release()
> @@ -550,21 +548,8 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
>  	disk_add_events(disk);
>  	blk_integrity_add(disk);
>  }
> -
> -void device_add_disk(struct device *parent, struct gendisk *disk,
> -		     const struct attribute_group **groups)
> -
> -{
> -	__device_add_disk(parent, disk, groups, true);
> -}
>  EXPORT_SYMBOL(device_add_disk);
>  
> -void device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk)
> -{
> -	__device_add_disk(parent, disk, NULL, false);
> -}
> -EXPORT_SYMBOL(device_add_disk_no_queue_reg);
> -
>  /**
>   * del_gendisk - remove the gendisk
>   * @disk: the struct gendisk to remove
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index dd95d53c75fa..fbc4bf269f63 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -218,12 +218,6 @@ static inline void add_disk(struct gendisk *disk)
>  {
>  	device_add_disk(NULL, disk, NULL);
>  }
> -extern void device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk);
> -static inline void add_disk_no_queue_reg(struct gendisk *disk)
> -{
> -	device_add_disk_no_queue_reg(NULL, disk);
> -}
> -
>  extern void del_gendisk(struct gendisk *gp);
>  
>  void set_disk_ro(struct gendisk *disk, bool read_only);
> -- 
> 2.30.2
> 

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2022-07-15  3:24         ` Yu Kuai
@ 2022-08-01  1:04           ` Yu Kuai
  0 siblings, 0 replies; 39+ messages in thread
From: Yu Kuai @ 2022-08-01  1:04 UTC (permalink / raw)
  To: Yu Kuai, Christoph Hellwig
  Cc: Jens Axboe, Mike Snitzer, zhangyi (F), linux-block, dm-devel, yukuai (C)

Hi, Christoph!

在 2022/07/15 11:24, Yu Kuai 写道:
> Hi, Christoph!
> 
> 在 2022/07/07 15:20, Yu Kuai 写道:
>> 在 2022/07/07 13:24, Christoph Hellwig 写道:
>>> On Thu, Jul 07, 2022 at 11:29:26AM +0800, Yu Kuai wrote:
>>>> We found that this patch fix a nullptr crash in our test:
>>>
>>>> Do you think it's ok to backport this patch(and all realted patches) to
>>>> lts, or it's better to fix that bio can be submitted with queue
>>>> uninitialized from block layer?
>>>
>>> Given how long ago this was I do not remember offhand how much prep
>>> work this would require.  The patch itself is of course tiny and
>>> backportable, but someone will need to do the work and figure out how
>>> much else would have to be backported.
>>
>> Ok, I'll try to figure out that, and backport them.(At least to 5.10.y)

I posted a stable patchset on stable 5.10, can you pleas take a loock ?

dm: fix nullptr crash
https://lore.kernel.org/all/20220729062356.1663513-1-yukuai1@huaweicloud.com/

Thanks,
Kuai
> 
> While reviewing the code, I didn't found any protection that
> bd_link_disk_holder() won't concurrent with
> bd_register_pending_holders(). If they do can concurrent,
> following scenario is problematic:
> 
> t1                t2
> device_add_disk
>   disk->slave_dir = kobject_create_and_add
>                  bd_link_disk_holder
>                   __link_disk_holder
>                   list_add
>   bd_register_pending_holders
>    list_for_each_entry
>     __link_disk_holder -> -EEXIST
> 
> In this case, I think maybe ignore '-EEXIST' is fine.
> 
> I'm not familiar with dm, and I'm not sure if I missed something,
> please kindly correct me if I'm wrong.
> 
> Thanks,
> Kuai
> 
> .
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2022-07-07  7:20       ` Yu Kuai
@ 2022-07-15  3:24         ` Yu Kuai
  2022-08-01  1:04           ` Yu Kuai
  0 siblings, 1 reply; 39+ messages in thread
From: Yu Kuai @ 2022-07-15  3:24 UTC (permalink / raw)
  To: Yu Kuai, Christoph Hellwig
  Cc: Jens Axboe, linux-block, dm-devel, Mike Snitzer

Hi, Christoph!

在 2022/07/07 15:20, Yu Kuai 写道:
> 在 2022/07/07 13:24, Christoph Hellwig 写道:
>> On Thu, Jul 07, 2022 at 11:29:26AM +0800, Yu Kuai wrote:
>>> We found that this patch fix a nullptr crash in our test:
>>
>>> Do you think it's ok to backport this patch(and all realted patches) to
>>> lts, or it's better to fix that bio can be submitted with queue
>>> uninitialized from block layer?
>>
>> Given how long ago this was I do not remember offhand how much prep
>> work this would require.  The patch itself is of course tiny and
>> backportable, but someone will need to do the work and figure out how
>> much else would have to be backported.
> 
> Ok, I'll try to figure out that, and backport them.(At least to 5.10.y)

While reviewing the code, I didn't found any protection that
bd_link_disk_holder() won't concurrent with
bd_register_pending_holders(). If they do can concurrent,
following scenario is problematic:

t1				t2
device_add_disk
  disk->slave_dir = kobject_create_and_add
				bd_link_disk_holder
				 __link_disk_holder
				 list_add
  bd_register_pending_holders
   list_for_each_entry
    __link_disk_holder -> -EEXIST

In this case, I think maybe ignore '-EEXIST' is fine.

I'm not familiar with dm, and I'm not sure if I missed something,
please kindly correct me if I'm wrong.

Thanks,
Kuai

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2022-07-07  5:24     ` Christoph Hellwig
@ 2022-07-07  7:20       ` Yu Kuai
  2022-07-15  3:24         ` Yu Kuai
  0 siblings, 1 reply; 39+ messages in thread
From: Yu Kuai @ 2022-07-07  7:20 UTC (permalink / raw)
  To: Christoph Hellwig, Yu Kuai
  Cc: Jens Axboe, linux-block, dm-devel, Mike Snitzer

在 2022/07/07 13:24, Christoph Hellwig 写道:
> On Thu, Jul 07, 2022 at 11:29:26AM +0800, Yu Kuai wrote:
>> We found that this patch fix a nullptr crash in our test:
> 
>> Do you think it's ok to backport this patch(and all realted patches) to
>> lts, or it's better to fix that bio can be submitted with queue
>> uninitialized from block layer?
> 
> Given how long ago this was I do not remember offhand how much prep
> work this would require.  The patch itself is of course tiny and
> backportable, but someone will need to do the work and figure out how
> much else would have to be backported.

Ok, I'll try to figure out that, and backport them.(At least to 5.10.y)

Thanks,
Kuai
> .
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2022-07-07  3:29   ` Yu Kuai
@ 2022-07-07  5:24     ` Christoph Hellwig
  2022-07-07  7:20       ` Yu Kuai
  0 siblings, 1 reply; 39+ messages in thread
From: Christoph Hellwig @ 2022-07-07  5:24 UTC (permalink / raw)
  To: Yu Kuai
  Cc: Jens Axboe, linux-block, dm-devel, Christoph Hellwig, Mike Snitzer

On Thu, Jul 07, 2022 at 11:29:26AM +0800, Yu Kuai wrote:
> We found that this patch fix a nullptr crash in our test:

> Do you think it's ok to backport this patch(and all realted patches) to
> lts, or it's better to fix that bio can be submitted with queue
> uninitialized from block layer?

Given how long ago this was I do not remember offhand how much prep
work this would require.  The patch itself is of course tiny and
backportable, but someone will need to do the work and figure out how
much else would have to be backported.

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2021-08-04  9:41 ` [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig
  2021-08-09 23:31   ` Alasdair G Kergon
@ 2022-07-07  3:29   ` Yu Kuai
  2022-07-07  5:24     ` Christoph Hellwig
  1 sibling, 1 reply; 39+ messages in thread
From: Yu Kuai @ 2022-07-07  3:29 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

Hi, Christoph

在 2021/08/04 17:41, Christoph Hellwig 写道:
> device mapper is currently the only outlier that tries to call
> register_disk after add_disk, leading to fairly inconsistent state
> of these block layer data structures.  Instead change device-mapper
> to just register the gendisk later now that the holder mechanism
> can cope with that.
> 
> Note that this introduces a user visible change: the dm kobject is
> now only visible after the initial table has been loaded.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Mike Snitzer <snitzer@redhat.com>

We found that this patch fix a nullptr crash in our test:
[   88.727918] BUG: kernel NULL pointer dereference, address: 
00000000000001a0
[   88.730698] #PF: supervisor read access in kernel mode
[   88.731381] #PF: error_code(0x0000) - not-present page
[   88.732086] PGD 0 P4D 0
[   88.732441] Oops: 0000 [#1] PREEMPT SMP
[   88.732964] CPU: 1 PID: 1317 Comm: mount Not tainted 
5.10.0-16691-gf6076432827d-dirty #169
[   88.734055] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
BIOS ?-20190727_073836-4
[   88.735819] RIP: 0010:__blk_mq_sched_bio_merge+0x9d/0x1a0
[   88.736544] Code: 87 1e 9d 89 d0 25 00 00 00 01 0f 85 ad 00 00 00 48 
83 05 25 a1 37 0c 01 3
[   88.739040] RSP: 0018:ffffc90000473b50 EFLAGS: 00010202
[   88.739744] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 
ffffc90000473b98
[   88.740697] RDX: 0000000000001000 RSI: ffff8881080c7500 RDI: 
ffff888103a9cc18
[   88.741659] RBP: ffff88813bc80000 R08: 0000000000000001 R09: 
0000000000000000
[   88.742611] R10: ffff88810710be30 R11: 0000000000000000 R12: 
ffff888103a9cc18
[   88.743551] R13: ffff8881080c7500 R14: 0000000000000001 R15: 
0000000000000000
[   88.744501] FS:  00007f51bcdbb040(0000) GS:ffff88813bc80000(0000) 
knlGS:0000000000000000
[   88.745581] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   88.746345] CR2: 00000000000001a0 CR3: 000000010d715000 CR4: 
00000000000006e0
[   88.747298] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
0000000000000000
[   88.748253] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 
0000000000000400

[   88.749204] Call Trace:
[   88.749549]  blk_mq_submit_bio+0x115/0xd80
[   88.750124]  submit_bio_noacct+0x4ff/0x610
[   88.750692]  submit_bio+0xaa/0x1a0
[   88.751149]  submit_bh_wbc+0x1cb/0x2f0
[   88.751662]  submit_bh+0x17/0x20
[   88.752102]  ext4_read_bh+0x63/0x170
[   88.752588]  ext4_read_bh_lock+0x2c/0xd0
[   88.753125]  __ext4_sb_bread_gfp.isra.0+0xa0/0xf0
[   88.753766]  ext4_fill_super+0x21f/0x5610
[   88.754317]  ? pointer+0x31b/0x5a0
[   88.754796]  ? vsnprintf+0x131/0x7d0
[   88.755304]  mount_bdev+0x233/0x280
[   88.755791]  ? ext4_calculate_overhead+0x660/0x660
[   88.756461]  ext4_mount+0x19/0x30
[   88.756926]  legacy_get_tree+0x35/0x90
[   88.757450]  vfs_get_tree+0x29/0x100
[   88.757955]  ? capable+0x1d/0x30
[   88.758406]  path_mount+0x8a7/0x1150
[   88.758918]  do_mount+0x8d/0xc0
[   88.759360]  __se_sys_mount+0x14a/0x220
[   88.759906]  __x64_sys_mount+0x29/0x40
[   88.760431]  do_syscall_64+0x45/0x70
[   88.760931]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   88.761634] RIP: 0033:0x7f51bbe1623a
[   88.762135] Code: 48 8b 0d 51 dc 2b 00 f7 d8 64 89 01 48 83 c8 ff c3 
66 2e 0f 1f 84 00 00 8
[   88.764657] RSP: 002b:00007fff173ae898 EFLAGS: 00000246 ORIG_RAX: 
00000000000000a5
[   88.765700] RAX: ffffffffffffffda RBX: 000056169a120030 RCX: 
00007f51bbe1623a
[   88.766675] RDX: 000056169a120210 RSI: 000056169a120250 RDI: 
000056169a120230
[   88.767642] RBP: 0000000000000000 R08: 0000000000000000 R09: 
00007fff173ad798
[   88.768619] R10: 00000000c0ed0000 R11: 0000000000000246 R12: 
000056169a120230
[   88.769605] R13: 000056169a120210 R14: 0000000000000000 R15: 
00007f51bcbac184
[   88.770611] Modules linked in: dm_service_time dm_multipath
[   88.771388] CR2: 00000000000001a0
[   88.776323] ---[ end trace ac5d86e09fdc7c98 ]---
[   88.777009] RIP: 0010:__blk_mq_sched_bio_merge+0x9d/0x1a0
[   88.778038] Code: 87 1e 9d 89 d0 25 00 00 00 01 0f 85 ad 00 00 00 48 
83 05 25 a1 37 0c 01 3
[   88.780708] RSP: 0018:ffffc90000473b50 EFLAGS: 00010202
[   88.781443] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 
ffffc90000473b98
[   88.782692] RDX: 0000000000001000 RSI: ffff8881080c7500 RDI: 
ffff888103a9cc18
[   88.783839] RBP: ffff88813bc80000 R08: 0000000000000001 R09: 
0000000000000000
[   88.784942] R10: ffff88810710be30 R11: 0000000000000000 R12: 
ffff888103a9cc18
[   88.786051] R13: ffff8881080c7500 R14: 0000000000000001 R15: 
0000000000000000
[   88.787142] FS:  00007f51bcdbb040(0000) GS:ffff88813bc80000(0000) 
knlGS:0000000000000000
[   88.788399] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   88.789444] CR2: 00007f10e97a5000 CR3: 000000010d715000 CR4: 
00000000000006e0
[   88.790586] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
0000000000000000
[   88.791686] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 
0000000000000400
[   88.792773] Kernel panic - not syncing: Fatal exception
[   88.793573] Kernel Offset: disabled
[   88.794052] ---[ end Kernel panic - not syncing: Fatal exception ]---

root cause:
t1 dm-mpath       t2 mount

alloc_dev
  md->queue = blk_alloc_queue
  add_disk_no_queue_reg

dm_setup_md_queue
  case DM_TYPE_REQUEST_BASED -> multipath
   md->disk->fops = &dm_rq_blk_dops;
                         ext4_fill_super
                         ┊__ext4_sb_bread_gfp
                         ┊ ext4_read_bh
                         ┊  submit_bio -> queue is not initialized yet
                         ┊   __blk_mq_sched_bio_merge
                         ┊    ctx = blk_mq_get_ctx(q); -> ctx is NULL
   dm_mq_init_request_queue

Do you think it's ok to backport this patch(and all realted patches) to
lts, or it's better to fix that bio can be submitted with queue
uninitialized from block layer?

Thanks,
Kuai
> ---
>   drivers/md/dm-rq.c |  1 -
>   drivers/md/dm.c    | 23 +++++++++++------------
>   2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
> index 0dbd48cbdff9..5b95eea517d1 100644
> --- a/drivers/md/dm-rq.c
> +++ b/drivers/md/dm-rq.c
> @@ -559,7 +559,6 @@ int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t)
>   	err = blk_mq_init_allocated_queue(md->tag_set, md->queue);
>   	if (err)
>   		goto out_tag_set;
> -	elevator_init_mq(md->queue);
>   	return 0;
>   
>   out_tag_set:
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index f003bd5b93ce..7981b7287628 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1693,7 +1693,10 @@ static void cleanup_mapped_device(struct mapped_device *md)
>   		spin_lock(&_minor_lock);
>   		md->disk->private_data = NULL;
>   		spin_unlock(&_minor_lock);
> -		del_gendisk(md->disk);
> +		if (dm_get_md_type(md) != DM_TYPE_NONE) {
> +			dm_sysfs_exit(md);
> +			del_gendisk(md->disk);
> +		}
>   		dm_queue_destroy_keyslot_manager(md->queue);
>   		blk_cleanup_disk(md->disk);
>   	}
> @@ -1788,7 +1791,6 @@ static struct mapped_device *alloc_dev(int minor)
>   			goto bad;
>   	}
>   
> -	add_disk_no_queue_reg(md->disk);
>   	format_dev_t(md->name, MKDEV(_major, minor));
>   
>   	md->wq = alloc_workqueue("kdmflush", WQ_MEM_RECLAIM, 0);
> @@ -1989,19 +1991,12 @@ static struct dm_table *__unbind(struct mapped_device *md)
>    */
>   int dm_create(int minor, struct mapped_device **result)
>   {
> -	int r;
>   	struct mapped_device *md;
>   
>   	md = alloc_dev(minor);
>   	if (!md)
>   		return -ENXIO;
>   
> -	r = dm_sysfs_init(md);
> -	if (r) {
> -		free_dev(md);
> -		return r;
> -	}
> -
>   	*result = md;
>   	return 0;
>   }
> @@ -2081,10 +2076,15 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
>   	r = dm_table_set_restrictions(t, md->queue, &limits);
>   	if (r)
>   		return r;
> -	md->type = type;
>   
> -	blk_register_queue(md->disk);
> +	add_disk(md->disk);
>   
> +	r = dm_sysfs_init(md);
> +	if (r) {
> +		del_gendisk(md->disk);
> +		return r;
> +	}
> +	md->type = type;
>   	return 0;
>   }
>   
> @@ -2190,7 +2190,6 @@ static void __dm_destroy(struct mapped_device *md, bool wait)
>   		DMWARN("%s: Forcibly removing mapped_device still in use! (%d users)",
>   		       dm_device_name(md), atomic_read(&md->holders));
>   
> -	dm_sysfs_exit(md);
>   	dm_table_destroy(__unbind(md));
>   	free_dev(md);
>   }
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2021-08-10 13:12     ` Peter Rajnoha
@ 2021-08-10 15:05       ` Alasdair G Kergon
  0 siblings, 0 replies; 39+ messages in thread
From: Alasdair G Kergon @ 2021-08-10 15:05 UTC (permalink / raw)
  To: Peter Rajnoha
  Cc: Jens Axboe, Mike Snitzer, linux-block, dm-devel,
	Christoph Hellwig, Alasdair G Kergon

On Tue, Aug 10, 2021 at 03:12:27PM +0200, Peter Rajnoha wrote:
> (I'm not counting the very corner use case of
> 'dmsetup --addnodeoncreate --verifyudev' which now ends up with a dev node
> in /dev that logically returns -ENODEV when accessed instead of zero-sized
> device as it was before.)
 
Yes.  That facility was provided to assist people having to work with
old or incorrect code or misconfigured systems and breaking it in this
way shouldn't be a concern.  (We could possibly still patch it up to
continue to do the best thing after the patchset goes in.)

Alasdair

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2021-08-09 23:31   ` Alasdair G Kergon
  2021-08-10  0:17     ` Alasdair G Kergon
@ 2021-08-10 13:12     ` Peter Rajnoha
  2021-08-10 15:05       ` Alasdair G Kergon
  1 sibling, 1 reply; 39+ messages in thread
From: Peter Rajnoha @ 2021-08-10 13:12 UTC (permalink / raw)
  To: Alasdair G Kergon
  Cc: Jens Axboe, linux-block, dm-devel, Christoph Hellwig, Mike Snitzer

On Tue 10 Aug 2021 00:31, Alasdair G Kergon wrote:
> On Wed, Aug 04, 2021 at 11:41:46AM +0200, Christoph Hellwig wrote:
> > device mapper is currently the only outlier that tries to call
> > register_disk after add_disk, leading to fairly inconsistent state
> > of these block layer data structures.  
> 
> > Note that this introduces a user visible change: the dm kobject is
> > now only visible after the initial table has been loaded.
> 
> Indeed.  We should try to document the userspace implications of this
> change in a bit more detail.  While lvm2 and any other tools that
> followed our recommendations about how to use dm should be OK, there's
> always the chance that some other less robustly-written code will need
> to make adjustments.
> 
> Currently to make a dm device, 3 ioctls are called in sequence:
> 
> 1. DM_DEV_CREATE  - triggers 'add' uevents
> 2. DM_TABLE_LOAD
> 3. DM_SUSPEND     - triggers 'change' uevent
> 
> After this patch we have:
> 
> 1. DM_DEV_CREATE  
> 2. DM_TABLE_LOAD  - triggers 'add' uevents
> 3. DM_SUSPEND     - triggers 'change' uevent
> 
> The equivalent dmsetup commands for a simple test device are
> 0. udevadm monitor --kernel --env &   # View the uevents as they happen
> 1. dmsetup create dev1 --notable
> 2. dmsetup load --table "0 1 error" dev1
> 3. dmsetup resume dev1
> 
>   => Anyone with a udev rule that relies on 'add' needs to check if they
>      need to change their code.
> 
> The udev rules that lvm2 uses to synchronise what it is doing rely
> only on the 'change' event - which is not moving.  The 'add' event
> gets ignored.  
> 
> When loading tables, our tools also always refer to devices using
> the 'major:minor' format, which isn't affected, rather than using
> pathnames in /dev which might not exist now after this change if a table
> hasn't been loaded into a referenced device yet.  Previously this was
> permissible but we always recommended against it to avoid a pointless
> pathname lookup that's subject to races and delays.
> 
> So again, any tools that followed our recommendations ought to be
> unaffected.
> 
> Here's an example of poor code that previously worked but will fail now:
>   dmsetup create dev1 --notable
>   dmsetup create dev2 --notable
>   dmsetup ls  <-- get the minor number of dev1 (say it's 1 corresponding
> to dm-1)
>   dmsetup load dev2 --table '0 1 linear /dev/dm-1 0'
>   ...
> 
> Peter - have I missed anything?

It looks this is the only area affected, but as you say, this should be
well documented (including comments in our own udev rules) so there are
no false assumptions made by other non-lvm/non-libdm users.

(I'm not counting the very corner use case of
'dmsetup --addnodeoncreate --verifyudev' which now ends up with a dev node
in /dev that logically returns -ENODEV when accessed instead of zero-sized
device as it was before.)

-- 
Peter

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2021-08-09 23:31   ` Alasdair G Kergon
@ 2021-08-10  0:17     ` Alasdair G Kergon
  2021-08-10 13:12     ` Peter Rajnoha
  1 sibling, 0 replies; 39+ messages in thread
From: Alasdair G Kergon @ 2021-08-10  0:17 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, linux-block, dm-devel, prajnoha, Mike Snitzer

On Tue, Aug 10, 2021 at 12:31:43AM +0100, Alasdair G Kergon wrote:
> When loading tables, our tools also always refer to devices using
> the 'major:minor' format, which isn't affected, rather than using
                            ^^^^^^^^^^^^^^^^^^^^
Wrong - that is also affected.

So there is a new general constraint that a table must be loaded into a
device before another device's table can reference that device.  (The
stacked device handling in lvm2 as supported by libdevmapper should
always be doing this.)

(The original implementation had to be a bit loose to accommodate
multipath device paths that were essentially placeholders at the point
they got set up.)

Alasdair

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2021-08-04  9:41 ` [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig
@ 2021-08-09 23:31   ` Alasdair G Kergon
  2021-08-10  0:17     ` Alasdair G Kergon
  2021-08-10 13:12     ` Peter Rajnoha
  2022-07-07  3:29   ` Yu Kuai
  1 sibling, 2 replies; 39+ messages in thread
From: Alasdair G Kergon @ 2021-08-09 23:31 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, linux-block, dm-devel, prajnoha, Mike Snitzer

On Wed, Aug 04, 2021 at 11:41:46AM +0200, Christoph Hellwig wrote:
> device mapper is currently the only outlier that tries to call
> register_disk after add_disk, leading to fairly inconsistent state
> of these block layer data structures.  

> Note that this introduces a user visible change: the dm kobject is
> now only visible after the initial table has been loaded.

Indeed.  We should try to document the userspace implications of this
change in a bit more detail.  While lvm2 and any other tools that
followed our recommendations about how to use dm should be OK, there's
always the chance that some other less robustly-written code will need
to make adjustments.

Currently to make a dm device, 3 ioctls are called in sequence:

1. DM_DEV_CREATE  - triggers 'add' uevents
2. DM_TABLE_LOAD
3. DM_SUSPEND     - triggers 'change' uevent

After this patch we have:

1. DM_DEV_CREATE  
2. DM_TABLE_LOAD  - triggers 'add' uevents
3. DM_SUSPEND     - triggers 'change' uevent

The equivalent dmsetup commands for a simple test device are
0. udevadm monitor --kernel --env &   # View the uevents as they happen
1. dmsetup create dev1 --notable
2. dmsetup load --table "0 1 error" dev1
3. dmsetup resume dev1

  => Anyone with a udev rule that relies on 'add' needs to check if they
     need to change their code.

The udev rules that lvm2 uses to synchronise what it is doing rely
only on the 'change' event - which is not moving.  The 'add' event
gets ignored.  

When loading tables, our tools also always refer to devices using
the 'major:minor' format, which isn't affected, rather than using
pathnames in /dev which might not exist now after this change if a table
hasn't been loaded into a referenced device yet.  Previously this was
permissible but we always recommended against it to avoid a pointless
pathname lookup that's subject to races and delays.

So again, any tools that followed our recommendations ought to be
unaffected.

Here's an example of poor code that previously worked but will fail now:
  dmsetup create dev1 --notable
  dmsetup create dev2 --notable
  dmsetup ls  <-- get the minor number of dev1 (say it's 1 corresponding
to dm-1)
  dmsetup load dev2 --table '0 1 linear /dev/dm-1 0'
  ...

Peter - have I missed anything?

Alasdair

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [dm-devel] [PATCH 7/8] dm: delay registering the gendisk
  2021-08-04  9:41 [dm-devel] use regular gendisk registration in device mapper v2 Christoph Hellwig
@ 2021-08-04  9:41 ` Christoph Hellwig
  2021-08-09 23:31   ` Alasdair G Kergon
  2022-07-07  3:29   ` Yu Kuai
  0 siblings, 2 replies; 39+ messages in thread
From: Christoph Hellwig @ 2021-08-04  9:41 UTC (permalink / raw)
  To: Jens Axboe, Mike Snitzer; +Cc: linux-block, dm-devel

device mapper is currently the only outlier that tries to call
register_disk after add_disk, leading to fairly inconsistent state
of these block layer data structures.  Instead change device-mapper
to just register the gendisk later now that the holder mechanism
can cope with that.

Note that this introduces a user visible change: the dm kobject is
now only visible after the initial table has been loaded.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-rq.c |  1 -
 drivers/md/dm.c    | 23 +++++++++++------------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 0dbd48cbdff9..5b95eea517d1 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -559,7 +559,6 @@ int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t)
 	err = blk_mq_init_allocated_queue(md->tag_set, md->queue);
 	if (err)
 		goto out_tag_set;
-	elevator_init_mq(md->queue);
 	return 0;
 
 out_tag_set:
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index f003bd5b93ce..7981b7287628 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1693,7 +1693,10 @@ static void cleanup_mapped_device(struct mapped_device *md)
 		spin_lock(&_minor_lock);
 		md->disk->private_data = NULL;
 		spin_unlock(&_minor_lock);
-		del_gendisk(md->disk);
+		if (dm_get_md_type(md) != DM_TYPE_NONE) {
+			dm_sysfs_exit(md);
+			del_gendisk(md->disk);
+		}
 		dm_queue_destroy_keyslot_manager(md->queue);
 		blk_cleanup_disk(md->disk);
 	}
@@ -1788,7 +1791,6 @@ static struct mapped_device *alloc_dev(int minor)
 			goto bad;
 	}
 
-	add_disk_no_queue_reg(md->disk);
 	format_dev_t(md->name, MKDEV(_major, minor));
 
 	md->wq = alloc_workqueue("kdmflush", WQ_MEM_RECLAIM, 0);
@@ -1989,19 +1991,12 @@ static struct dm_table *__unbind(struct mapped_device *md)
  */
 int dm_create(int minor, struct mapped_device **result)
 {
-	int r;
 	struct mapped_device *md;
 
 	md = alloc_dev(minor);
 	if (!md)
 		return -ENXIO;
 
-	r = dm_sysfs_init(md);
-	if (r) {
-		free_dev(md);
-		return r;
-	}
-
 	*result = md;
 	return 0;
 }
@@ -2081,10 +2076,15 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
 	r = dm_table_set_restrictions(t, md->queue, &limits);
 	if (r)
 		return r;
-	md->type = type;
 
-	blk_register_queue(md->disk);
+	add_disk(md->disk);
 
+	r = dm_sysfs_init(md);
+	if (r) {
+		del_gendisk(md->disk);
+		return r;
+	}
+	md->type = type;
 	return 0;
 }
 
@@ -2190,7 +2190,6 @@ static void __dm_destroy(struct mapped_device *md, bool wait)
 		DMWARN("%s: Forcibly removing mapped_device still in use! (%d users)",
 		       dm_device_name(md), atomic_read(&md->holders));
 
-	dm_sysfs_exit(md);
 	dm_table_destroy(__unbind(md));
 	free_dev(md);
 }
-- 
2.30.2

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


^ permalink raw reply related	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2022-08-01  1:05 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-25  5:54 [dm-devel] use regular gendisk registration in device mapper Christoph Hellwig
2021-07-25  5:54 ` [dm-devel] [PATCH 1/8] block: make the block holder code optional Christoph Hellwig
2021-07-29 16:25   ` Mike Snitzer
2021-07-25  5:54 ` [dm-devel] [PATCH 2/8] block: remove the extra kobject reference in bd_link_disk_holder Christoph Hellwig
2021-07-29 16:27   ` Mike Snitzer
2021-07-25  5:54 ` [dm-devel] [PATCH 3/8] block: look up holders by bdev Christoph Hellwig
2021-07-25  5:54 ` [dm-devel] [PATCH 4/8] block: support delayed holder registration Christoph Hellwig
2021-07-27 16:06   ` Mike Snitzer
2021-07-28  6:54     ` Christoph Hellwig
2021-07-29 16:32   ` Mike Snitzer
2021-07-25  5:54 ` [dm-devel] [PATCH 5/8] dm: cleanup cleanup_mapped_device Christoph Hellwig
2021-07-29 16:33   ` Mike Snitzer
2021-07-25  5:54 ` [dm-devel] [PATCH 6/8] dm: move setting md->type into dm_setup_md_queue Christoph Hellwig
2021-07-29 16:34   ` Mike Snitzer
2021-07-25  5:54 ` [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig
2021-07-29 16:36   ` Mike Snitzer
2021-07-25  5:54 ` [dm-devel] [PATCH 8/8] block: remove support for delayed queue registrations Christoph Hellwig
2021-07-29 16:37   ` Mike Snitzer
2021-07-27 15:58 ` [dm-devel] use regular gendisk registration in device mapper Mike Snitzer
2021-07-27 16:02   ` Christoph Hellwig
2021-07-27 16:18     ` Mike Snitzer
2021-07-27 20:38       ` Milan Broz
2021-07-28  7:06         ` Christoph Hellwig
2021-07-28  8:37           ` Milan Broz
2021-07-28 11:24             ` Christoph Hellwig
2021-07-29 15:01               ` Milan Broz
2021-07-28 16:17         ` Mike Snitzer
2021-07-29  7:50           ` Milan Broz
2021-07-27 22:52       ` Mike Snitzer
2021-08-04  9:41 [dm-devel] use regular gendisk registration in device mapper v2 Christoph Hellwig
2021-08-04  9:41 ` [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig
2021-08-09 23:31   ` Alasdair G Kergon
2021-08-10  0:17     ` Alasdair G Kergon
2021-08-10 13:12     ` Peter Rajnoha
2021-08-10 15:05       ` Alasdair G Kergon
2022-07-07  3:29   ` Yu Kuai
2022-07-07  5:24     ` Christoph Hellwig
2022-07-07  7:20       ` Yu Kuai
2022-07-15  3:24         ` Yu Kuai
2022-08-01  1:04           ` Yu Kuai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).