[Devel,rh7] md: add support for dm-crypted ploops

Submitted by Maxim Patlasov on Oct. 19, 2016, 3:41 a.m.

Details

Message ID 147684848569.4395.12716513100340885701.stgit@maxim-thinkpad
State New
Series "md: add support for dm-crypted ploops"
Headers show

Commit Message

Maxim Patlasov Oct. 19, 2016, 3:41 a.m.
Previos patch adding support for dm-crypt ploops naively suggested
that every time we build dm-crypt device, crypt_ctr() is called, and
vice versa - every time we dismantle it, crypt_dtr() is called. But
in practice, crypt_ctr/dtr is called more than once because md->map
is RCU-protected. For example, during resize, new dm target is
constructed and registetred in md->map, then the former md->map is
released.

Only dm-crypt knows how to find underlaying ploop device. That's why
the patch implements ploop_modify methods of dm-crypt target. And
only general dm code (dm.c, dm-table.c, dm-ioctl.c and friends) knows
which instance of dm-target is actual and which is obsoleted. That's
why the patch orchestrates calling this new method from general code,
close to __bind/__unbind managing md->map pointer.

https://jira.sw.ru/browse/PSBM-53386

Signed-off-by: Maxim Patlasov <mpatlasov@virtuozzo.com>
---
 drivers/md/dm-crypt.c         |   25 ++++++++++++++++++++-----
 drivers/md/dm-ioctl.c         |    3 +++
 drivers/md/dm-table.c         |   15 +++++++++++++++
 drivers/md/dm.c               |    4 +++-
 drivers/md/dm.h               |    1 +
 include/linux/device-mapper.h |    9 +++++++++
 6 files changed, 51 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index bcdd794..41019b8 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1647,10 +1647,8 @@  static void crypt_dtr(struct dm_target *ti)
 	if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
 		cc->iv_gen_ops->dtr(cc);
 
-	if (cc->dev) {
-		ploop_set_dm_crypt_bdev(cc->dev->bdev, NULL);
+	if (cc->dev)
 		dm_put_device(ti, cc->dev);
-	}
 
 	kzfree(cc->cipher);
 	kzfree(cc->cipher_string);
@@ -1919,8 +1917,6 @@  static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto bad;
 	}
 
-	ploop_set_dm_crypt_bdev(cc->dev->bdev, dm_md_get_bdev(dm_table_get_md(ti->table)));
-
 	if (sscanf(argv[4], "%llu%c", &tmpll, &dummy) != 1) {
 		ti->error = "Invalid device sector";
 		goto bad;
@@ -2173,6 +2169,24 @@  static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
 	limits->max_segment_size = PAGE_SIZE;
 }
 
+static void crypt_ploop_modify(struct dm_target *ti, int action)
+{
+	struct crypt_config *cc = ti->private;
+
+	if (cc && cc->dev)
+		switch (action) {
+		case DM_PLOOP_ATTACH:
+			ploop_set_dm_crypt_bdev(cc->dev->bdev,
+				dm_md_get_bdev(dm_table_get_md(ti->table)));
+			break;
+		case DM_PLOOP_DETACH:
+			ploop_set_dm_crypt_bdev(cc->dev->bdev, NULL);
+			break;
+		default:
+			BUG();
+		}
+}
+
 static struct target_type crypt_target = {
 	.name   = "crypt",
 	.version = {1, 14, 1},
@@ -2188,6 +2202,7 @@  static struct target_type crypt_target = {
 	.merge  = crypt_merge,
 	.iterate_devices = crypt_iterate_devices,
 	.io_hints = crypt_io_hints,
+	.ploop_modify = crypt_ploop_modify,
 };
 
 static int __init dm_crypt_init(void)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 0fe4233..fd4b2cc 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1029,6 +1029,9 @@  static int do_resume(struct dm_ioctl *param)
 			return PTR_ERR(old_map);
 		}
 
+		dm_table_ploop_modify(old_map, DM_PLOOP_DETACH);
+		dm_table_ploop_modify(new_map, DM_PLOOP_ATTACH);
+
 		if (dm_table_get_mode(new_map) & FMODE_WRITE)
 			set_disk_ro(dm_disk(md), 0);
 		else
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 16ba55a..e910fbe 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1705,3 +1705,18 @@  void dm_table_run_md_queue_async(struct dm_table *t)
 }
 EXPORT_SYMBOL(dm_table_run_md_queue_async);
 
+void dm_table_ploop_modify(struct dm_table *t, int action)
+{
+	unsigned int i;
+
+	if (!t)
+		return;
+
+	/* attach or detach the targets */
+	for (i = 0; i < t->num_targets; i++) {
+		struct dm_target *tgt = t->targets + i;
+
+		if (tgt->type->ploop_modify)
+			tgt->type->ploop_modify(tgt, action);
+	}
+}
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index a7993cf..210221e 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -3088,7 +3088,9 @@  static void __dm_destroy(struct mapped_device *md, bool wait)
 		       dm_device_name(md), atomic_read(&md->holders));
 
 	dm_sysfs_exit(md);
-	dm_table_destroy(__unbind(md));
+	map = __unbind(md);
+	dm_table_ploop_modify(map, DM_PLOOP_DETACH);
+	dm_table_destroy(map);
 	free_dev(md);
 }
 
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 815afa5..7330c89 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -76,6 +76,7 @@  bool dm_table_request_based(struct dm_table *t);
 bool dm_table_mq_request_based(struct dm_table *t);
 void dm_table_free_md_mempools(struct dm_table *t);
 struct dm_md_mempools *dm_table_get_md_mempools(struct dm_table *t);
+void dm_table_ploop_modify(struct dm_table *t, int action);
 
 int dm_queue_merge_is_compulsory(struct request_queue *q);
 
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 0e7360c..bea9446 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -112,6 +112,14 @@  typedef int (*dm_iterate_devices_fn) (struct dm_target *ti,
 typedef void (*dm_io_hints_fn) (struct dm_target *ti,
 				struct queue_limits *limits);
 
+typedef void (*dm_ploop_modify_fn) (struct dm_target *ti, int action);
+
+/* "action" arg of dm_ploop_modify_fn */
+enum {
+	DM_PLOOP_ATTACH,
+	DM_PLOOP_DETACH,
+};
+
 /*
  * Returns:
  *    0: The target can handle the next I/O immediately.
@@ -164,6 +172,7 @@  struct target_type {
 	dm_busy_fn busy;
 	dm_iterate_devices_fn iterate_devices;
 	dm_io_hints_fn io_hints;
+	dm_ploop_modify_fn ploop_modify;
 
 	/* For internal device-mapper use. */
 	struct list_head list;

Comments

Andrey Ryabinin Oct. 24, 2016, 12:18 p.m.
On 10/19/2016 06:41 AM, Maxim Patlasov wrote:
> Previos patch adding support for dm-crypt ploops naively suggested
> that every time we build dm-crypt device, crypt_ctr() is called, and
> vice versa - every time we dismantle it, crypt_dtr() is called. But
> in practice, crypt_ctr/dtr is called more than once because md->map
> is RCU-protected. For example, during resize, new dm target is
> constructed and registetred in md->map, then the former md->map is
> released.
> 
> Only dm-crypt knows how to find underlaying ploop device. That's why
> the patch implements ploop_modify methods of dm-crypt target. And
> only general dm code (dm.c, dm-table.c, dm-ioctl.c and friends) knows
> which instance of dm-target is actual and which is obsoleted. That's
> why the patch orchestrates calling this new method from general code,
> close to __bind/__unbind managing md->map pointer.
> 
> https://jira.sw.ru/browse/PSBM-53386
> 
> Signed-off-by: Maxim Patlasov <mpatlasov@virtuozzo.com>

Reviewed-by: Andrey Ryabinin <aryabinin@virtuozzo.com>