[PATCH] RFC: Changing dm core data structure relationships (0/5)

[PATCH] RFC: Changing dm core data structure relationships (0/5)

[Jun'ichi Nomura]

Hello,

This is a proposal of changing dm core structure for
simplification, SMP race avoidance and guaranteed release
ordering. Patches don't affect dm targets.

I would like to hear your opinion on this.

The core motivation for these changes are followings:
  - Device open and remove can race.
    As a result, the device we opened might be freed by someone.
  - We lost control on device when it's removed.
    e.g. if the device is opened and suspended, we no longer
    really remove the device.
  - With dm-table-store-md.patch in -mm, table has reference to md.
    So we would like to guarantee that table is freed before md.
It's achived in the third patch.

Patches are created and tested based on 2.6.16-rc6-mm1 plus:
  - dm-flush-queue-eintr.patch
  - dm-tidy-mdptr.patch
  - dm-table-store-md.patch
  - dm-md-dependency-tree-in-sysfs-add_subdirs.patch                           14-Mar-2006 18:33  3.2K
  - dm-md-dependency-tree-in-sysfs-bd_claim_by_kobj.patch                      14-Mar-2006 18:33  9.5K
  - dm-md-dependency-tree-in-sysfs-dm_deptree.patch                            14-Mar-2006 18:33  2.8K
  - dm-md-dependency-tree-in-sysfs-kobject_add_dir.patch                       14-Mar-2006 18:33  1.9K
  - dm-md-dependency-tree-in-sysfs-md_deptree.patch

Thanks,
-- 
Jun'ichi Nomura, NEC Solutions (America), Inc.

--------------------------------------------------------------------------
Description of patches

01-dm-sem.patch
  Literally replace _hash_lock with dm_sem. Make it extern.

02-put-table-after-unlock.patch
  Moving dm_table_put() which may destroy table to outside of dm_sem.
  This is for next patches which dm_sem is held is device close
  thus table destructor may otherwise deadlock.

03-hc-open-count.patch
  Moving open counter to hc. Disables removal of hc with openers.
  md->holders become purely a reference counter of md.
    * disk->private_data points to hc rather than md.
      When hc is removed, disk->private_data becomes NULL and no new open
      is possible.
    * Open count is protected by dm_sem to avoid open vs. remove race.
    * Upon hc removal, tables are released from md.
    * Table gets reference to md on creation, puts on free.
      Thus we can guarantee md is not freed before its tables are feed.
    * dm_get_md(), which maps minor to md, checks interface_ptr to avoid
      getting md being removed.

04-remove-dm-md-get.patch
    * dm_get_md() is removed. There is no user.

05-move-new-map-to-md.patch
  Moving hc->new_map to md->new_map.
    * Map loading no longer needs global dm_sem.
      Just grab md and lock map_lock.
    * Most of __find_device_hash_cell() can be replaced by find_device().

Should be applied in this order.

Concerns:
  - Is moving dm_blk_open/close to dm-ioctl.c right thing?
    Is it nice to rename dm-ioctl.c to dm-interface.c or something?
  - Does other .c files need to get hc->count?
  - dm_table_get_md() really needs dm_get()?
    We can assume md exists whenever the table exists.

--------------------------------------------------------------------
Structures:
  - mapped_device (md)
  - hash_cell (hc)
  - dm_table (table)
  - gendisk (disk)
  - request_queue (queue)
  - minor number (minor)

Relationships:
  - hc owns 1 md
  - md owns 0-2 tables, 1 disk
  - disk owns 1 queue and 1 minor number

Roles:
  - hc manages access from outside of dm (block device, ioctl).
    It has open counter. hc can be removed from list only when
    there is no opener.
  - md is a core of dm device. Access to table needs to go through md.

Locks:
  - dm_sem: hc list, gendisk->dm device binding
  - io_lock: DMF_BLOCK_IO, md->deferred, md->wait
  - map_lock: md->map, md->new_map
  - suspend_lock: DMF_SUSPENDED

Ordering:
  - Allocation/Register
       1) allocation of md, hc, disk, queue, minor can occur in any order
       2) register md to disk
       3) register disk (it eventually register minor and queue)
          <device visible from block layer>
       4) register md to hc
          <block device access enabled>
       5) register hc to hash list
          <ioctl access enabled>
       6) table allocation (load)
       7) table register (resume -> bind)
          <I/O enabled>
  - Freeing/Unregister (reordered proposal)
       [lock dm_sem]
       3) unregister hc from hash list
          <ioctl access disabled>
       4) unregister md from hc
       6) unregister md from disk
          <block device access rejected>
       [unlock dm_sem]
       1) table unregister (resume -> unbind)
          <I/O disabled>
       2) table free
       5) unregister disk
          <block device access disabled>
       7) free in any order

       (*) Moving 1) and 2) later has a benefit that we don't
           need to care for racing with new table registration.
       (*) Swapping the order of 5) and 6), because 5) eventually
           cause kobject_uevent(hotplug) which may sleep.
           It's not good thing to do in global dm_sem.
           Doing 6) makes future open to fail so it has same shutdown
           effect and faster.

[PATCH] RFC: Changing dm core (1/5): Make _hash_lock extern

[Jun'ichi Nomura]

[-- Attachment #1: Type: text/plain, Size: 146 bytes --]

This patch makes _hash_lock extern so that dm.c can touch it.
It's renamed to dm_sem.

Thanks,
-- 
Jun'ichi Nomura, NEC Solutions (America), Inc.

[-- Attachment #2: 01-dm-sem.patch --]
[-- Type: text/x-patch, Size: 5688 bytes --]

Rename _hash_lock to dm_sem and make it extern for dm.c.

Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>

--- linux-2.6.16-rc6-mm1-dm.orig/drivers/md/dm.h	2006-03-13 12:16:38.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.01-dm-sem/drivers/md/dm.h	2006-03-16 16:24:31.000000000 -0500
@@ -172,6 +172,7 @@ int dm_split_args(int *argc, char ***arg
  */
 int dm_interface_init(void);
 void dm_interface_exit(void);
+extern struct rw_semaphore dm_sem;
 
 /*
  * Targets for linear and striped mappings
--- linux-2.6.16-rc6-mm1-dm.orig/drivers/md/dm-ioctl.c	2006-03-13 11:20:09.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.01-dm-sem/drivers/md/dm-ioctl.c	2006-03-13 17:35:40.000000000 -0500
@@ -52,7 +52,7 @@ static void dm_hash_remove_all(void);
 /*
  * Guards access to both hash tables.
  */
-static DECLARE_RWSEM(_hash_lock);
+DECLARE_RWSEM(dm_sem);
 
 static void init_buckets(struct list_head *buckets)
 {
@@ -202,7 +202,7 @@ static int dm_hash_insert(const char *na
 	/*
 	 * Insert the cell into both hash tables.
 	 */
-	down_write(&_hash_lock);
+	down_write(&dm_sem);
 	if (__get_name_cell(name))
 		goto bad;
 
@@ -218,12 +218,12 @@ static int dm_hash_insert(const char *na
 	register_with_devfs(cell);
 	dm_get(md);
 	dm_set_mdptr(md, cell);
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 
 	return 0;
 
  bad:
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 	free_cell(cell);
 	return -EBUSY;
 }
@@ -256,14 +256,14 @@ static void dm_hash_remove_all(void)
 	struct hash_cell *hc;
 	struct list_head *tmp, *n;
 
-	down_write(&_hash_lock);
+	down_write(&dm_sem);
 	for (i = 0; i < NUM_BUCKETS; i++) {
 		list_for_each_safe (tmp, n, _name_buckets + i) {
 			hc = list_entry(tmp, struct hash_cell, name_list);
 			__hash_remove(hc);
 		}
 	}
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 }
 
 static int dm_hash_rename(const char *old, const char *new)
@@ -279,7 +279,7 @@ static int dm_hash_rename(const char *ol
 	if (!new_name)
 		return -ENOMEM;
 
-	down_write(&_hash_lock);
+	down_write(&dm_sem);
 
 	/*
 	 * Is new free ?
@@ -288,7 +288,7 @@ static int dm_hash_rename(const char *ol
 	if (hc) {
 		DMWARN("asked to rename to an already existing name %s -> %s",
 		       old, new);
-		up_write(&_hash_lock);
+		up_write(&dm_sem);
 		kfree(new_name);
 		return -EBUSY;
 	}
@@ -300,7 +300,7 @@ static int dm_hash_rename(const char *ol
 	if (!hc) {
 		DMWARN("asked to rename a non existent device %s -> %s",
 		       old, new);
-		up_write(&_hash_lock);
+		up_write(&dm_sem);
 		kfree(new_name);
 		return -ENXIO;
 	}
@@ -327,7 +327,7 @@ static int dm_hash_rename(const char *ol
 		dm_table_put(table);
 	}
 
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 	kfree(old_name);
 	return 0;
 }
@@ -382,7 +382,7 @@ static int list_devices(struct dm_ioctl 
 	struct gendisk *disk;
 	struct dm_name_list *nl, *old_nl = NULL;
 
-	down_write(&_hash_lock);
+	down_write(&dm_sem);
 
 	/*
 	 * Loop through all the devices working out how much
@@ -427,7 +427,7 @@ static int list_devices(struct dm_ioctl 
 	}
 
  out:
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 	return 0;
 }
 
@@ -623,7 +623,7 @@ static struct mapped_device *find_device
 	struct hash_cell *hc;
 	struct mapped_device *md = NULL;
 
-	down_read(&_hash_lock);
+	down_read(&dm_sem);
 	hc = __find_device_hash_cell(param);
 	if (hc) {
 		md = hc->md;
@@ -644,7 +644,7 @@ static struct mapped_device *find_device
 		else
 			param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
 	}
-	up_read(&_hash_lock);
+	up_read(&dm_sem);
 
 	return md;
 }
@@ -653,17 +653,17 @@ static int dev_remove(struct dm_ioctl *p
 {
 	struct hash_cell *hc;
 
-	down_write(&_hash_lock);
+	down_write(&dm_sem);
 	hc = __find_device_hash_cell(param);
 
 	if (!hc) {
 		DMWARN("device doesn't appear to be in the dev hash table.");
-		up_write(&_hash_lock);
+		up_write(&dm_sem);
 		return -ENXIO;
 	}
 
 	__hash_remove(hc);
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 	param->data_size = 0;
 	return 0;
 }
@@ -731,12 +731,12 @@ static int do_resume(struct dm_ioctl *pa
 	struct mapped_device *md;
 	struct dm_table *new_map;
 
-	down_write(&_hash_lock);
+	down_write(&dm_sem);
 
 	hc = __find_device_hash_cell(param);
 	if (!hc) {
 		DMWARN("device doesn't appear to be in the dev hash table.");
-		up_write(&_hash_lock);
+		up_write(&dm_sem);
 		return -ENXIO;
 	}
 
@@ -747,7 +747,7 @@ static int do_resume(struct dm_ioctl *pa
 	hc->new_map = NULL;
 	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
 
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 
 	/* Do we need to load a new map ? */
 	if (new_map) {
@@ -1001,12 +1001,12 @@ static int table_load(struct dm_ioctl *p
 		goto out;
 	}
 
-	down_write(&_hash_lock);
+	down_write(&dm_sem);
 	hc = dm_get_mdptr(md);
 	if (!hc || hc->md != md) {
 		DMWARN("device has been removed from the dev hash table.");
 		dm_table_put(t);
-		up_write(&_hash_lock);
+		up_write(&dm_sem);
 		r = -ENXIO;
 		goto out;
 	}
@@ -1014,7 +1014,7 @@ static int table_load(struct dm_ioctl *p
 	if (hc->new_map)
 		dm_table_put(hc->new_map);
 	hc->new_map = t;
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 
 	param->flags |= DM_INACTIVE_PRESENT_FLAG;
 	r = __dev_status(md, param);
@@ -1030,12 +1030,12 @@ static int table_clear(struct dm_ioctl *
 	int r;
 	struct hash_cell *hc;
 
-	down_write(&_hash_lock);
+	down_write(&dm_sem);
 
 	hc = __find_device_hash_cell(param);
 	if (!hc) {
 		DMWARN("device doesn't appear to be in the dev hash table.");
-		up_write(&_hash_lock);
+		up_write(&dm_sem);
 		return -ENXIO;
 	}
 
@@ -1047,7 +1047,7 @@ static int table_clear(struct dm_ioctl *
 	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
 
 	r = __dev_status(hc->md, param);
-	up_write(&_hash_lock);
+	up_write(&dm_sem);
 	return r;
 }
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

[PATCH] RFC: Changing dm core (2/5) : Put_table in safer place

[Jun'ichi Nomura]

[-- Attachment #1: Type: text/plain, Size: 99 bytes --]

Move dm_table_put() outside of dm_sem.

Thanks,
-- 
Jun'ichi Nomura, NEC Solutions (America), Inc.

[-- Attachment #2: 02-put-table-after-unlock.patch --]
[-- Type: text/x-patch, Size: 1680 bytes --]

Move dm_table_put() outside of dm_sem.

Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>

 dm-ioctl.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

--- linux-2.6.16-rc6-mm1-dm.01-dm-sem/drivers/md/dm-ioctl.c	2006-03-13 17:35:40.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.02-put-table-after-unlock/drivers/md/dm-ioctl.c	2006-03-15 10:15:11.000000000 -0500
@@ -984,7 +984,7 @@ static int table_load(struct dm_ioctl *p
 {
 	int r;
 	struct hash_cell *hc;
-	struct dm_table *t;
+	struct dm_table *t, *oldmap = NULL;
 	struct mapped_device *md;
 
 	md = find_device(param);
@@ -1005,14 +1005,14 @@ static int table_load(struct dm_ioctl *p
 	hc = dm_get_mdptr(md);
 	if (!hc || hc->md != md) {
 		DMWARN("device has been removed from the dev hash table.");
-		dm_table_put(t);
+		oldmap = t;
 		up_write(&dm_sem);
 		r = -ENXIO;
 		goto out;
 	}
 
 	if (hc->new_map)
-		dm_table_put(hc->new_map);
+		oldmap = hc->new_map;
 	hc->new_map = t;
 	up_write(&dm_sem);
 
@@ -1020,6 +1020,8 @@ static int table_load(struct dm_ioctl *p
 	r = __dev_status(md, param);
 
 out:
+	if (oldmap)
+		dm_table_put(oldmap);
 	dm_put(md);
 
 	return r;
@@ -1029,6 +1031,7 @@ static int table_clear(struct dm_ioctl *
 {
 	int r;
 	struct hash_cell *hc;
+	struct dm_table *oldmap = NULL;
 
 	down_write(&dm_sem);
 
@@ -1040,7 +1043,7 @@ static int table_clear(struct dm_ioctl *
 	}
 
 	if (hc->new_map) {
-		dm_table_put(hc->new_map);
+		oldmap = hc->new_map;
 		hc->new_map = NULL;
 	}
 
@@ -1048,6 +1051,10 @@ static int table_clear(struct dm_ioctl *
 
 	r = __dev_status(hc->md, param);
 	up_write(&dm_sem);
+
+	if (oldmap)
+		dm_table_put(oldmap);
+
 	return r;
 }
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

[PATCH] RFC: Changing dm core (3/5): hash_cell open counter

[Jun'ichi Nomura]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

Move open counter from md to hash_cell.
The counter is checked at hash_cell removal.
Non-zero counter disallows removal.

As a result, md->holders becomes pure reference counter of md.
It guarantees md is not freed until all tables are freed.

By protecting the counter, the race between open and remove
can be avoided.

Thanks,
-- 
Jun'ichi Nomura, NEC Solutions (America), Inc.

[-- Attachment #2: 03-hc-open-count.patch --]
[-- Type: text/x-patch, Size: 10592 bytes --]

Move open counter from md to hash_cell.
The counter is checked at hash_cell removal.
Non-zero counter disallows removal.

As a result, md->holders becomes pure reference counter of md.
It guarantees md is not freed until all tables are freed.

Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>

 dm-ioctl.c |  133 ++++++++++++++++++++++++++++++++++++++++++++++---------------
 dm-table.c |    2 
 dm.c       |   63 +++++++++++++---------------
 dm.h       |    1 
 4 files changed, 134 insertions(+), 65 deletions(-)

--- linux-2.6.16-rc6-mm1-dm.02-put-table-after-unlock/drivers/md/dm.c	2006-03-16 14:22:16.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.03-hc-open-count/drivers/md/dm.c	2006-03-16 18:23:51.000000000 -0500
@@ -206,27 +206,6 @@ static void __exit dm_exit(void)
 		_exits[i]();
 }
 
-/*
- * Block device functions
- */
-static int dm_blk_open(struct inode *inode, struct file *file)
-{
-	struct mapped_device *md;
-
-	md = inode->i_bdev->bd_disk->private_data;
-	dm_get(md);
-	return 0;
-}
-
-static int dm_blk_close(struct inode *inode, struct file *file)
-{
-	struct mapped_device *md;
-
-	md = inode->i_bdev->bd_disk->private_data;
-	dm_put(md);
-	return 0;
-}
-
 static inline struct dm_io *alloc_io(struct mapped_device *md)
 {
 	return mempool_alloc(md->io_pool, GFP_NOIO);
@@ -835,7 +814,6 @@ static struct mapped_device *alloc_dev(u
 	md->disk->first_minor = minor;
 	md->disk->fops = &dm_blk_dops;
 	md->disk->queue = md->queue;
-	md->disk->private_data = md;
 	sprintf(md->disk->disk_name, "dm-%d", minor);
 	add_disk(md->disk);
 
@@ -861,6 +839,11 @@ static void free_dev(struct mapped_devic
 {
 	unsigned int minor = md->disk->first_minor;
 
+	if (atomic_read(&md->holders) || md->interface_ptr) {
+		DMERR("Trying to free a device which still has user");
+		BUG();
+	}
+
 	if (md->suspended_bdev) {
 		thaw_bdev(md->suspended_bdev, NULL);
 		bdput(md->suspended_bdev);
@@ -978,8 +961,10 @@ struct mapped_device *dm_get_md(dev_t de
 {
 	struct mapped_device *md = dm_find_md(dev);
 
-	if (md)
-		dm_get(md);
+	if (!md || !md->interface_ptr)
+		return NULL;
+
+	dm_get(md);
 
 	return md;
 }
@@ -1001,18 +986,25 @@ void dm_get(struct mapped_device *md)
 
 void dm_put(struct mapped_device *md)
 {
+	if (atomic_dec_and_test(&md->holders))
+		free_dev(md);
+}
+
+/*
+ * Called when md is released from interface container
+ */
+void dm_free(struct mapped_device *md)
+{
 	struct dm_table *map;
 
-	if (atomic_dec_and_test(&md->holders)) {
-		map = dm_get_table(md);
-		if (!dm_suspended(md)) {
-			dm_table_presuspend_targets(map);
-			dm_table_postsuspend_targets(map);
-		}
-		__unbind(md);
-		dm_table_put(map);
-		free_dev(md);
+	map = dm_get_table(md);
+	if (!dm_suspended(md)) {
+		dm_table_presuspend_targets(map);
+		dm_table_postsuspend_targets(map);
 	}
+	dm_table_put(map);
+	__unbind(md);
+	dm_put(md); /* this likely to be the last put */
 }
 
 /*
@@ -1252,6 +1244,11 @@ int dm_suspended(struct mapped_device *m
 	return test_bit(DMF_SUSPENDED, &md->flags);
 }
 
+/*
+ * Block device interface
+ */
+int dm_blk_open(struct inode *inode, struct file *file);
+int dm_blk_close(struct inode *inode, struct file *file);
 static struct block_device_operations dm_blk_dops = {
 	.open = dm_blk_open,
 	.release = dm_blk_close,
--- linux-2.6.16-rc6-mm1-dm.02-put-table-after-unlock/drivers/md/dm.h	2006-03-16 16:24:31.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.03-hc-open-count/drivers/md/dm.h	2006-03-16 18:19:28.000000000 -0500
@@ -55,6 +55,7 @@ struct mapped_device *dm_get_md(dev_t de
  */
 void dm_get(struct mapped_device *md);
 void dm_put(struct mapped_device *md);
+void dm_free(struct mapped_device *md);
 
 /*
  * A device can still be used while suspended, but I/O is deferred.
--- linux-2.6.16-rc6-mm1-dm.02-put-table-after-unlock/drivers/md/dm-ioctl.c	2006-03-15 10:15:11.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.03-hc-open-count/drivers/md/dm-ioctl.c	2006-03-16 15:38:26.000000000 -0500
@@ -28,6 +28,7 @@ struct hash_cell {
 	struct list_head name_list;
 	struct list_head uuid_list;
 
+	atomic_t count;
 	char *name;
 	char *uuid;
 	struct mapped_device *md;
@@ -54,6 +55,24 @@ static void dm_hash_remove_all(void);
  */
 DECLARE_RWSEM(dm_sem);
 
+/*
+ * Protects links between block device / ioctl and hash_cell
+ */
+static void __get_cell(struct hash_cell *hc)
+{
+	atomic_inc(&hc->count);
+}
+
+static void __put_cell(struct hash_cell *hc)
+{
+	atomic_dec(&hc->count);
+}
+
+static void __put_md(struct mapped_device *md)
+{
+	__put_cell(dm_get_mdptr(md));
+}
+
 static void init_buckets(struct list_head *buckets)
 {
 	unsigned int i;
@@ -153,6 +172,7 @@ static struct hash_cell *alloc_cell(cons
 	INIT_LIST_HEAD(&hc->uuid_list);
 	hc->md = md;
 	hc->new_map = NULL;
+	atomic_set(&hc->count, 0);
 	return hc;
 }
 
@@ -218,6 +238,7 @@ static int dm_hash_insert(const char *na
 	register_with_devfs(cell);
 	dm_get(md);
 	dm_set_mdptr(md, cell);
+	dm_disk(md)->private_data = cell;
 	up_write(&dm_sem);
 
 	return 0;
@@ -228,6 +249,9 @@ static int dm_hash_insert(const char *na
 	return -EBUSY;
 }
 
+/*
+ * Should be called under dm_sem
+ */
 static void __hash_remove(struct hash_cell *hc)
 {
 	struct dm_table *table;
@@ -237,33 +261,55 @@ static void __hash_remove(struct hash_ce
 	list_del(&hc->name_list);
 	unregister_with_devfs(hc);
 	dm_set_mdptr(hc->md, NULL);
+	dm_put(hc->md);
+	dm_disk(hc->md)->private_data = NULL;
 
 	table = dm_get_table(hc->md);
 	if (table) {
 		dm_table_event(table);
 		dm_table_put(table);
 	}
+}
 
+/*
+ * Called outside of dm_sem
+ */
+static void __hash_free(struct hash_cell *hc)
+{
 	if (hc->new_map)
 		dm_table_put(hc->new_map);
-	dm_put(hc->md);
+	dm_free(hc->md);
 	free_cell(hc);
 }
 
 static void dm_hash_remove_all(void)
 {
-	int i;
+	int i, freed;
 	struct hash_cell *hc;
-	struct list_head *tmp, *n;
+	struct list_head *t, *n, r;
 
+rescan:
+	INIT_LIST_HEAD(&r);
 	down_write(&dm_sem);
-	for (i = 0; i < NUM_BUCKETS; i++) {
-		list_for_each_safe (tmp, n, _name_buckets + i) {
-			hc = list_entry(tmp, struct hash_cell, name_list);
-			__hash_remove(hc);
-		}
-	}
+	do {
+		freed = 0;
+		for (i = 0; i < NUM_BUCKETS; i++)
+			list_for_each_safe (t, n, _name_buckets + i) {
+				hc = list_entry(t, struct hash_cell, name_list);
+				if (atomic_read(&hc->count) == 0) {
+					__hash_remove(hc);
+					list_add(&hc->name_list, &r);
+					freed++;
+				}
+			}
+	} while (freed > 0);
 	up_write(&dm_sem);
+
+	if (!list_empty(&r)) {
+		list_for_each_safe (t, n, &r)
+			__hash_free(list_entry(t, struct hash_cell, name_list));
+		goto rescan;
+	}
 }
 
 static int dm_hash_rename(const char *old, const char *new)
@@ -583,14 +629,13 @@ static int dev_create(struct dm_ioctl *p
 
 	r = dm_hash_insert(param->name, *param->uuid ? param->uuid : NULL, md);
 	if (r) {
-		dm_put(md);
+		dm_free(md);
 		return r;
 	}
 
 	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
 
 	r = __dev_status(md, param);
-	dm_put(md);
 
 	return r;
 }
@@ -600,22 +645,13 @@ static int dev_create(struct dm_ioctl *p
  */
 static struct hash_cell *__find_device_hash_cell(struct dm_ioctl *param)
 {
-	struct mapped_device *md;
-	void *mdptr = NULL;
-
 	if (*param->uuid)
 		return __get_uuid_cell(param->uuid);
 
 	if (*param->name)
 		return __get_name_cell(param->name);
 
-	md = dm_get_md(huge_decode_dev(param->dev));
-	if (md) {
-		mdptr = dm_get_mdptr(md);
-		dm_put(md);
-	}
-
-	return mdptr;
+	return NULL;
 }
 
 static struct mapped_device *find_device(struct dm_ioctl *param)
@@ -627,7 +663,7 @@ static struct mapped_device *find_device
 	hc = __find_device_hash_cell(param);
 	if (hc) {
 		md = hc->md;
-		dm_get(md);
+		__get_cell(hc);
 
 		/*
 		 * Sneakily write in both the name and the uuid
@@ -662,8 +698,14 @@ static int dev_remove(struct dm_ioctl *p
 		return -ENXIO;
 	}
 
+	if (atomic_read(&hc->count)) {
+		up_write(&dm_sem);
+		return -EBUSY;
+	}
+
 	__hash_remove(hc);
 	up_write(&dm_sem);
+	__hash_free(hc);
 	param->data_size = 0;
 	return 0;
 }
@@ -719,7 +761,7 @@ static int do_suspend(struct dm_ioctl *p
 	if (!r)
 		r = __dev_status(md, param);
 
-	dm_put(md);
+	__put_md(md);
 	return r;
 }
 
@@ -741,7 +783,7 @@ static int do_resume(struct dm_ioctl *pa
 	}
 
 	md = hc->md;
-	dm_get(md);
+	atomic_inc(&hc->count);
 
 	new_map = hc->new_map;
 	hc->new_map = NULL;
@@ -759,7 +801,7 @@ static int do_resume(struct dm_ioctl *pa
 
 		r = dm_swap_table(md, new_map);
 		if (r) {
-			dm_put(md);
+			__put_md(md);
 			dm_table_put(new_map);
 			return r;
 		}
@@ -778,7 +820,7 @@ static int do_resume(struct dm_ioctl *pa
 	if (!r)
 		r = __dev_status(md, param);
 
-	dm_put(md);
+	__put_md(md);
 	return r;
 }
 
@@ -808,7 +850,7 @@ static int dev_status(struct dm_ioctl *p
 		return -ENXIO;
 
 	r = __dev_status(md, param);
-	dm_put(md);
+	__put_md(md);
 	return r;
 }
 
@@ -916,7 +958,7 @@ static int dev_wait(struct dm_ioctl *par
 	}
 
  out:
-	dm_put(md);
+	__put_md(md);
 	return r;
 }
 
@@ -1022,7 +1064,7 @@ static int table_load(struct dm_ioctl *p
 out:
 	if (oldmap)
 		dm_table_put(oldmap);
-	dm_put(md);
+	__put_md(md);
 
 	return r;
 }
@@ -1119,7 +1161,7 @@ static int table_deps(struct dm_ioctl *p
 	}
 
  out:
-	dm_put(md);
+	__put_md(md);
 	return r;
 }
 
@@ -1148,7 +1190,7 @@ static int table_status(struct dm_ioctl 
 	}
 
  out:
-	dm_put(md);
+	__put_md(md);
 	return r;
 }
 
@@ -1209,7 +1251,7 @@ static int target_message(struct dm_ioct
 	kfree(argv);
  out:
 	param->data_size = 0;
-	dm_put(md);
+	__put_md(md);
 	return r;
 }
 
@@ -1424,6 +1466,33 @@ static struct miscdevice _dm_misc = {
 };
 
 /*
+ * Block device interface
+ */
+int dm_blk_open(struct inode *inode, struct file *file)
+{
+	struct hash_cell *hc;
+
+	down_read(&dm_sem);
+	hc = inode->i_bdev->bd_disk->private_data;
+	if (hc)
+		__get_cell(hc);
+	up_read(&dm_sem);
+	return hc ? 0 : -ENXIO;
+}
+
+int dm_blk_close(struct inode *inode, struct file *file)
+{
+	struct hash_cell *hc;
+
+	down_read(&dm_sem);
+	hc = inode->i_bdev->bd_disk->private_data;
+	if (hc)
+		__put_cell(hc);
+	up_read(&dm_sem);
+	return 0;
+}
+
+/*
  * Create misc character device and link to DM_DIR/control.
  */
 int __init dm_interface_init(void)
--- linux-2.6.16-rc6-mm1-dm.02-put-table-after-unlock/drivers/md/dm-table.c	2006-03-13 14:58:13.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.03-hc-open-count/drivers/md/dm-table.c	2006-03-16 00:37:43.000000000 -0500
@@ -233,6 +233,7 @@ int dm_table_create(struct dm_table **re
 
 	t->mode = mode;
 	t->md = md;
+	dm_get(md);
 	*result = t;
 	return 0;
 }
@@ -276,6 +277,7 @@ static void table_destroy(struct dm_tabl
 		free_devices(&t->devices);
 	}
 
+	dm_put(t->md);
 	kfree(t);
 }
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

[PATCH] RFC: Changing dm core (4/5): remove dm_get_md()

[Jun'ichi Nomura]

[-- Attachment #1: Type: text/plain, Size: 105 bytes --]

Remove dm_get_md(), which is no longer used.

Thanks,
-- 
Jun'ichi Nomura, NEC Solutions (America), Inc.

[-- Attachment #2: 04-remove-dm-get-md.patch --]
[-- Type: text/x-patch, Size: 1648 bytes --]

Remove dm_get_md(), which is no longer used.

Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>

 dm.c |   31 -------------------------------
 dm.h |    1 -
 2 files changed, 32 deletions(-)

--- linux-2.6.16-rc6-mm1-dm.03-hc-open-count/drivers/md/dm.c	2006-03-16 18:23:51.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.04-remove-dm-get-md/drivers/md/dm.c	2006-03-16 18:15:58.000000000 -0500
@@ -938,37 +938,6 @@ int dm_create_with_minor(unsigned int mi
 	return create_aux(minor, 1, result);
 }
 
-static struct mapped_device *dm_find_md(dev_t dev)
-{
-	struct mapped_device *md;
-	unsigned minor = MINOR(dev);
-
-	if (MAJOR(dev) != _major || minor >= (1 << MINORBITS))
-		return NULL;
-
-	mutex_lock(&_minor_lock);
-
-	md = idr_find(&_minor_idr, minor);
-	if (!md || (dm_disk(md)->first_minor != minor))
-		md = NULL;
-
-	mutex_unlock(&_minor_lock);
-
-	return md;
-}
-
-struct mapped_device *dm_get_md(dev_t dev)
-{
-	struct mapped_device *md = dm_find_md(dev);
-
-	if (!md || !md->interface_ptr)
-		return NULL;
-
-	dm_get(md);
-
-	return md;
-}
-
 void *dm_get_mdptr(struct mapped_device *md)
 {
 	return md->interface_ptr;
--- linux-2.6.16-rc6-mm1-dm.03-hc-open-count/drivers/md/dm.h	2006-03-16 18:19:28.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.04-remove-dm-get-md/drivers/md/dm.h	2006-03-16 18:19:35.000000000 -0500
@@ -48,7 +48,6 @@ int dm_create(struct mapped_device **md)
 int dm_create_with_minor(unsigned int minor, struct mapped_device **md);
 void dm_set_mdptr(struct mapped_device *md, void *ptr);
 void *dm_get_mdptr(struct mapped_device *md);
-struct mapped_device *dm_get_md(dev_t dev);
 
 /*
  * Reference counting for md.

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

[PATCH] RFC: Changing dm core: (5/5): Move new_map from hash_cell to mapped_device

[Jun'ichi Nomura]

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]

Move new_map from hash_cell to mapped_device.

This simplifies dm-ioctl.c a bit.

Thanks,
-- 
Jun'ichi Nomura, NEC Solutions (America), Inc.

[-- Attachment #2: 05-move-new-map-to-md.patch --]
[-- Type: text/x-patch, Size: 7925 bytes --]

Move new_map from hash_cell to mapped_device.

Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>

 dm-ioctl.c |   72 +++++++++++--------------------------------------------------
 dm.c       |   58 +++++++++++++++++++++++++++++++++++++++++++++++--
 dm.h       |   10 +++++++-
 3 files changed, 78 insertions(+), 62 deletions(-)

--- linux-2.6.16-rc6-mm1-dm.04-remove-dm-get-md/drivers/md/dm.c	2006-03-16 18:15:58.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.05-move-new-map-to-md/drivers/md/dm.c	2006-03-16 18:16:10.000000000 -0500
@@ -84,6 +84,10 @@ struct mapped_device {
 	 * The current mapping.
 	 */
 	struct dm_table *map;
+	/*
+	 * Loaded, inactive mapping.
+	 */
+	struct dm_table *new_map;
 
 	/*
 	 * io objects are allocated from here.
@@ -877,6 +881,18 @@ static void __set_size(struct mapped_dev
 	mutex_unlock(&md->suspended_bdev->bd_inode->i_mutex);
 }
 
+static struct dm_table *__extract_new_map(struct mapped_device *md)
+{
+	struct dm_table *t;
+
+	write_lock(&md->map_lock);
+	t = md->new_map;
+	md->new_map = NULL;
+	write_unlock(&md->map_lock);
+
+	return t;
+}
+
 static int __bind(struct mapped_device *md, struct dm_table *t)
 {
 	request_queue_t *q = md->queue;
@@ -887,7 +903,6 @@ static int __bind(struct mapped_device *
 	if (size == 0)
 		return 0;
 
-	dm_table_get(t);
 	dm_table_event_callback(t, event_callback, md);
 
 	write_lock(&md->map_lock);
@@ -966,6 +981,7 @@ void dm_free(struct mapped_device *md)
 {
 	struct dm_table *map;
 
+	dm_table_put(md->new_map);
 	map = dm_get_table(md);
 	if (!dm_suspended(md)) {
 		dm_table_presuspend_targets(map);
@@ -994,9 +1010,10 @@ static void __flush_deferred_io(struct m
 /*
  * Swap in a new table (destroying old one).
  */
-int dm_swap_table(struct mapped_device *md, struct dm_table *table)
+int dm_swap_table(struct mapped_device *md)
 {
 	int r = -EINVAL;
+	sturct dm_table *table;
 
 	down(&md->suspend_lock);
 
@@ -1004,6 +1021,13 @@ int dm_swap_table(struct mapped_device *
 	if (!dm_suspended(md))
 		goto out;
 
+	/* no-op if no table is loaded */
+	table = __extract_new_map(md);
+	if (!table) {
+		r = 0;
+		goto out;
+	}
+
 	__unbind(md);
 	r = __bind(md, table);
 
@@ -1012,6 +1036,26 @@ out:
 	return r;
 }
 
+ /*
+  * Load a new table (or clear a loaded table if t == NULL).
+  * Should be called outside of dm_sem.
+  */
+int dm_load_table(struct mapped_device *md, struct dm_table *t)
+{
+	struct dm_table *old_map = NULL;
+
+	write_lock(&md->map_lock);
+	if (md->new_map)
+		old_map = md->new_map;
+	md->new_map = t;
+	write_unlock(&md->map_lock);
+
+	if (old_map)
+		dm_table_put(old_map);
+
+	return 1;
+}
+
 /*
  * Functions to lock and unlock any filesystem running on the
  * device.
@@ -1213,6 +1257,16 @@ int dm_suspended(struct mapped_device *m
 	return test_bit(DMF_SUSPENDED, &md->flags);
 }
 
+int dm_inactive_present(struct mapped_device *md)
+{
+	return md->new_map != NULL;
+}
+
+int dm_get_mode(struct mapped_device *md)
+{
+	return md->map ? dm_table_get_mode(md->map) : 0;
+}
+
 /*
  * Block device interface
  */
--- linux-2.6.16-rc6-mm1-dm.04-remove-dm-get-md/drivers/md/dm.h	2006-03-16 18:19:35.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.05-move-new-map-to-md/drivers/md/dm.h	2006-03-16 15:48:13.000000000 -0500
@@ -65,7 +65,13 @@ int dm_resume(struct mapped_device *md);
 /*
  * The device must be suspended before calling this method.
  */
-int dm_swap_table(struct mapped_device *md, struct dm_table *t);
+int dm_swap_table(struct mapped_device *md);
+
+/*
+ * No lock is needed
+ */
+int dm_load_table(struct mapped_device *md, struct dm_table *t);
+
 
 /*
  * Drop a reference on the table when you've finished with the
@@ -84,6 +90,8 @@ int dm_wait_event(struct mapped_device *
  */
 struct gendisk *dm_disk(struct mapped_device *md);
 int dm_suspended(struct mapped_device *md);
+int dm_inactive_present(struct mapped_device *md);
+int dm_get_mode(struct mapped_device *md);
 
 /*-----------------------------------------------------------------
  * Functions for manipulating a table.  Tables are also reference
--- linux-2.6.16-rc6-mm1-dm.04-remove-dm-get-md/drivers/md/dm-ioctl.c	2006-03-16 15:38:26.000000000 -0500
+++ linux-2.6.16-rc6-mm1-dm.05-move-new-map-to-md/drivers/md/dm-ioctl.c	2006-03-16 15:53:19.000000000 -0500
@@ -32,7 +32,6 @@ struct hash_cell {
 	char *name;
 	char *uuid;
 	struct mapped_device *md;
-	struct dm_table *new_map;
 };
 
 struct vers_iter {
@@ -171,7 +170,6 @@ static struct hash_cell *alloc_cell(cons
 	INIT_LIST_HEAD(&hc->name_list);
 	INIT_LIST_HEAD(&hc->uuid_list);
 	hc->md = md;
-	hc->new_map = NULL;
 	atomic_set(&hc->count, 0);
 	return hc;
 }
@@ -276,8 +274,6 @@ static void __hash_remove(struct hash_ce
  */
 static void __hash_free(struct hash_cell *hc)
 {
-	if (hc->new_map)
-		dm_table_put(hc->new_map);
 	dm_free(hc->md);
 	free_cell(hc);
 }
@@ -675,7 +671,7 @@ static struct mapped_device *find_device
 		else
 			param->uuid[0] = '\0';
 
-		if (hc->new_map)
+		if (dm_inactive_present(md))
 			param->flags |= DM_INACTIVE_PRESENT_FLAG;
 		else
 			param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
@@ -769,44 +765,29 @@ static int do_resume(struct dm_ioctl *pa
 {
 	int r = 0;
 	int do_lockfs = 1;
-	struct hash_cell *hc;
 	struct mapped_device *md;
-	struct dm_table *new_map;
-
-	down_write(&dm_sem);
 
-	hc = __find_device_hash_cell(param);
-	if (!hc) {
+	md = find_device(param);
+	if (!md) {
 		DMWARN("device doesn't appear to be in the dev hash table.");
-		up_write(&dm_sem);
 		return -ENXIO;
 	}
 
-	md = hc->md;
-	atomic_inc(&hc->count);
-
-	new_map = hc->new_map;
-	hc->new_map = NULL;
-	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
-
-	up_write(&dm_sem);
-
 	/* Do we need to load a new map ? */
-	if (new_map) {
+	if (dm_inactive_present(md)) {
 		/* Suspend if it isn't already suspended */
 		if (param->flags & DM_SKIP_LOCKFS_FLAG)
 			do_lockfs = 0;
 		if (!dm_suspended(md))
 			dm_suspend(md, do_lockfs);
 
-		r = dm_swap_table(md, new_map);
+		r = dm_swap_table(md);
 		if (r) {
 			__put_md(md);
-			dm_table_put(new_map);
 			return r;
 		}
 
-		if (dm_table_get_mode(new_map) & FMODE_WRITE)
+		if (dm_get_mode(md) & FMODE_WRITE)
 			set_disk_ro(dm_disk(md), 0);
 		else
 			set_disk_ro(dm_disk(md), 1);
@@ -817,6 +798,7 @@ static int do_resume(struct dm_ioctl *pa
 	if (dm_suspended(md))
 		r = dm_resume(md);
 
+	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
 	if (!r)
 		r = __dev_status(md, param);
 
@@ -1025,8 +1007,7 @@ static int populate_table(struct dm_tabl
 static int table_load(struct dm_ioctl *param, size_t param_size)
 {
 	int r;
-	struct hash_cell *hc;
-	struct dm_table *t, *oldmap = NULL;
+	struct dm_table *t;
 	struct mapped_device *md;
 
 	md = find_device(param);
@@ -1043,27 +1024,12 @@ static int table_load(struct dm_ioctl *p
 		goto out;
 	}
 
-	down_write(&dm_sem);
-	hc = dm_get_mdptr(md);
-	if (!hc || hc->md != md) {
-		DMWARN("device has been removed from the dev hash table.");
-		oldmap = t;
-		up_write(&dm_sem);
-		r = -ENXIO;
-		goto out;
-	}
-
-	if (hc->new_map)
-		oldmap = hc->new_map;
-	hc->new_map = t;
-	up_write(&dm_sem);
+	dm_load_table(md, t);
 
 	param->flags |= DM_INACTIVE_PRESENT_FLAG;
 	r = __dev_status(md, param);
 
 out:
-	if (oldmap)
-		dm_table_put(oldmap);
 	__put_md(md);
 
 	return r;
@@ -1072,30 +1038,18 @@ out:
 static int table_clear(struct dm_ioctl *param, size_t param_size)
 {
 	int r;
-	struct hash_cell *hc;
-	struct dm_table *oldmap = NULL;
-
-	down_write(&dm_sem);
+	struct mapped_device *md;
 
-	hc = __find_device_hash_cell(param);
-	if (!hc) {
+	md = find_device(param);
+	if (!md) {
 		DMWARN("device doesn't appear to be in the dev hash table.");
-		up_write(&dm_sem);
 		return -ENXIO;
 	}
 
-	if (hc->new_map) {
-		oldmap = hc->new_map;
-		hc->new_map = NULL;
-	}
+	dm_load_table(md, NULL);
 
 	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
-
 	r = __dev_status(hc->md, param);
-	up_write(&dm_sem);
-
-	if (oldmap)
-		dm_table_put(oldmap);
 
 	return r;
 }

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]