From ecdb2e257abc33ae6798d3ccba87bdafa40ef6b6 Mon Sep 17 00:00:00 2001 From: Kiyoshi Ueda Date: Sat, 6 Mar 2010 02:29:52 +0000 Subject: 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 Signed-off-by: Jun'ichi Nomura Signed-off-by: Alasdair G Kergon --- drivers/md/dm.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) (limited to 'drivers/md/dm.c') diff --git a/drivers/md/dm.c b/drivers/md/dm.c index aa4e2aa86d4..21222f5193f 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -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); -- cgit v1.2.3 From a97f925a32aad2a37971d7bfb657006acf04e42d Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Sat, 6 Mar 2010 02:32:29 +0000 Subject: dm: free dm_io before bio_endio not after Free the dm_io structure before calling bio_endio() instead of after it, to ensure that the io_pool containing it is not referenced after it is freed. This partially fixes a problem described here https://www.redhat.com/archives/dm-devel/2010-February/msg00109.html thread 1: bio_endio(bio, io_error); /* scheduling happens */ thread 2: close the device remove the device thread 1: free_io(md, io); Thread 2, when removing the device, sees non-empty md->io_pool (because the io hasn't been freed by thread 1 yet) and may crash with BUG in mempool_free. Thread 1 may also crash, when freeing into a nonexisting mempool. To fix this we must make sure that bio_endio() is the last call and the md structure is not accessed afterwards. There is another bio_endio in process_barrier, but it is called from the thread and the thread is destroyed prior to freeing the mempools, so this call is not affected by the bug. A similar bug exists with module unloads - the module may be unloaded immediately after bio_endio - but that is more difficult to fix. Signed-off-by: Mikulas Patocka Cc: stable@kernel.org Signed-off-by: Alasdair G Kergon --- drivers/md/dm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/md/dm.c') diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 21222f5193f..7199846364e 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -635,8 +635,10 @@ static void dec_pending(struct dm_io *io, int error) if (!md->barrier_error && io_error != -EOPNOTSUPP) md->barrier_error = io_error; end_io_acct(io); + free_io(md, io); } else { end_io_acct(io); + free_io(md, io); if (io_error != DM_ENDIO_REQUEUE) { trace_block_bio_complete(md->queue, bio); @@ -644,8 +646,6 @@ static void dec_pending(struct dm_io *io, int error) bio_endio(bio, io_error); } } - - free_io(md, io); } } -- cgit v1.2.3 From 3abf85b5b5851b5f28d3d8a920ebb844edd08352 Mon Sep 17 00:00:00 2001 From: Peter Rajnoha Date: Sat, 6 Mar 2010 02:32:31 +0000 Subject: dm ioctl: introduce flag indicating uevent was generated Set a new DM_UEVENT_GENERATED_FLAG when returning from ioctls to indicate that a uevent was actually generated. This tells the userspace caller that it may need to wait for the event to be processed. Signed-off-by: Peter Rajnoha Signed-off-by: Alasdair G Kergon --- drivers/md/dm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/md/dm.c') diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 7199846364e..d21e1284604 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -2618,18 +2618,19 @@ out: /*----------------------------------------------------------------- * Event notification. *---------------------------------------------------------------*/ -void dm_kobject_uevent(struct mapped_device *md, enum kobject_action action, +int dm_kobject_uevent(struct mapped_device *md, enum kobject_action action, unsigned cookie) { char udev_cookie[DM_COOKIE_LENGTH]; char *envp[] = { udev_cookie, NULL }; if (!cookie) - kobject_uevent(&disk_to_dev(md->disk)->kobj, action); + return kobject_uevent(&disk_to_dev(md->disk)->kobj, action); else { snprintf(udev_cookie, DM_COOKIE_LENGTH, "%s=%u", DM_COOKIE_ENV_VAR_NAME, cookie); - kobject_uevent_env(&disk_to_dev(md->disk)->kobj, action, envp); + return kobject_uevent_env(&disk_to_dev(md->disk)->kobj, + action, envp); } } -- cgit v1.2.3