diff options
author | Shiva Krishna Merla <shivakrishna.merla@netapp.com> | 2013-10-29 23:26:38 -0400 |
---|---|---|
committer | Mike Snitzer <snitzer@redhat.com> | 2013-10-31 21:39:47 -0400 |
commit | 954a73d5d3073df2231820c718fdd2f18b0fe4c9 (patch) | |
tree | 2e095be465d8c9418e02ae4d8a185090a2a57da4 /drivers/md/dm-mpath.c | |
parent | f36afb3957353d2529cb2b00f78fdccd14fc5e9c (diff) |
dm mpath: fix race condition between multipath_dtr and pg_init_done
Whenever multipath_dtr() is happening we must prevent queueing any
further path activation work. Implement this by adding a new
'pg_init_disabled' flag to the multipath structure that denotes future
path activation work should be skipped if it is set. By disabling
pg_init and then re-enabling in flush_multipath_work() we also avoid the
potential for pg_init to be initiated while suspending an mpath device.
Without this patch a race condition exists that may result in a kernel
panic:
1) If after pg_init_done() decrements pg_init_in_progress to 0, a call
to wait_for_pg_init_completion() assumes there are no more pending path
management commands.
2) If pg_init_required is set by pg_init_done(), due to retryable
mode_select errors, then process_queued_ios() will again queue the
path activation work.
3) If free_multipath() completes before activate_path() work is called a
NULL pointer dereference like the following can be seen when
accessing members of the recently destructed multipath:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000090
RIP: 0010:[<ffffffffa003db1b>] [<ffffffffa003db1b>] activate_path+0x1b/0x30 [dm_multipath]
[<ffffffff81090ac0>] worker_thread+0x170/0x2a0
[<ffffffff81096c80>] ? autoremove_wake_function+0x0/0x40
[switch to disabling pg_init in flush_multipath_work & header edits by Mike Snitzer]
Signed-off-by: Shiva Krishna Merla <shivakrishna.merla@netapp.com>
Reviewed-by: Krishnasamy Somasundaram <somasundaram.krishnasamy@netapp.com>
Tested-by: Speagle Andy <Andy.Speagle@netapp.com>
Acked-by: Junichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: stable@vger.kernel.org
Diffstat (limited to 'drivers/md/dm-mpath.c')
-rw-r--r-- | drivers/md/dm-mpath.c | 18 |
1 files changed, 15 insertions, 3 deletions
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index de570a558764..799e479db93b 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c | |||
@@ -87,6 +87,7 @@ struct multipath { | |||
87 | unsigned queue_if_no_path:1; /* Queue I/O if last path fails? */ | 87 | unsigned queue_if_no_path:1; /* Queue I/O if last path fails? */ |
88 | unsigned saved_queue_if_no_path:1; /* Saved state during suspension */ | 88 | unsigned saved_queue_if_no_path:1; /* Saved state during suspension */ |
89 | unsigned retain_attached_hw_handler:1; /* If there's already a hw_handler present, don't change it. */ | 89 | unsigned retain_attached_hw_handler:1; /* If there's already a hw_handler present, don't change it. */ |
90 | unsigned pg_init_disabled:1; /* pg_init is not currently allowed */ | ||
90 | 91 | ||
91 | unsigned pg_init_retries; /* Number of times to retry pg_init */ | 92 | unsigned pg_init_retries; /* Number of times to retry pg_init */ |
92 | unsigned pg_init_count; /* Number of times pg_init called */ | 93 | unsigned pg_init_count; /* Number of times pg_init called */ |
@@ -497,7 +498,8 @@ static void process_queued_ios(struct work_struct *work) | |||
497 | (!pgpath && !m->queue_if_no_path)) | 498 | (!pgpath && !m->queue_if_no_path)) |
498 | must_queue = 0; | 499 | must_queue = 0; |
499 | 500 | ||
500 | if (m->pg_init_required && !m->pg_init_in_progress && pgpath) | 501 | if (m->pg_init_required && !m->pg_init_in_progress && pgpath && |
502 | !m->pg_init_disabled) | ||
501 | __pg_init_all_paths(m); | 503 | __pg_init_all_paths(m); |
502 | 504 | ||
503 | spin_unlock_irqrestore(&m->lock, flags); | 505 | spin_unlock_irqrestore(&m->lock, flags); |
@@ -942,10 +944,20 @@ static void multipath_wait_for_pg_init_completion(struct multipath *m) | |||
942 | 944 | ||
943 | static void flush_multipath_work(struct multipath *m) | 945 | static void flush_multipath_work(struct multipath *m) |
944 | { | 946 | { |
947 | unsigned long flags; | ||
948 | |||
949 | spin_lock_irqsave(&m->lock, flags); | ||
950 | m->pg_init_disabled = 1; | ||
951 | spin_unlock_irqrestore(&m->lock, flags); | ||
952 | |||
945 | flush_workqueue(kmpath_handlerd); | 953 | flush_workqueue(kmpath_handlerd); |
946 | multipath_wait_for_pg_init_completion(m); | 954 | multipath_wait_for_pg_init_completion(m); |
947 | flush_workqueue(kmultipathd); | 955 | flush_workqueue(kmultipathd); |
948 | flush_work(&m->trigger_event); | 956 | flush_work(&m->trigger_event); |
957 | |||
958 | spin_lock_irqsave(&m->lock, flags); | ||
959 | m->pg_init_disabled = 0; | ||
960 | spin_unlock_irqrestore(&m->lock, flags); | ||
949 | } | 961 | } |
950 | 962 | ||
951 | static void multipath_dtr(struct dm_target *ti) | 963 | static void multipath_dtr(struct dm_target *ti) |
@@ -1164,7 +1176,7 @@ static int pg_init_limit_reached(struct multipath *m, struct pgpath *pgpath) | |||
1164 | 1176 | ||
1165 | spin_lock_irqsave(&m->lock, flags); | 1177 | spin_lock_irqsave(&m->lock, flags); |
1166 | 1178 | ||
1167 | if (m->pg_init_count <= m->pg_init_retries) | 1179 | if (m->pg_init_count <= m->pg_init_retries && !m->pg_init_disabled) |
1168 | m->pg_init_required = 1; | 1180 | m->pg_init_required = 1; |
1169 | else | 1181 | else |
1170 | limit_reached = 1; | 1182 | limit_reached = 1; |
@@ -1714,7 +1726,7 @@ out: | |||
1714 | *---------------------------------------------------------------*/ | 1726 | *---------------------------------------------------------------*/ |
1715 | static struct target_type multipath_target = { | 1727 | static struct target_type multipath_target = { |
1716 | .name = "multipath", | 1728 | .name = "multipath", |
1717 | .version = {1, 5, 1}, | 1729 | .version = {1, 6, 0}, |
1718 | .module = THIS_MODULE, | 1730 | .module = THIS_MODULE, |
1719 | .ctr = multipath_ctr, | 1731 | .ctr = multipath_ctr, |
1720 | .dtr = multipath_dtr, | 1732 | .dtr = multipath_dtr, |