Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Kiyoshi Ueda and committed by
Alasdair G Kergon
ecdb2e25 f7b934c8

+4 -19
-2
drivers/md/dm-table.c
··· 1231 1231 1232 1232 struct mapped_device *dm_table_get_md(struct dm_table *t) 1233 1233 { 1234 - dm_get(t->md); 1235 - 1236 1234 return t->md; 1237 1235 } 1238 1236
+2 -5
drivers/md/dm-uevent.c
··· 187 187 188 188 if (event_type >= ARRAY_SIZE(_dm_uevent_type_names)) { 189 189 DMERR("%s: Invalid event_type %d", __func__, event_type); 190 - goto out; 190 + return; 191 191 } 192 192 193 193 event = dm_build_path_uevent(md, ti, ··· 195 195 _dm_uevent_type_names[event_type].name, 196 196 path, nr_valid_paths); 197 197 if (IS_ERR(event)) 198 - goto out; 198 + return; 199 199 200 200 dm_uevent_add(md, &event->elist); 201 - 202 - out: 203 - dm_put(md); 204 201 } 205 202 EXPORT_SYMBOL_GPL(dm_path_uevent); 206 203
+2 -12
drivers/md/dm.c
··· 2699 2699 2700 2700 int dm_suspended(struct dm_target *ti) 2701 2701 { 2702 - struct mapped_device *md = dm_table_get_md(ti->table); 2703 - int r = dm_suspended_md(md); 2704 - 2705 - dm_put(md); 2706 - 2707 - return r; 2702 + return dm_suspended_md(dm_table_get_md(ti->table)); 2708 2703 } 2709 2704 EXPORT_SYMBOL_GPL(dm_suspended); 2710 2705 2711 2706 int dm_noflush_suspending(struct dm_target *ti) 2712 2707 { 2713 - struct mapped_device *md = dm_table_get_md(ti->table); 2714 - int r = __noflush_suspending(md); 2715 - 2716 - dm_put(md); 2717 - 2718 - return r; 2708 + return __noflush_suspending(dm_table_get_md(ti->table)); 2719 2709 } 2720 2710 EXPORT_SYMBOL_GPL(dm_noflush_suspending); 2721 2711