dm: Remove dm_get() from dm_table_get_md()
RFC PATCH 1/1 dm: Remove dm_get() from dm_table_get_md()
Hi Alasdair and all,
I'd like to remove 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.
Please give me your opinion if you object this change.
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().
--- drivers/md/dm-table.c | 2 -- drivers/md/dm-uevent.c | 7 ++----- drivers/md/dm.c | 14 ++------------ 3 files changed, 4 insertions(+), 19 deletions(-)
drivers/md/dm-table.c +0 -2
@@ -1241,8 +1241,6 @@ void dm_table_unplug_all(struct dm_tablestruct mapped_device *dm_table_get_md (struct dm_table *t) { -dm_get (t->md); -return t->md; }
drivers/md/dm-uevent.c +2 -5
@@ -187,7 +187,7 @@ void dm_path_uevent(enum dm_uevent_typeif (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 _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);
drivers/md/dm.c +2 -12
@@ -2686,23 +2686,13 @@ int dm_suspended_md(struct mapped_deviceint 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);
Alasdair G Kergon
maintainerQuoted 4 lines · Kiyoshi Ueda
I'd like to remove
dm_get()indm_table_get_md()becausedm_table_get_md()could be called from presuspend/postsuspend, which are called whilemapped_deviceis inDMF_FREEINGstate, wheredm_get()is not allowed.
I haven't spotted any problems with this, so I'm queueing it to linux-next.
Alasdair