Re: [RFC PATCH 2/2] dm: only initialize full request_queue for request-based device

[Mike Snitzer <snitzer@redhat.com>]

On Wed, May 19 2010 at  8:01am -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Wed, May 19, 2010 at 1:57 AM, Kiyoshi Ueda <k-ueda@ct.jp.nec.com> wrote:
> > Also, your patch changes the queue configuration even when a table is
> > already active and used.  (e.g. Loading bio-based table to a mapped_device
> > which is already active/used as request-based sets q->requst_fn in NULL.)
> > That could cause some critical problems.
> 
> Yes, that is possible and I can add additional checks to prevent this.
> But this speaks to a more general problem with the existing DM code.
> 
> dm_swap_table() has the negative check to prevent such table loads, e.g.:
> /* cannot change the device type, once a table is bound */
> 
> This check should come during table_load, as part of
> dm_table_set_type(), rather than during table resume.

FYI, the following patch is the relevant portion of the v6 patch that
addresses the issue discussed above.

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 990cf17..62ebd94 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -782,6 +782,7 @@ int dm_table_set_type(struct dm_table *t)
 {
 	unsigned i;
 	unsigned bio_based = 0, request_based = 0;
+	struct dm_table *live_table = NULL;
 	struct dm_target *tgt;
 	struct dm_dev_internal *dd;
 	struct list_head *devices;
@@ -803,7 +804,7 @@ int dm_table_set_type(struct dm_table *t)
 	if (bio_based) {
 		/* We must use this table as bio-based */
 		t->type = DM_TYPE_BIO_BASED;
-		return 0;
+		goto finish;
 	}
 
 	BUG_ON(!request_based); /* No targets in this table */
@@ -831,6 +832,18 @@ int dm_table_set_type(struct dm_table *t)
 
 	t->type = DM_TYPE_REQUEST_BASED;
 
+finish:
+	/* cannot change the device type, once a table is bound */
+	live_table = dm_get_live_table(t->md);
+	if (live_table) {
+		if (dm_table_get_type(live_table) != dm_table_get_type(t)) {
+			DMWARN("can't change the device type after a table is bound");
+			dm_table_put(live_table);
+			return -EINVAL;
+		}
+		dm_table_put(live_table);
+	}
+
 	return 0;
 }
 
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index a47f0b8..923b662 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2470,13 +2470,6 @@ struct dm_table *dm_swap_table(struct mapped_device *md, struct dm_table *table)
 		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: