Commit 436d4108 authored by Alasdair G Kergon's avatar Alasdair G Kergon Committed by Linus Torvalds

[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: default avatarAlasdair G Kergon <agk@redhat.com>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent a044d016
...@@ -72,7 +72,7 @@ struct multipath { ...@@ -72,7 +72,7 @@ struct multipath {
unsigned queue_io; /* Must we queue all I/O? */ unsigned queue_io; /* Must we queue all I/O? */
unsigned queue_if_no_path; /* Queue I/O if last path fails? */ 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 work_struct process_queued_ios;
struct bio_list 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, ...@@ -304,7 +304,7 @@ static int map_io(struct multipath *m, struct bio *bio, struct mpath_io *mpio,
m->queue_size--; m->queue_size--;
if ((pgpath && m->queue_io) || 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 */ /* Queue for the daemon to resubmit */
bio_list_add(&m->queued_ios, bio); bio_list_add(&m->queued_ios, bio);
m->queue_size++; m->queue_size++;
...@@ -333,6 +333,7 @@ static int queue_if_no_path(struct multipath *m, unsigned queue_if_no_path) ...@@ -333,6 +333,7 @@ static int queue_if_no_path(struct multipath *m, unsigned queue_if_no_path)
spin_lock_irqsave(&m->lock, flags); 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; m->queue_if_no_path = queue_if_no_path;
if (!m->queue_if_no_path) if (!m->queue_if_no_path)
queue_work(kmultipathd, &m->process_queued_ios); queue_work(kmultipathd, &m->process_queued_ios);
...@@ -391,7 +392,7 @@ static void process_queued_ios(void *data) ...@@ -391,7 +392,7 @@ static void process_queued_ios(void *data)
pgpath = m->current_pgpath; pgpath = m->current_pgpath;
if ((pgpath && m->queue_io) || if ((pgpath && m->queue_io) ||
(!pgpath && m->queue_if_no_path && !m->suspended)) (!pgpath && m->queue_if_no_path))
must_queue = 1; must_queue = 1;
init_required = m->pg_init_required; init_required = m->pg_init_required;
...@@ -998,7 +999,7 @@ static int do_end_io(struct multipath *m, struct bio *bio, ...@@ -998,7 +999,7 @@ static int do_end_io(struct multipath *m, struct bio *bio,
spin_lock(&m->lock); spin_lock(&m->lock);
if (!m->nr_valid_paths) { if (!m->nr_valid_paths) {
if (!m->queue_if_no_path || m->suspended) { if (!m->queue_if_no_path) {
spin_unlock(&m->lock); spin_unlock(&m->lock);
return -EIO; return -EIO;
} else { } else {
...@@ -1059,27 +1060,27 @@ static int multipath_end_io(struct dm_target *ti, struct bio *bio, ...@@ -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 * 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) static void multipath_presuspend(struct dm_target *ti)
{ {
struct multipath *m = (struct multipath *) ti->private; struct multipath *m = (struct multipath *) ti->private;
unsigned long flags;
spin_lock_irqsave(&m->lock, flags); queue_if_no_path(m, 0);
m->suspended = 1;
if (m->queue_if_no_path)
queue_work(kmultipathd, &m->process_queued_ios);
spin_unlock_irqrestore(&m->lock, flags);
} }
/*
* Restore the queue_if_no_path setting.
*/
static void multipath_resume(struct dm_target *ti) static void multipath_resume(struct dm_target *ti)
{ {
struct multipath *m = (struct multipath *) ti->private; struct multipath *m = (struct multipath *) ti->private;
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&m->lock, 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); spin_unlock_irqrestore(&m->lock, flags);
} }
......
...@@ -1055,14 +1055,17 @@ int dm_suspend(struct mapped_device *md) ...@@ -1055,14 +1055,17 @@ int dm_suspend(struct mapped_device *md)
if (test_bit(DMF_BLOCK_IO, &md->flags)) if (test_bit(DMF_BLOCK_IO, &md->flags))
goto out_read_unlock; goto out_read_unlock;
error = __lock_fs(md);
if (error)
goto out_read_unlock;
map = dm_get_table(md); map = dm_get_table(md);
if (map) if (map)
/* This does not get reverted if there's an error later. */
dm_table_presuspend_targets(map); dm_table_presuspend_targets(map);
error = __lock_fs(md);
if (error) {
dm_table_put(map);
goto out_read_unlock;
}
up_read(&md->lock); up_read(&md->lock);
/* /*
...@@ -1121,7 +1124,6 @@ int dm_suspend(struct mapped_device *md) ...@@ -1121,7 +1124,6 @@ int dm_suspend(struct mapped_device *md)
return 0; return 0;
out_unfreeze: out_unfreeze:
/* FIXME Undo dm_table_presuspend_targets */
__unlock_fs(md); __unlock_fs(md);
clear_bit(DMF_BLOCK_IO, &md->flags); clear_bit(DMF_BLOCK_IO, &md->flags);
out_write_unlock: out_write_unlock:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment