diff options
author | Takahiro Yasui <tyasui@redhat.com> | 2010-03-05 21:32:35 -0500 |
---|---|---|
committer | Alasdair G Kergon <agk@redhat.com> | 2010-03-05 21:32:35 -0500 |
commit | f070304094edb8d516423e79edd27c97ec2020b0 (patch) | |
tree | 0330115839e84c4b5db8e2318a932f2dee78eba4 | |
parent | 924e600d417ead9ef67043988055ba236f114718 (diff) |
dm raid1: fix deadlock when suspending failed device
To prevent deadlock, bios in the hold list should be flushed before
dm_rh_stop_recovery() is called in mirror_suspend().
The recovery can't start because there are pending bios and therefore
dm_rh_stop_recovery deadlocks.
When there are pending bios in the hold list, the recovery waits for
the completion of the bios after recovery_count is acquired.
The recovery_count is released when the recovery finished, however,
the bios in the hold list are processed after dm_rh_stop_recovery() in
mirror_presuspend(). dm_rh_stop_recovery() also acquires recovery_count,
then deadlock occurs.
Signed-off-by: Takahiro Yasui <tyasui@redhat.com>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>
Reviewed-by: Mikulas Patocka <mpatocka@redhat.com>
-rw-r--r-- | drivers/md/dm-raid1.c | 41 |
1 files changed, 23 insertions, 18 deletions
diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c index 6d66ddf39071..ddda531723dc 100644 --- a/drivers/md/dm-raid1.c +++ b/drivers/md/dm-raid1.c | |||
@@ -465,9 +465,17 @@ static void map_region(struct dm_io_region *io, struct mirror *m, | |||
465 | static void hold_bio(struct mirror_set *ms, struct bio *bio) | 465 | static void hold_bio(struct mirror_set *ms, struct bio *bio) |
466 | { | 466 | { |
467 | /* | 467 | /* |
468 | * If device is suspended, complete the bio. | 468 | * Lock is required to avoid race condition during suspend |
469 | * process. | ||
469 | */ | 470 | */ |
471 | spin_lock_irq(&ms->lock); | ||
472 | |||
470 | if (atomic_read(&ms->suspend)) { | 473 | if (atomic_read(&ms->suspend)) { |
474 | spin_unlock_irq(&ms->lock); | ||
475 | |||
476 | /* | ||
477 | * If device is suspended, complete the bio. | ||
478 | */ | ||
471 | if (dm_noflush_suspending(ms->ti)) | 479 | if (dm_noflush_suspending(ms->ti)) |
472 | bio_endio(bio, DM_ENDIO_REQUEUE); | 480 | bio_endio(bio, DM_ENDIO_REQUEUE); |
473 | else | 481 | else |
@@ -478,7 +486,6 @@ static void hold_bio(struct mirror_set *ms, struct bio *bio) | |||
478 | /* | 486 | /* |
479 | * Hold bio until the suspend is complete. | 487 | * Hold bio until the suspend is complete. |
480 | */ | 488 | */ |
481 | spin_lock_irq(&ms->lock); | ||
482 | bio_list_add(&ms->holds, bio); | 489 | bio_list_add(&ms->holds, bio); |
483 | spin_unlock_irq(&ms->lock); | 490 | spin_unlock_irq(&ms->lock); |
484 | } | 491 | } |
@@ -1261,6 +1268,20 @@ static void mirror_presuspend(struct dm_target *ti) | |||
1261 | atomic_set(&ms->suspend, 1); | 1268 | atomic_set(&ms->suspend, 1); |
1262 | 1269 | ||
1263 | /* | 1270 | /* |
1271 | * Process bios in the hold list to start recovery waiting | ||
1272 | * for bios in the hold list. After the process, no bio has | ||
1273 | * a chance to be added in the hold list because ms->suspend | ||
1274 | * is set. | ||
1275 | */ | ||
1276 | spin_lock_irq(&ms->lock); | ||
1277 | holds = ms->holds; | ||
1278 | bio_list_init(&ms->holds); | ||
1279 | spin_unlock_irq(&ms->lock); | ||
1280 | |||
1281 | while ((bio = bio_list_pop(&holds))) | ||
1282 | hold_bio(ms, bio); | ||
1283 | |||
1284 | /* | ||
1264 | * We must finish up all the work that we've | 1285 | * We must finish up all the work that we've |
1265 | * generated (i.e. recovery work). | 1286 | * generated (i.e. recovery work). |
1266 | */ | 1287 | */ |
@@ -1280,22 +1301,6 @@ static void mirror_presuspend(struct dm_target *ti) | |||
1280 | * we know that all of our I/O has been pushed. | 1301 | * we know that all of our I/O has been pushed. |
1281 | */ | 1302 | */ |
1282 | flush_workqueue(ms->kmirrord_wq); | 1303 | flush_workqueue(ms->kmirrord_wq); |
1283 | |||
1284 | /* | ||
1285 | * Now set ms->suspend is set and the workqueue flushed, no more | ||
1286 | * entries can be added to ms->hold list, so process it. | ||
1287 | * | ||
1288 | * Bios can still arrive concurrently with or after this | ||
1289 | * presuspend function, but they cannot join the hold list | ||
1290 | * because ms->suspend is set. | ||
1291 | */ | ||
1292 | spin_lock_irq(&ms->lock); | ||
1293 | holds = ms->holds; | ||
1294 | bio_list_init(&ms->holds); | ||
1295 | spin_unlock_irq(&ms->lock); | ||
1296 | |||
1297 | while ((bio = bio_list_pop(&holds))) | ||
1298 | hold_bio(ms, bio); | ||
1299 | } | 1304 | } |
1300 | 1305 | ||
1301 | static void mirror_postsuspend(struct dm_target *ti) | 1306 | static void mirror_postsuspend(struct dm_target *ti) |