Hi!

The upgrade_mode function in dm-table.c seems somewhat strange to me:

When dm_get_device -> __table_get_device finds that a block device is
already opened by dm but read-only and read-write access is requested it
decides to upgrade the mode. Ok so far.

upgrade_mode opens the device a second time, open_dev calls bd_claim a
second time. But dm_put_device will release the device only once.

Has this ever been tested? I think it is very unlikely that one target
will open a block device read-only and the next one read-write. Hmm.

If there is a bug, might this be a solution?


--- linux.orig/drivers/md/dm-table.c	2004-01-20 14:12:29.621546000 +0100
+++ linux/drivers/md/dm-table.c	2004-01-20 14:55:04.147199944 +0100
@@ -406,8 +406,10 @@
 	r = open_dev(dd, dev);
 	if (!r)
 		close_dev(&dd_copy);
-	else
+	else {
 		memcpy(dd, &dd_copy, sizeof(dd_copy));
+		bd_release(dd->bdev);
+	}
 
 	return r;
 }


The other way I could think of is to add paramters to open_dev and
close_dev to tell whether the bd_claim/bd_release mechanism should
be used, and then make upgrade_mode not use it.


--- linux.orig/drivers/md/dm-table.c	2004-01-20 14:12:29.621546000 +0100
+++ linux/drivers/md/dm-table.c	2004-01-20 14:51:25.786395832 +0100
@@ -343,12 +343,12 @@
 /*
  * Open a device so we can use it as a map destination.
  */
-static int open_dev(struct dm_dev *d, dev_t dev)
+static int open_dev(struct dm_dev *d, dev_t dev, int claim)
 {
 	static char *_claim_ptr = "I belong to device-mapper";
 	struct block_device *bdev;
 
-	int r;
+	int r = 0;
 
 	if (d->bdev)
 		BUG();
@@ -356,7 +356,8 @@
 	bdev = open_by_devnum(dev, d->mode, BDEV_RAW);
 	if (IS_ERR(bdev))
 		return PTR_ERR(bdev);
-	r = bd_claim(bdev, _claim_ptr);
+	if (claim)
+		r = bd_claim(bdev, _claim_ptr);
 	if (r)
 		blkdev_put(bdev, BDEV_RAW);
 	else
@@ -367,12 +368,13 @@
 /*
  * Close a device that we've been using.
  */
-static void close_dev(struct dm_dev *d)
+static void close_dev(struct dm_dev *d, int release)
 {
 	if (!d->bdev)
 		return;
 
-	bd_release(d->bdev);
+	if (release)
+		bd_release(d->bdev);
 	blkdev_put(d->bdev, BDEV_RAW);
 	d->bdev = NULL;
 }
@@ -403,9 +405,9 @@
 
 	dd->mode |= new_mode;
 	dd->bdev = NULL;
-	r = open_dev(dd, dev);
+	r = open_dev(dd, dev, 0);
 	if (!r)
-		close_dev(&dd_copy);
+		close_dev(&dd_copy, 0);
 	else
 		memcpy(dd, &dd_copy, sizeof(dd_copy));
 
@@ -448,7 +450,7 @@
 		dd->mode = mode;
 		dd->bdev = NULL;
 
-		if ((r = open_dev(dd, dev))) {
+		if ((r = open_dev(dd, dev, 1))) {
 			kfree(dd);
 			return r;
 		}
@@ -532,7 +534,7 @@
 void dm_put_device(struct dm_target *ti, struct dm_dev *dd)
 {
 	if (atomic_dec_and_test(&dd->count)) {
-		close_dev(dd);
+		close_dev(dd, 1);
 		list_del(&dd->list);
 		kfree(dd);
 	}


_______________________________________________
dm-devel mailing list
dm-devel@sistina.com
http://lists.sistina.com/mailman/listinfo/dm-devel