diff options
author | Kiyoshi Ueda <k-ueda@ct.jp.nec.com> | 2010-03-05 21:29:52 -0500 |
---|---|---|
committer | Alasdair G Kergon <agk@redhat.com> | 2010-03-05 21:29:52 -0500 |
commit | ecdb2e257abc33ae6798d3ccba87bdafa40ef6b6 (patch) | |
tree | 497db6a95a9f06270506f6a75354d5df855d5a66 /drivers | |
parent | f7b934c8127deebf4eb56fbe4a4ae0da16b6dbcd (diff) |
dm table: remove dm_get from dm_table_get_md
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>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/md/dm-table.c | 2 | ||||
-rw-r--r-- | drivers/md/dm-uevent.c | 7 | ||||
-rw-r--r-- | drivers/md/dm.c | 14 |
3 files changed, 4 insertions, 19 deletions
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index 4b22feb01a0c..7d70cca585ac 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c | |||
@@ -1231,8 +1231,6 @@ void dm_table_unplug_all(struct dm_table *t) | |||
1231 | 1231 | ||
1232 | struct mapped_device *dm_table_get_md(struct dm_table *t) | 1232 | struct mapped_device *dm_table_get_md(struct dm_table *t) |
1233 | { | 1233 | { |
1234 | dm_get(t->md); | ||
1235 | |||
1236 | return t->md; | 1234 | return t->md; |
1237 | } | 1235 | } |
1238 | 1236 | ||
diff --git a/drivers/md/dm-uevent.c b/drivers/md/dm-uevent.c index c7c555a8c7b2..6b1e3b61b25e 100644 --- a/drivers/md/dm-uevent.c +++ b/drivers/md/dm-uevent.c | |||
@@ -187,7 +187,7 @@ void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti, | |||
187 | 187 | ||
188 | if (event_type >= ARRAY_SIZE(_dm_uevent_type_names)) { | 188 | if (event_type >= ARRAY_SIZE(_dm_uevent_type_names)) { |
189 | DMERR("%s: Invalid event_type %d", __func__, event_type); | 189 | DMERR("%s: Invalid event_type %d", __func__, event_type); |
190 | goto out; | 190 | return; |
191 | } | 191 | } |
192 | 192 | ||
193 | event = dm_build_path_uevent(md, ti, | 193 | 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, | |||
195 | _dm_uevent_type_names[event_type].name, | 195 | _dm_uevent_type_names[event_type].name, |
196 | path, nr_valid_paths); | 196 | path, nr_valid_paths); |
197 | if (IS_ERR(event)) | 197 | if (IS_ERR(event)) |
198 | goto out; | 198 | return; |
199 | 199 | ||
200 | dm_uevent_add(md, &event->elist); | 200 | dm_uevent_add(md, &event->elist); |
201 | |||
202 | out: | ||
203 | dm_put(md); | ||
204 | } | 201 | } |
205 | EXPORT_SYMBOL_GPL(dm_path_uevent); | 202 | EXPORT_SYMBOL_GPL(dm_path_uevent); |
206 | 203 | ||
diff --git a/drivers/md/dm.c b/drivers/md/dm.c index aa4e2aa86d49..21222f5193fb 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c | |||
@@ -2699,23 +2699,13 @@ int dm_suspended_md(struct mapped_device *md) | |||
2699 | 2699 | ||
2700 | int dm_suspended(struct dm_target *ti) | 2700 | int dm_suspended(struct dm_target *ti) |
2701 | { | 2701 | { |
2702 | struct mapped_device *md = dm_table_get_md(ti->table); | 2702 | return dm_suspended_md(dm_table_get_md(ti->table)); |
2703 | int r = dm_suspended_md(md); | ||
2704 | |||
2705 | dm_put(md); | ||
2706 | |||
2707 | return r; | ||
2708 | } | 2703 | } |
2709 | EXPORT_SYMBOL_GPL(dm_suspended); | 2704 | EXPORT_SYMBOL_GPL(dm_suspended); |
2710 | 2705 | ||
2711 | int dm_noflush_suspending(struct dm_target *ti) | 2706 | int dm_noflush_suspending(struct dm_target *ti) |
2712 | { | 2707 | { |
2713 | struct mapped_device *md = dm_table_get_md(ti->table); | 2708 | return __noflush_suspending(dm_table_get_md(ti->table)); |
2714 | int r = __noflush_suspending(md); | ||
2715 | |||
2716 | dm_put(md); | ||
2717 | |||
2718 | return r; | ||
2719 | } | 2709 | } |
2720 | EXPORT_SYMBOL_GPL(dm_noflush_suspending); | 2710 | EXPORT_SYMBOL_GPL(dm_noflush_suspending); |
2721 | 2711 | ||