dm: prevent table type changes after initial table load

PATCH New Mike Snitzer 1 participant 2 messages Jun 29, 2010 Device mapper

Share & tools

Mike Snitzer

author

2010-06-29 17:40 UTC #0 lore raw

PATCH v5 1/2 dm: prevent table type changes after initial table load

Before table_load() stages an inactive table check if its type conflicts with a previously established device type (md->type).

Allowed md->type transitions:
DM_TYPE_NONE => DM_TYPE_BIO_BASED
DM_TYPE_NONE => DM_TYPE_REQUEST_BASED

Once a table load occurs the DM device's type is completely immutable. This prevents a table reload from switching the inactive table directly to a conflicting type (even if the table is explicitly cleared before load).

Introduce 'type_lock' in mapped_device structure and use it to protect md->type access. Use of mutex prepares for follow-on DM device queue initialization changes that will allocate memory while md->type_lock is held.

Signed-off-by Mike Snitzer <[email protected]>

Acked-by Kiyoshi Ueda <[email protected]>

---
v5: revised patch header, no code changes

 drivers/md/dm-ioctl.c |   15 +++++++++++++++
 drivers/md/dm.c       |   37 ++++++++++++++++++++++++++++++-------
 drivers/md/dm.h       |    5 +++++
 3 files changed, 50 insertions(+), 7 deletions(-)

drivers/md/dm-ioctl.c +15 -0

Index: linux-2.6/drivers/md/dm-ioctl.c
===================================================================
@@ -1176,6 +1176,21 @@ static int table_load(struct dm_ioctl *p
 		goto out;
 	}
 
+	/* Protect md->type against concurrent table loads. */
+	dm_lock_md_type(md);
+	if (dm_get_md_type(md) == DM_TYPE_NONE) {
+		/* initial table load, set md's type based on table's type */
+		dm_set_md_type(md, dm_table_get_type(t));
+	} else if (dm_get_md_type(md) != dm_table_get_type(t)) {
+		DMWARN("can't change device type after initial table load.");
+		dm_table_destroy(t);
+		dm_unlock_md_type(md);
+		r = -EINVAL;
+		goto out;
+	}
+	dm_unlock_md_type(md);
+
+	/* stage inactive table */
 	down_write(&_hash_lock);
 	hc = dm_get_mdptr(md);
 	if (!hc || hc->md != md) {

drivers/md/dm.c +30 -7

Index: linux-2.6/drivers/md/dm.c
===================================================================
@@ -123,6 +123,10 @@ struct mapped_device {
 	unsigned long flags;
 
 	struct request_queue *queue;
+	unsigned type;
+	/* Protect type against concurrent access. */
+	struct mutex type_lock;
+
 	struct gendisk *disk;
 	char name[16];

@@ -1874,8 +1878,10 @@ static struct mapped_device *alloc_dev(i
 	if (r < 0)
 		goto bad_minor;
 
+	md->type = DM_TYPE_NONE;
 	init_rwsem(&md->io_lock);
 	mutex_init(&md->suspend_lock);
+	mutex_init(&md->type_lock);
 	spin_lock_init(&md->deferred_lock);
 	spin_lock_init(&md->barrier_error_lock);
 	rwlock_init(&md->map_lock);

@@ -2128,6 +2134,30 @@ int dm_create(int minor, struct mapped_d
 	return 0;
 }
 
+/*
+ * Functions to manage md->type.
+ * All are required to hold md->type_lock.
+ */
+void dm_lock_md_type(struct mapped_device *md)
+{
+	mutex_lock(&md->type_lock);
+}
+
+void dm_unlock_md_type(struct mapped_device *md)
+{
+	mutex_unlock(&md->type_lock);
+}
+
+void dm_set_md_type(struct mapped_device *md, unsigned type)
+{
+	md->type = type;
+}
+
+unsigned dm_get_md_type(struct mapped_device *md)
+{
+	return md->type;
+}
+
 static struct mapped_device *dm_find_md(dev_t dev)
 {
 	struct mapped_device *md;

@@ -2403,13 +2433,6 @@ struct dm_table *dm_swap_table(struct ma
 		goto out;
 	}
 
-	/* cannot change the device type, once a table is bound */
-	if (md->map &&
-	    (dm_table_get_type(md->map) != dm_table_get_type(table))) {
-		DMWARN("can't change the device type after a table is bound");
-		goto out;
-	}
-
 	map = __bind(md, table, &limits);
 
 out:

drivers/md/dm.h +5 -0

Index: linux-2.6/drivers/md/dm.h
===================================================================
@@ -66,6 +66,11 @@ int dm_table_alloc_md_mempools(struct dm
 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_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, unsigned type);
+unsigned dm_get_md_type(struct mapped_device *md);
+
 /*
  * To check the return value from dm_table_find_target().
  */

Mike Snitzer

author

2010-06-29 17:40 UTC #1 lore raw

PATCH v5 2/2 dm: only initialize full request_queue for request-based device

A bio-based DM device no longer defaults to having a fully initialized request_queue (request_fn, elevator, etc). This means bio-based DM devices no longer register elevator sysfs attributes ('iosched/' tree or 'scheduler' other than "none").

In contrast, a request-based DM device will always have a full request_queue and will register elevator sysfs attributes. Therefore a user can determine a DM device's type by checking if elevator sysfs attributes exist.

First allocate a minimalist request_queue structure for a DM device (needed for both bio and request-based DM).

Initialization of a full request_queue is deferred until it is known that the DM device is request-based -- at the end of the table load sequence.

Factor DM device's request_queue initialization:
- common to both request-based and bio-based into dm_init_md_queue().
- specific to request-based into dm_init_request_based_queue().

The md->type_lock mutex is used to protect md->queue, in addition to md->type, during table_load().

A DM device's first table_load will establish the immutable md->type. But md->queue initialization, based on md->type, may fail at that time (because blk_init_allocated_queue cannot allocate memory). Therefore any subsequent table_load must (re)try dm_setup_md_queue independent of establishing md->type.

Signed-off-by Mike Snitzer <[email protected]>

Acked-by Kiyoshi Ueda <[email protected]>

---
v5: revised patch header, no code changes

 drivers/md/dm-ioctl.c |   11 +++++
 drivers/md/dm.c       |   94 ++++++++++++++++++++++++++++++++++++--------------
 drivers/md/dm.h       |    2 +
 3 files changed, 80 insertions(+), 27 deletions(-)

drivers/md/dm-ioctl.c +10 -1

Index: linux-2.6/drivers/md/dm-ioctl.c
===================================================================
@@ -1176,7 +1176,7 @@ static int table_load(struct dm_ioctl *p
 		goto out;
 	}
 
-	/* Protect md->type against concurrent table loads. */
+	/* Protect md->type and md->queue against concurrent table loads. */
 	dm_lock_md_type(md);
 	if (dm_get_md_type(md) == DM_TYPE_NONE) {
 		/* initial table load, set md's type based on table's type */

@@ -1188,6 +1188,15 @@ static int table_load(struct dm_ioctl *p
 		r = -EINVAL;
 		goto out;
 	}
+
+	/* setup md->queue to reflect md's type (may block) */
+	r = dm_setup_md_queue(md);
+	if (r) {
+		DMWARN("unable to setup device queue for this table.");
+		dm_table_destroy(t);
+		dm_unlock_md_type(md);
+		goto out;
+	}
 	dm_unlock_md_type(md);
 
 	/* stage inactive table */

drivers/md/dm.c +68 ~~-26~~

121 lines not on this page · raw message

drivers/md/dm.h +2 -0

Index: linux-2.6/drivers/md/dm.h
===================================================================
@@ -71,6 +71,8 @@ void dm_unlock_md_type(struct mapped_dev
 void dm_set_md_type(struct mapped_device *md, unsigned type);
 unsigned dm_get_md_type(struct mapped_device *md);
 
+int dm_setup_md_queue(struct mapped_device *md);
+
 /*
  * To check the return value from dm_table_find_target().
  */