From: Alasdair G Kergon Date: Tue, 12 Jul 2005 22:53:03 +0000 (-0700) Subject: [PATCH] device-mapper multipath: Avoid possible suspension deadlock X-Git-Tag: v2.6.13-rc3~7 X-Git-Url: http://pilppa.com/gitweb/?a=commitdiff_plain;h=436d41087d047b61f8ab0604dc74fff3240a8933;p=linux-2.6-omap-h63xx.git [PATCH] device-mapper multipath: Avoid possible suspension deadlock To avoid deadlock when suspending a multipath device after all its paths have failed, stop queueing any I/O that is about to fail *before* calling freeze_bdev instead of after. Instead of setting a multipath 'suspended' flag which would have to be reset if an error occurs during the process, save the previous queueing state and leave userspace to restore if it wishes. Signed-off-by: Alasdair G Kergon Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index fa72f015320..98da8eee2d2 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -72,7 +72,7 @@ struct multipath { unsigned queue_io; /* Must we queue all I/O? */ unsigned queue_if_no_path; /* Queue I/O if last path fails? */ - unsigned suspended; /* Has dm core suspended our I/O? */ + unsigned saved_queue_if_no_path;/* Saved state during suspension */ struct work_struct process_queued_ios; struct bio_list queued_ios; @@ -304,7 +304,7 @@ static int map_io(struct multipath *m, struct bio *bio, struct mpath_io *mpio, m->queue_size--; if ((pgpath && m->queue_io) || - (!pgpath && m->queue_if_no_path && !m->suspended)) { + (!pgpath && m->queue_if_no_path)) { /* Queue for the daemon to resubmit */ bio_list_add(&m->queued_ios, bio); m->queue_size++; @@ -333,6 +333,7 @@ static int queue_if_no_path(struct multipath *m, unsigned queue_if_no_path) spin_lock_irqsave(&m->lock, flags); + m->saved_queue_if_no_path = m->queue_if_no_path; m->queue_if_no_path = queue_if_no_path; if (!m->queue_if_no_path) queue_work(kmultipathd, &m->process_queued_ios); @@ -391,7 +392,7 @@ static void process_queued_ios(void *data) pgpath = m->current_pgpath; if ((pgpath && m->queue_io) || - (!pgpath && m->queue_if_no_path && !m->suspended)) + (!pgpath && m->queue_if_no_path)) must_queue = 1; init_required = m->pg_init_required; @@ -998,7 +999,7 @@ static int do_end_io(struct multipath *m, struct bio *bio, spin_lock(&m->lock); if (!m->nr_valid_paths) { - if (!m->queue_if_no_path || m->suspended) { + if (!m->queue_if_no_path) { spin_unlock(&m->lock); return -EIO; } else { @@ -1059,27 +1060,27 @@ static int multipath_end_io(struct dm_target *ti, struct bio *bio, /* * Suspend can't complete until all the I/O is processed so if - * the last path failed we will now error any queued I/O. + * the last path fails we must error any remaining I/O. + * Note that if the freeze_bdev fails while suspending, the + * queue_if_no_path state is lost - userspace should reset it. */ static void multipath_presuspend(struct dm_target *ti) { struct multipath *m = (struct multipath *) ti->private; - unsigned long flags; - spin_lock_irqsave(&m->lock, flags); - m->suspended = 1; - if (m->queue_if_no_path) - queue_work(kmultipathd, &m->process_queued_ios); - spin_unlock_irqrestore(&m->lock, flags); + queue_if_no_path(m, 0); } +/* + * Restore the queue_if_no_path setting. + */ static void multipath_resume(struct dm_target *ti) { struct multipath *m = (struct multipath *) ti->private; unsigned long flags; spin_lock_irqsave(&m->lock, flags); - m->suspended = 0; + m->queue_if_no_path = m->saved_queue_if_no_path; spin_unlock_irqrestore(&m->lock, flags); } diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 5d40555b42b..bb3ad79c14d 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1055,14 +1055,17 @@ int dm_suspend(struct mapped_device *md) if (test_bit(DMF_BLOCK_IO, &md->flags)) goto out_read_unlock; - error = __lock_fs(md); - if (error) - goto out_read_unlock; - map = dm_get_table(md); if (map) + /* This does not get reverted if there's an error later. */ dm_table_presuspend_targets(map); + error = __lock_fs(md); + if (error) { + dm_table_put(map); + goto out_read_unlock; + } + up_read(&md->lock); /* @@ -1121,7 +1124,6 @@ int dm_suspend(struct mapped_device *md) return 0; out_unfreeze: - /* FIXME Undo dm_table_presuspend_targets */ __unlock_fs(md); clear_bit(DMF_BLOCK_IO, &md->flags); out_write_unlock: