dm table: remove dm_get from dm_table_get_md
Kiyoshi Ueda [Sat, 6 Mar 2010 02:29:52 +0000 (02:29 +0000)]
Remove the dm_get() in dm_table_get_md() because dm_table_get_md() could
be called from presuspend/postsuspend, which are called while
mapped_device is in DMF_FREEING state, where dm_get() is not allowed.

Justification for that is the lifetime of both objects: As far as the
current dm design/implementation, mapped_device is never freed while
targets are doing something, because dm core waits for targets to become
quiet in dm_put() using presuspend/postsuspend.  So targets should be
able to touch mapped_device without holding reference count of the
mapped_device, and we should allow targets to touch mapped_device even
if it is in DMF_FREEING state.

Backgrounds:
I'm trying to remove the multipath internal queue, since dm core now has
a generic queue for request-based dm.  In the patch-set, the multipath
target wants to request dm core to start/stop queue.  One of such
start/stop requests can happen during postsuspend() while the target
waits for pg-init to complete, because the target stops queue when
starting pg-init and tries to restart it when completing pg-init.  Since
queue belongs to mapped_device, it involves calling dm_table_get_md()
and dm_put().  On the other hand, postsuspend() is called in dm_put()
for mapped_device which is in DMF_FREEING state, and that triggers
BUG_ON(DMF_FREEING) in the 2nd dm_put().

I had tried to solve this problem by changing only multipath not to
touch mapped_device which is in DMF_FREEING state, but I couldn't and I
came up with a question why we need dm_get() in dm_table_get_md().

Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>

drivers/md/dm-table.c
drivers/md/dm-uevent.c
drivers/md/dm.c

index 4b22feb..7d70cca 100644 (file)
@@ -1231,8 +1231,6 @@ void dm_table_unplug_all(struct dm_table *t)
 
 struct mapped_device *dm_table_get_md(struct dm_table *t)
 {
-       dm_get(t->md);
-
        return t->md;
 }
 
index c7c555a..6b1e3b6 100644 (file)
@@ -187,7 +187,7 @@ void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti,
 
        if (event_type >= ARRAY_SIZE(_dm_uevent_type_names)) {
                DMERR("%s: Invalid event_type %d", __func__, event_type);
-               goto out;
+               return;
        }
 
        event = dm_build_path_uevent(md, ti,
@@ -195,12 +195,9 @@ void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti,
                                     _dm_uevent_type_names[event_type].name,
                                     path, nr_valid_paths);
        if (IS_ERR(event))
-               goto out;
+               return;
 
        dm_uevent_add(md, &event->elist);
-
-out:
-       dm_put(md);
 }
 EXPORT_SYMBOL_GPL(dm_path_uevent);
 
index aa4e2aa..21222f5 100644 (file)
@@ -2699,23 +2699,13 @@ int dm_suspended_md(struct mapped_device *md)
 
 int dm_suspended(struct dm_target *ti)
 {
-       struct mapped_device *md = dm_table_get_md(ti->table);
-       int r = dm_suspended_md(md);
-
-       dm_put(md);
-
-       return r;
+       return dm_suspended_md(dm_table_get_md(ti->table));
 }
 EXPORT_SYMBOL_GPL(dm_suspended);
 
 int dm_noflush_suspending(struct dm_target *ti)
 {
-       struct mapped_device *md = dm_table_get_md(ti->table);
-       int r = __noflush_suspending(md);
-
-       dm_put(md);
-
-       return r;
+       return __noflush_suspending(dm_table_get_md(ti->table));
 }
 EXPORT_SYMBOL_GPL(dm_noflush_suspending);