[dm-devel] [RFC PATCHv2 0/2] block-layer interposer

[dm-devel] [RFC PATCHv2 0/2] block-layer interposer

[Hannes Reinecke]

Hi all,

here's a combined version of the earlier patchset from Sergei, fixing
some issues I've found during testing and including (some) feedback
from Mike. So with this patchset I could do a

echo "0 33554432 linear /dev/sda 0" | dmsetup create sda-cloned

on a system with root-fs on /dev/sda2, and things would continue to
run just like normal.

There are some things which might need to be improved:
- I've tried to remove the stub bio clone for the dm interposer as
suggested by Mike, but that resulted in an endless recursion in
submit_bio_noacct(). That needs more debugging,  but I'm also not
sure if we _can_ do it; the end_io callback might refer to
bi_disk->driver_private, and then will get confused as we've
reassigned the 'bi_disk' setting. Not sure if there are fixed rules
for it, so for now I'll probably leave it.
- The interposer is 'magically' hooked to the first block device
in the device-mapper table. What I really want is to have an explicit
listing of the interposer in the output of things like 'dmsetup table'
like

0 33554432 interposer /dev/dm-0
0 33554432 linear /dev/sda 0

and the first entry having the device number of /dev/sda.
Plan was to have that automatically registered once you set up
the original table, but then I'm not sure if that's the right
way to go. Mike?
- The removal of blkdev_get() as an exported API made things
awkward, as we now have to do a blkdev_get_by_dev(); if not we
end up with an uninitalized blkcg and a resulting crash.
We might be able to fix this up by moving bdget() into dm_blk_open(),
and have md->bdev uninitalized otherwise. But again I'm not
sure if that's the right way to go.

Anyway, comments and reviews are welcome.

Sergei Shtepa (2):
  blk_interposer - Block Layer Interposer
  dm_interposer - blk_interposer for device-mapper

 block/blk-core.c          |  34 +++++++++++
 block/genhd.c             |  55 ++++++++++++++++++
 drivers/md/dm-table.c     |  59 +++++++++++++++++++
 drivers/md/dm.c           | 140 ++++++++++++++++++++++++++++++++++++++++++----
 drivers/md/dm.h           |   4 +-
 include/linux/blk_types.h |   6 +-
 include/linux/genhd.h     |  19 +++++++
 7 files changed, 303 insertions(+), 14 deletions(-)

-- 
2.16.4

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

[dm-devel] [PATCH 1/2] blk_interposer - Block Layer Interposer

[Hannes Reinecke]

From: Sergei Shtepa <sergei.shtepa@veeam.com>

The block layer interposer allows to intercept bio requests.
This allows you to connect device mapper and other kernel
modules to the block device stack on the fly.

Signed-off-by: Sergei Shtepa <sergei.shtepa@veeam.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-core.c          | 34 +++++++++++++++++++++++++++++
 block/genhd.c             | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/blk_types.h |  6 ++++--
 include/linux/genhd.h     | 19 ++++++++++++++++
 4 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 2db8bda43b6e..130f0124d939 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1030,6 +1030,37 @@ static blk_qc_t __submit_bio_noacct_mq(struct bio *bio)
 	return ret;
 }
 
+static blk_qc_t __submit_bio_interposed(struct bio *bio)
+{
+	struct bio_list bio_list[2] = { };
+	blk_qc_t ret = BLK_QC_T_NONE;
+
+	current->bio_list = bio_list;
+	if (likely(bio_queue_enter(bio) == 0)) {
+		struct gendisk *disk = bio->bi_disk;
+
+		bio_set_flag(bio, BIO_INTERPOSED);
+		if (likely(blk_has_interposer(disk))) {
+			struct blk_interposer *ip = disk->interposer;
+			ret = ip->ip_submit_bio(ip, bio);
+
+			if (ret == BLK_QC_T_NONE)
+				blk_queue_exit(disk->queue);
+		} else {
+			/* interposer was removed */
+			bio_list_add(&current->bio_list[0], bio);
+			blk_queue_exit(bio->bi_disk->queue);
+		}
+	}
+	current->bio_list = NULL;
+
+	/* Resubmit remaining bios */
+	while ((bio = bio_list_pop(&bio_list[0])))
+		ret = submit_bio_noacct(bio);
+
+	return ret;
+}
+
 /**
  * submit_bio_noacct - re-submit a bio to the block device layer for I/O
  * @bio:  The bio describing the location in memory and on the device.
@@ -1055,6 +1086,9 @@ blk_qc_t submit_bio_noacct(struct bio *bio)
 		return BLK_QC_T_NONE;
 	}
 
+	if (blk_has_interposer(bio->bi_disk) &&
+	    !bio_flagged(bio, BIO_INTERPOSED))
+		return __submit_bio_interposed(bio);
 	if (!bio->bi_disk->fops->submit_bio)
 		return __submit_bio_noacct_mq(bio);
 	return __submit_bio_noacct(bio);
diff --git a/block/genhd.c b/block/genhd.c
index 9387f050c248..17b43e2ed06a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -2364,3 +2364,58 @@ static void disk_release_events(struct gendisk *disk)
 	WARN_ON_ONCE(disk->ev && disk->ev->block != 1);
 	kfree(disk->ev);
 }
+
+/**
+ * blk_interposer_attach - Attach interposer to disk
+ * @disk: target disk
+ * @interposer: block device interposer
+ *
+ * Returns:
+ *     -EINVAL if @interposer is NULL.
+ *     -ENODEV if interposer is not initialized,
+ *     -EBUSY if the block device already has interposer.
+ */
+int blk_interposer_attach(struct gendisk *disk,
+			  struct blk_interposer *interposer)
+{
+	int ret = 0;
+
+	if (!interposer)
+		return -EINVAL;
+
+	blk_mq_freeze_queue(disk->queue);
+	blk_mq_quiesce_queue(disk->queue);
+	if (blk_has_interposer(disk))
+		ret = -EBUSY;
+	else
+		disk->interposer = interposer;
+	blk_mq_unquiesce_queue(disk->queue);
+	blk_mq_unfreeze_queue(disk->queue);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(blk_interposer_attach);
+
+/**
+ * blk_interposer_detach - Detach interposer from disk
+ * @disk: target disk
+ *
+ * Returns the attached interposer or NULL if none was attached.
+ */
+struct blk_interposer *blk_interposer_detach(struct gendisk *disk)
+{
+	struct blk_interposer *interposer;
+
+	if (WARN_ON(!disk))
+		return NULL;
+
+	blk_mq_freeze_queue(disk->queue);
+	blk_mq_quiesce_queue(disk->queue);
+	interposer = disk->interposer;
+	disk->interposer = NULL;
+	blk_mq_unquiesce_queue(disk->queue);
+	blk_mq_unfreeze_queue(disk->queue);
+
+	return interposer;
+}
+EXPORT_SYMBOL_GPL(blk_interposer_detach);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index d9b69bbde5cc..996b803e5aa1 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -207,7 +207,7 @@ struct bio {
 						 * top bits REQ_OP. Use
 						 * accessors.
 						 */
-	unsigned short		bi_flags;	/* status, etc and bvec pool number */
+	unsigned int		bi_flags;	/* status, etc and bvec pool number */
 	unsigned short		bi_ioprio;
 	unsigned short		bi_write_hint;
 	blk_status_t		bi_status;
@@ -284,6 +284,8 @@ enum {
 				 * of this bio. */
 	BIO_CGROUP_ACCT,	/* has been accounted to a cgroup */
 	BIO_TRACKED,		/* set if bio goes through the rq_qos path */
+	BIO_INTERPOSED,		/* bio has been interposed and can be moved to
+				 * a different disk */
 	BIO_FLAG_LAST
 };
 
@@ -302,7 +304,7 @@ enum {
  * freed.
  */
 #define BVEC_POOL_BITS		(3)
-#define BVEC_POOL_OFFSET	(16 - BVEC_POOL_BITS)
+#define BVEC_POOL_OFFSET	(32 - BVEC_POOL_BITS)
 #define BVEC_POOL_IDX(bio)	((bio)->bi_flags >> BVEC_POOL_OFFSET)
 #if (1<< BVEC_POOL_BITS) < (BVEC_POOL_NR+1)
 # error "BVEC_POOL_BITS is too small"
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 03da3f603d30..f58ab391deda 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -164,6 +164,15 @@ struct blk_integrity {
 	unsigned char				tag_size;
 };
 
+struct blk_interposer;
+typedef blk_qc_t (*ip_submit_bio_t) (struct blk_interposer *ip, struct bio *bio);
+
+struct blk_interposer {
+	struct gendisk *ip_disk;
+	ip_submit_bio_t ip_submit_bio;
+	void *ip_private;
+};
+
 struct gendisk {
 	/* major, first_minor and minors are input parameters only,
 	 * don't use directly.  Use disk_devt() and disk_max_parts().
@@ -188,6 +197,7 @@ struct gendisk {
 
 	const struct block_device_operations *fops;
 	struct request_queue *queue;
+	struct blk_interposer *interposer;
 	void *private_data;
 
 	int flags;
@@ -409,4 +419,13 @@ static inline dev_t blk_lookup_devt(const char *name, int partno)
 }
 #endif /* CONFIG_BLOCK */
 
+/*
+ * Block device interposing
+ */
+#define blk_has_interposer(d) ((d)->interposer != NULL)
+#define blk_interposer_active(ip) ((ip)->ip_disk != NULL)
+
+int blk_interposer_attach(struct gendisk *, struct blk_interposer *);
+struct blk_interposer * blk_interposer_detach(struct gendisk *);
+
 #endif /* _LINUX_GENHD_H */
-- 
2.16.4

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

[dm-devel] [PATCH 2/2] dm_interposer - blk_interposer for device-mapper

[Hannes Reinecke]

From: Sergei Shtepa <sergei.shtepa@veeam.com>

Implement a block interposer for device-mapper to
attach to an existing block layer stack.

Signed-off-by: Sergei Shtepa <sergei.shtepa@veeam.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/md/dm-table.c |  59 +++++++++++++++++++++
 drivers/md/dm.c       | 140 ++++++++++++++++++++++++++++++++++++++++++++++----
 drivers/md/dm.h       |   4 +-
 3 files changed, 191 insertions(+), 12 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index ce543b761be7..56923d0795b2 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1809,6 +1809,65 @@ static bool dm_table_requires_stable_pages(struct dm_table *t)
 	return false;
 }
 
+static char *_dm_interposer_claim_ptr = "device-mapper interposer";
+
+static int device_activate_interposer(struct dm_target *ti,
+				      struct dm_dev *dev, sector_t start,
+				      sector_t len, void *data)
+{
+	struct blk_interposer *blk_ip = dev->bdev->bd_disk->interposer;
+	struct mapped_device *md = data;
+
+	if (!blk_ip)
+		return false;
+	if (md) {
+		struct block_device *bdev;
+
+		bdev = blkdev_get_by_dev(md->bdev->bd_dev,
+					     dev->mode | FMODE_EXCL,
+					     _dm_interposer_claim_ptr);
+		if (!bdev)
+			return false;
+		blk_ip->ip_private = md;
+	} else if (blk_ip->ip_private) {
+		md = blk_ip->ip_private;
+		blkdev_put(md->bdev, dev->mode | FMODE_EXCL);
+		blk_ip->ip_private = NULL;
+	}
+	return true;
+}
+
+bool dm_table_activate_interposer(struct dm_table *t, struct mapped_device *md)
+{
+	struct dm_target *ti;
+
+	if (t->num_targets) {
+		ti = t->targets;
+
+		if (!ti->type->iterate_devices ||
+		    !ti->type->iterate_devices(ti,
+				device_activate_interposer, md))
+			return false;
+		DMINFO("%s: activated interposer", dm_device_name(md));
+	}
+	return true;
+}
+
+bool dm_table_deactivate_interposer(struct dm_table *t)
+{
+	struct dm_target *ti;
+
+	if (t->num_targets) {
+		ti = t->targets;
+
+		if (!ti->type->iterate_devices ||
+		    !ti->type->iterate_devices(ti,
+				device_activate_interposer, NULL))
+			return false;
+	}
+	return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index c18fc2548518..ec6df4ad4c85 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -48,6 +48,7 @@ static DEFINE_IDR(_minor_idr);
 static DEFINE_SPINLOCK(_minor_lock);
 
 static void do_deferred_remove(struct work_struct *w);
+static blk_qc_t dm_submit_bio_interposed(struct blk_interposer *ip, struct bio *bio);
 
 static DECLARE_WORK(deferred_remove_work, do_deferred_remove);
 
@@ -730,6 +731,38 @@ static void dm_put_live_table_fast(struct mapped_device *md) __releases(RCU)
 	rcu_read_unlock();
 }
 
+static inline int dm_install_interposer(struct gendisk *disk,
+					struct mapped_device *md)
+{
+	struct blk_interposer *blk_ip;
+	int ret;
+
+	blk_ip = kzalloc(sizeof(struct blk_interposer), GFP_KERNEL);
+	if (!blk_ip)
+		return -ENOMEM;
+
+	blk_ip->ip_submit_bio = dm_submit_bio_interposed;
+	blk_ip->ip_disk = disk;
+	blk_ip->ip_private = md;
+
+	ret = blk_interposer_attach(disk, blk_ip);
+	if (ret) {
+		kfree(blk_ip);
+		return ret;
+	}
+
+	return 0;
+}
+
+static inline void dm_uninstall_interposer(struct gendisk *disk)
+{
+	struct blk_interposer *blk_ip;
+
+	blk_ip = blk_interposer_detach(disk);
+	if (blk_ip)
+		kfree(blk_ip);
+}
+
 static char *_dm_claim_ptr = "I belong to device-mapper";
 
 /*
@@ -739,19 +772,38 @@ static int open_table_device(struct table_device *td, dev_t dev,
 			     struct mapped_device *md)
 {
 	struct block_device *bdev;
-
-	int r;
+	fmode_t fmode = td->dm_dev.mode | FMODE_EXCL;
+	int ret;
 
 	BUG_ON(td->dm_dev.bdev);
 
-	bdev = blkdev_get_by_dev(dev, td->dm_dev.mode | FMODE_EXCL, _dm_claim_ptr);
-	if (IS_ERR(bdev))
-		return PTR_ERR(bdev);
+	bdev = blkdev_get_by_dev(dev, fmode, _dm_claim_ptr);
+	if (IS_ERR(bdev)) {
+		ret = PTR_ERR(bdev);
+		if (ret != -EBUSY)
+			return ret;
 
-	r = bd_link_disk_holder(bdev, dm_disk(md));
-	if (r) {
-		blkdev_put(bdev, td->dm_dev.mode | FMODE_EXCL);
-		return r;
+		/*
+		 * If device cannot be opened in exclusive mode,
+		 * then try to use blk_interpose.
+		 */
+		fmode = td->dm_dev.mode;
+		bdev = blkdev_get_by_dev(dev, fmode, NULL);
+		if (IS_ERR(bdev))
+			return PTR_ERR(bdev);
+
+		ret = dm_install_interposer(bdev->bd_disk, md);
+		if (ret) {
+			blkdev_put(bdev, fmode);
+			return ret;
+		}
+	}
+
+	ret = bd_link_disk_holder(bdev, dm_disk(md));
+	if (ret) {
+		dm_uninstall_interposer(bdev->bd_disk);
+		blkdev_put(bdev, fmode);
+		return ret;
 	}
 
 	td->dm_dev.bdev = bdev;
@@ -764,11 +816,18 @@ static int open_table_device(struct table_device *td, dev_t dev,
  */
 static void close_table_device(struct table_device *td, struct mapped_device *md)
 {
+	fmode_t fmode = td->dm_dev.mode | FMODE_EXCL;
+
 	if (!td->dm_dev.bdev)
 		return;
 
 	bd_unlink_disk_holder(td->dm_dev.bdev, dm_disk(md));
-	blkdev_put(td->dm_dev.bdev, td->dm_dev.mode | FMODE_EXCL);
+	if (blk_has_interposer(td->dm_dev.bdev->bd_disk)) {
+		dm_uninstall_interposer(td->dm_dev.bdev->bd_disk);
+		fmode = td->dm_dev.mode;
+	}
+	blkdev_put(td->dm_dev.bdev, fmode);
+
 	put_dax(td->dm_dev.dax_dev);
 	td->dm_dev.bdev = NULL;
 	td->dm_dev.dax_dev = NULL;
@@ -1666,6 +1725,62 @@ static blk_qc_t dm_submit_bio(struct bio *bio)
 	return ret;
 }
 
+static void dm_interposed_endio(struct bio *clone)
+{
+	struct bio *bio = clone->bi_private;
+
+	bio->bi_status = clone->bi_status;
+	bio_endio(bio);
+	bio_put(bio);
+
+	bio_put(clone);
+}
+
+static blk_qc_t dm_submit_bio_interposed(struct blk_interposer *interposer,
+					 struct bio *bio)
+{
+	struct mapped_device *md = interposer->ip_private;
+	blk_qc_t ret = BLK_QC_T_NONE;
+	int srcu_idx;
+	struct dm_table *map;
+	struct bio *clone;
+
+	if (unlikely(!md))
+		return submit_bio_noacct(bio);
+
+	map = dm_get_live_table(md, &srcu_idx);
+	if (unlikely(!map)) {
+		DMERR_LIMIT("%s: mapping table unavailable, erroring io",
+			    dm_device_name(md));
+		goto out;
+	}
+
+	clone = bio_clone_fast(bio, GFP_NOIO, NULL);
+	if (unlikely(!clone)) {
+		DMERR_LIMIT("%s: failed to clone bio",
+			    dm_device_name(md));
+		goto out;
+	}
+
+	bio_get(bio);
+	bio_set_flag(clone, BIO_INTERPOSED);
+	clone->bi_private = bio;
+	clone->bi_disk = dm_disk(md);
+	clone->bi_end_io = dm_interposed_endio;
+
+	trace_block_bio_remap(clone->bi_disk->queue, bio, bio_dev(bio),
+			      bio->bi_iter.bi_sector);
+
+	ret = submit_bio_noacct(clone);
+	dm_put_live_table(md, srcu_idx);
+	return  ret;
+
+out:
+	bio_io_error(bio);
+	dm_put_live_table(md, srcu_idx);
+	return ret;
+}
+
 /*-----------------------------------------------------------------
  * An IDR is used to keep track of allocated minor numbers.
  *---------------------------------------------------------------*/
@@ -2005,8 +2120,11 @@ static struct dm_table *__bind(struct mapped_device *md, struct dm_table *t,
 	md->immutable_target_type = dm_table_get_immutable_target_type(t);
 
 	dm_table_set_restrictions(t, q, limits);
-	if (old_map)
+	if (old_map) {
+		dm_table_deactivate_interposer(old_map);
 		dm_sync_table(md);
+	}
+	dm_table_activate_interposer(t, md);
 
 out:
 	return old_map;
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index fffe1e289c53..51b98e10e9bb 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -75,7 +75,9 @@ bool dm_table_supports_dax(struct dm_table *t, iterate_devices_callout_fn fn,
 			   int *blocksize);
 int device_supports_dax(struct dm_target *ti, struct dm_dev *dev,
 			   sector_t start, sector_t len, void *data);
-
+bool dm_table_activate_interposer(struct dm_table *t,
+				  struct mapped_device *md);
+bool dm_table_deactivate_interposer(struct dm_table *t);
 void dm_lock_md_type(struct mapped_device *md);
 void dm_unlock_md_type(struct mapped_device *md);
 void dm_set_md_type(struct mapped_device *md, enum dm_queue_mode type);
-- 
2.16.4

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