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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ 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; 30+ 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] 30+ messages in thread

* [dm-devel] [PATCH 3/8] block: look up holders by bdev
  2021-08-04  9:41 [dm-devel] use regular gendisk registration in device mapper v2 Christoph Hellwig
@ 2021-08-04  9:41 ` Christoph Hellwig
  0 siblings, 0 replies; 30+ messages in thread
From: Christoph Hellwig @ 2021-08-04  9:41 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             |  4 +++-
 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(+), 17 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index a4817e42f3a3..cd4eab744667 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1289,7 +1289,9 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
 	disk_to_dev(disk)->type = &disk_type;
 	device_initialize(disk_to_dev(disk));
 	inc_diskseq(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 ae9651cad923..cc801767a377 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -902,9 +902,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 e21a91c16a79..0721807d76ee 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] 30+ messages in thread

end of thread, other threads:[~2021-08-04  9:44 UTC | newest]

Thread overview: 30+ 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 3/8] block: look up holders by bdev Christoph Hellwig

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).