From 83c0d5d5388a8d45f7a45e0ec34adc52a78c81ad Mon Sep 17 00:00:00 2001 From: "Moger, Babu" Date: Sat, 6 Mar 2010 02:29:45 +0000 Subject: dm mpath: pass struct pgpath to pg init done This patch removes some unnecessary argument casting. There is no functional change with this patch. Passes 'struct pgpath' through to pg_init_done() instead of the enclosed 'struct dm_path'. Tested the changes with LSI storage.. CC: Chandra Seetharaman Signed-off-by: Babu Moger Acked-by: Kiyoshi Ueda Signed-off-by: Alasdair G Kergon --- drivers/md/dm-mpath.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index e81345a1d08..2c6bf74ad5c 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -1128,8 +1128,7 @@ static int pg_init_limit_reached(struct multipath *m, struct pgpath *pgpath) static void pg_init_done(void *data, int errors) { - struct dm_path *path = data; - struct pgpath *pgpath = path_to_pgpath(path); + struct pgpath *pgpath = data; struct priority_group *pg = pgpath->pg; struct multipath *m = pg->m; unsigned long flags; @@ -1198,7 +1197,7 @@ static void activate_path(struct work_struct *work) container_of(work, struct pgpath, activate_path); scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev), - pg_init_done, &pgpath->path); + pg_init_done, pgpath); } /* -- cgit v1.2.3-70-g09d2 From f7b934c8127deebf4eb56fbe4a4ae0da16b6dbcd Mon Sep 17 00:00:00 2001 From: "Moger, Babu" Date: Sat, 6 Mar 2010 02:29:49 +0000 Subject: dm mpath: skip activate_path for failed paths This patch adds two minor fixes while processing device mapper path activation. Skip failed paths while calling activate_path. If the path is already failed then activate_path will fail for sure. We don't have to call in that case. In some case this might cause prolonged retries unnecessarily. Change the misleading message if the path being activated fails with SCSI_DH_NOSYS. Signed-off-by: Babu Moger Signed-off-by: Alasdair G Kergon --- drivers/md/dm-mpath.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 2c6bf74ad5c..ecada419809 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -461,6 +461,9 @@ static void process_queued_ios(struct work_struct *work) m->pg_init_count++; m->pg_init_required = 0; list_for_each_entry(tmp, &pgpath->pg->pgpaths, list) { + /* Skip failed paths */ + if (!tmp->is_active) + continue; if (queue_work(kmpath_handlerd, &tmp->activate_path)) m->pg_init_in_progress++; } @@ -1142,8 +1145,8 @@ static void pg_init_done(void *data, int errors) errors = 0; break; } - DMERR("Cannot failover device because scsi_dh_%s was not " - "loaded.", m->hw_handler_name); + DMERR("Could not failover the device: Handler scsi_dh_%s " + "Error %d.", m->hw_handler_name, errors); /* * Fail path for now, so we do not ping pong */ -- cgit v1.2.3-70-g09d2 From fce323dd68e13354071538c765b062859e6f8286 Mon Sep 17 00:00:00 2001 From: Kiyoshi Ueda Date: Sat, 6 Mar 2010 02:29:59 +0000 Subject: dm mpath: avoid storing private suspended state 'suspended' flag in struct multipath was introduced to check whether the multipath target is in suspended state, but the same check is done through dm_suspended() now, so remove the flag and related code. Signed-off-by: Kiyoshi Ueda Signed-off-by: Jun'ichi Nomura Cc: Mike Anderson Signed-off-by: Alasdair G Kergon --- drivers/md/dm-mpath.c | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index ecada419809..8fa0c955eeb 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -95,8 +95,6 @@ struct multipath { mempool_t *mpio_pool; struct mutex work_mutex; - - unsigned suspended; /* Don't create new I/O internally when set. */ }; /* @@ -1278,7 +1276,6 @@ static void multipath_postsuspend(struct dm_target *ti) struct multipath *m = ti->private; mutex_lock(&m->work_mutex); - m->suspended = 1; flush_multipath_work(); mutex_unlock(&m->work_mutex); } @@ -1291,10 +1288,6 @@ static void multipath_resume(struct dm_target *ti) struct multipath *m = (struct multipath *) ti->private; unsigned long flags; - mutex_lock(&m->work_mutex); - m->suspended = 0; - mutex_unlock(&m->work_mutex); - spin_lock_irqsave(&m->lock, flags); m->queue_if_no_path = m->saved_queue_if_no_path; spin_unlock_irqrestore(&m->lock, flags); @@ -1430,11 +1423,6 @@ static int multipath_message(struct dm_target *ti, unsigned argc, char **argv) mutex_lock(&m->work_mutex); - if (m->suspended) { - r = -EBUSY; - goto out; - } - if (dm_suspended(ti)) { r = -EBUSY; goto out; -- cgit v1.2.3-70-g09d2 From d0259bf0eefc503d3c9c9ccda35033c3dd3aac30 Mon Sep 17 00:00:00 2001 From: Kiyoshi Ueda Date: Sat, 6 Mar 2010 02:30:02 +0000 Subject: dm mpath: hold io until all pg_inits completed m->queue_io is set to block processing I/Os, and it needs to be kept while pg-init, which issues multiple path activations, is in progress. But m->queue is cleared when a path activation completes without error in pg_init_done(), even while other path activations are in progress. That may cause undesired -EIO on paths which are not complete activation. This patch fixes that by not clearing m->queue_io until all path activations complete. (Before the hardware handlers were moved into the SCSI layer, pg_init only used one path.) Signed-off-by: Kiyoshi Ueda Signed-off-by: Jun'ichi Nomura Signed-off-by: Alasdair G Kergon --- drivers/md/dm-mpath.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 8fa0c955eeb..ae187e5d7d5 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -1181,14 +1181,19 @@ static void pg_init_done(void *data, int errors) m->current_pgpath = NULL; m->current_pg = NULL; } - } else if (!m->pg_init_required) { - m->queue_io = 0; + } else if (!m->pg_init_required) pg->bypassed = 0; - } - m->pg_init_in_progress--; - if (!m->pg_init_in_progress) - queue_work(kmultipathd, &m->process_queued_ios); + if (--m->pg_init_in_progress) + /* Activations of other paths are still on going */ + goto out; + + if (!m->pg_init_required) + m->queue_io = 0; + + queue_work(kmultipathd, &m->process_queued_ios); + +out: spin_unlock_irqrestore(&m->lock, flags); } -- cgit v1.2.3-70-g09d2 From 2bded7bd7e8b12a913b0b58167a48220560e1514 Mon Sep 17 00:00:00 2001 From: Kiyoshi Ueda Date: Sat, 6 Mar 2010 02:32:13 +0000 Subject: dm mpath: wait for pg_init completion when suspending When suspending the device we must wait for all I/O to complete, but pg-init may be still in progress even after flushing the workqueue for kmpath_handlerd in multipath_postsuspend. This patch waits for pg-init completion correctly in multipath_postsuspend(). Signed-off-by: Kiyoshi Ueda Signed-off-by: Jun'ichi Nomura Signed-off-by: Alasdair G Kergon --- drivers/md/dm-mpath.c | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index ae187e5d7d5..030bc2a053e 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -69,6 +69,7 @@ struct multipath { struct list_head priority_groups; unsigned pg_init_required; /* pg_init needs calling? */ unsigned pg_init_in_progress; /* Only one pg_init allowed at once */ + wait_queue_head_t pg_init_wait; /* Wait for pg_init completion */ unsigned nr_valid_paths; /* Total number of usable paths */ struct pgpath *current_pgpath; @@ -200,6 +201,7 @@ static struct multipath *alloc_multipath(struct dm_target *ti) m->queue_io = 1; INIT_WORK(&m->process_queued_ios, process_queued_ios); INIT_WORK(&m->trigger_event, trigger_event); + init_waitqueue_head(&m->pg_init_wait); mutex_init(&m->work_mutex); m->mpio_pool = mempool_create_slab_pool(MIN_IOS, _mpio_cache); if (!m->mpio_pool) { @@ -891,9 +893,34 @@ static int multipath_ctr(struct dm_target *ti, unsigned int argc, return r; } -static void flush_multipath_work(void) +static void multipath_wait_for_pg_init_completion(struct multipath *m) +{ + DECLARE_WAITQUEUE(wait, current); + unsigned long flags; + + add_wait_queue(&m->pg_init_wait, &wait); + + while (1) { + set_current_state(TASK_UNINTERRUPTIBLE); + + spin_lock_irqsave(&m->lock, flags); + if (!m->pg_init_in_progress) { + spin_unlock_irqrestore(&m->lock, flags); + break; + } + spin_unlock_irqrestore(&m->lock, flags); + + io_schedule(); + } + set_current_state(TASK_RUNNING); + + remove_wait_queue(&m->pg_init_wait, &wait); +} + +static void flush_multipath_work(struct multipath *m) { flush_workqueue(kmpath_handlerd); + multipath_wait_for_pg_init_completion(m); flush_workqueue(kmultipathd); flush_scheduled_work(); } @@ -902,7 +929,7 @@ static void multipath_dtr(struct dm_target *ti) { struct multipath *m = ti->private; - flush_multipath_work(); + flush_multipath_work(m); free_multipath(m); } @@ -1193,6 +1220,11 @@ static void pg_init_done(void *data, int errors) queue_work(kmultipathd, &m->process_queued_ios); + /* + * Wake up any thread waiting to suspend. + */ + wake_up(&m->pg_init_wait); + out: spin_unlock_irqrestore(&m->lock, flags); } @@ -1281,7 +1313,7 @@ static void multipath_postsuspend(struct dm_target *ti) struct multipath *m = ti->private; mutex_lock(&m->work_mutex); - flush_multipath_work(); + flush_multipath_work(m); mutex_unlock(&m->work_mutex); } -- cgit v1.2.3-70-g09d2 From fb61264297ca42a2a132f0433f75ccf7fd304ac6 Mon Sep 17 00:00:00 2001 From: Kiyoshi Ueda Date: Sat, 6 Mar 2010 02:32:18 +0000 Subject: dm mpath: refactor pg_init This patch pulls the pg_init path activation code out of process_queued_ios() into a new function. No functional change. Signed-off-by: Kiyoshi Ueda Signed-off-by: Jun'ichi Nomura Signed-off-by: Alasdair G Kergon --- drivers/md/dm-mpath.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 030bc2a053e..c1335487cc7 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -235,6 +235,21 @@ static void free_multipath(struct multipath *m) * Path selection *-----------------------------------------------*/ +static void __pg_init_all_paths(struct multipath *m) +{ + struct pgpath *pgpath; + + m->pg_init_count++; + m->pg_init_required = 0; + list_for_each_entry(pgpath, &m->current_pg->pgpaths, list) { + /* Skip failed paths */ + if (!pgpath->is_active) + continue; + if (queue_work(kmpath_handlerd, &pgpath->activate_path)) + m->pg_init_in_progress++; + } +} + static void __switch_pg(struct multipath *m, struct pgpath *pgpath) { m->current_pg = pgpath->pg; @@ -439,7 +454,7 @@ static void process_queued_ios(struct work_struct *work) { struct multipath *m = container_of(work, struct multipath, process_queued_ios); - struct pgpath *pgpath = NULL, *tmp; + struct pgpath *pgpath = NULL; unsigned must_queue = 1; unsigned long flags; @@ -457,17 +472,9 @@ static void process_queued_ios(struct work_struct *work) (!pgpath && !m->queue_if_no_path)) must_queue = 0; - if (m->pg_init_required && !m->pg_init_in_progress && pgpath) { - m->pg_init_count++; - m->pg_init_required = 0; - list_for_each_entry(tmp, &pgpath->pg->pgpaths, list) { - /* Skip failed paths */ - if (!tmp->is_active) - continue; - if (queue_work(kmpath_handlerd, &tmp->activate_path)) - m->pg_init_in_progress++; - } - } + if (m->pg_init_required && !m->pg_init_in_progress && pgpath) + __pg_init_all_paths(m); + out: spin_unlock_irqrestore(&m->lock, flags); if (!must_queue) -- cgit v1.2.3-70-g09d2 From 8215d6ec5fee1e76545decea2cd73717efb5cb42 Mon Sep 17 00:00:00 2001 From: Nikanth Karthikesan Date: Sat, 6 Mar 2010 02:32:27 +0000 Subject: dm table: remove unused dm_get_device range parameters Remove unused parameters(start and len) of dm_get_device() and fix the callers. Signed-off-by: Nikanth Karthikesan Signed-off-by: Alasdair G Kergon --- drivers/md/dm-crypt.c | 3 +-- drivers/md/dm-delay.c | 8 ++++---- drivers/md/dm-linear.c | 3 +-- drivers/md/dm-log.c | 3 +-- drivers/md/dm-mpath.c | 7 +++---- drivers/md/dm-raid1.c | 3 +-- drivers/md/dm-snap.c | 8 +++----- drivers/md/dm-stripe.c | 3 +-- drivers/md/dm-table.c | 10 ++++------ include/linux/device-mapper.h | 5 ++--- 10 files changed, 21 insertions(+), 32 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index a93637223c8..3bdbb611570 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1160,8 +1160,7 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv) } cc->start = tmpll; - if (dm_get_device(ti, argv[3], cc->start, ti->len, - dm_table_get_mode(ti->table), &cc->dev)) { + if (dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &cc->dev)) { ti->error = "Device lookup failed"; goto bad_device; } diff --git a/drivers/md/dm-delay.c b/drivers/md/dm-delay.c index ebe7381f47c..852052880d7 100644 --- a/drivers/md/dm-delay.c +++ b/drivers/md/dm-delay.c @@ -156,8 +156,8 @@ static int delay_ctr(struct dm_target *ti, unsigned int argc, char **argv) goto bad; } - if (dm_get_device(ti, argv[0], dc->start_read, ti->len, - dm_table_get_mode(ti->table), &dc->dev_read)) { + if (dm_get_device(ti, argv[0], dm_table_get_mode(ti->table), + &dc->dev_read)) { ti->error = "Device lookup failed"; goto bad; } @@ -177,8 +177,8 @@ static int delay_ctr(struct dm_target *ti, unsigned int argc, char **argv) goto bad_dev_read; } - if (dm_get_device(ti, argv[3], dc->start_write, ti->len, - dm_table_get_mode(ti->table), &dc->dev_write)) { + if (dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), + &dc->dev_write)) { ti->error = "Write device lookup failed"; goto bad_dev_read; } diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c index 82f7d6e6b1e..9200dbf2391 100644 --- a/drivers/md/dm-linear.c +++ b/drivers/md/dm-linear.c @@ -47,8 +47,7 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv) } lc->start = tmp; - if (dm_get_device(ti, argv[0], lc->start, ti->len, - dm_table_get_mode(ti->table), &lc->dev)) { + if (dm_get_device(ti, argv[0], dm_table_get_mode(ti->table), &lc->dev)) { ti->error = "dm-linear: Device lookup failed"; goto bad; } diff --git a/drivers/md/dm-log.c b/drivers/md/dm-log.c index 7035582786f..5a08be0222d 100644 --- a/drivers/md/dm-log.c +++ b/drivers/md/dm-log.c @@ -543,8 +543,7 @@ static int disk_ctr(struct dm_dirty_log *log, struct dm_target *ti, return -EINVAL; } - r = dm_get_device(ti, argv[0], 0, 0 /* FIXME */, - FMODE_READ | FMODE_WRITE, &dev); + r = dm_get_device(ti, argv[0], FMODE_READ | FMODE_WRITE, &dev); if (r) return r; diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index c1335487cc7..826bce7343b 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -607,8 +607,8 @@ static struct pgpath *parse_path(struct arg_set *as, struct path_selector *ps, if (!p) return ERR_PTR(-ENOMEM); - r = dm_get_device(ti, shift(as), ti->begin, ti->len, - dm_table_get_mode(ti->table), &p->path.dev); + r = dm_get_device(ti, shift(as), dm_table_get_mode(ti->table), + &p->path.dev); if (r) { ti->error = "error getting device"; goto bad; @@ -1505,8 +1505,7 @@ static int multipath_message(struct dm_target *ti, unsigned argc, char **argv) goto out; } - r = dm_get_device(ti, argv[1], ti->begin, ti->len, - dm_table_get_mode(ti->table), &dev); + r = dm_get_device(ti, argv[1], dm_table_get_mode(ti->table), &dev); if (r) { DMWARN("message: error getting device %s", argv[1]); diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c index de26fde4098..6d66ddf3907 100644 --- a/drivers/md/dm-raid1.c +++ b/drivers/md/dm-raid1.c @@ -920,8 +920,7 @@ static int get_mirror(struct mirror_set *ms, struct dm_target *ti, return -EINVAL; } - if (dm_get_device(ti, argv[0], offset, ti->len, - dm_table_get_mode(ti->table), + if (dm_get_device(ti, argv[0], dm_table_get_mode(ti->table), &ms->mirror[mirror].dev)) { ti->error = "Device lookup failure"; return -ENXIO; diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index ee8eb283650..0789c22ff0d 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -1081,8 +1081,7 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv) argv++; argc--; - r = dm_get_device(ti, cow_path, 0, 0, - FMODE_READ | FMODE_WRITE, &s->cow); + r = dm_get_device(ti, cow_path, FMODE_READ | FMODE_WRITE, &s->cow); if (r) { ti->error = "Cannot get COW device"; goto bad_cow; @@ -1098,7 +1097,7 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv) argv += args_used; argc -= args_used; - r = dm_get_device(ti, origin_path, 0, ti->len, origin_mode, &s->origin); + r = dm_get_device(ti, origin_path, origin_mode, &s->origin); if (r) { ti->error = "Cannot get origin device"; goto bad_origin; @@ -2100,8 +2099,7 @@ static int origin_ctr(struct dm_target *ti, unsigned int argc, char **argv) return -EINVAL; } - r = dm_get_device(ti, argv[0], 0, ti->len, - dm_table_get_mode(ti->table), &dev); + r = dm_get_device(ti, argv[0], dm_table_get_mode(ti->table), &dev); if (r) { ti->error = "Cannot get target device"; return r; diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c index bd58703ee8f..e610725db76 100644 --- a/drivers/md/dm-stripe.c +++ b/drivers/md/dm-stripe.c @@ -80,8 +80,7 @@ static int get_stripe(struct dm_target *ti, struct stripe_c *sc, if (sscanf(argv[1], "%llu", &start) != 1) return -EINVAL; - if (dm_get_device(ti, argv[0], start, sc->stripe_width, - dm_table_get_mode(ti->table), + if (dm_get_device(ti, argv[0], dm_table_get_mode(ti->table), &sc->stripe[stripe].dev)) return -ENXIO; diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index 7d70cca585a..9924ea23032 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -429,8 +429,7 @@ static int upgrade_mode(struct dm_dev_internal *dd, fmode_t new_mode, * it's already present. */ static int __table_get_device(struct dm_table *t, struct dm_target *ti, - const char *path, sector_t start, sector_t len, - fmode_t mode, struct dm_dev **result) + const char *path, fmode_t mode, struct dm_dev **result) { int r; dev_t uninitialized_var(dev); @@ -527,11 +526,10 @@ int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev, } EXPORT_SYMBOL_GPL(dm_set_device_limits); -int dm_get_device(struct dm_target *ti, const char *path, sector_t start, - sector_t len, fmode_t mode, struct dm_dev **result) +int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode, + struct dm_dev **result) { - return __table_get_device(ti->table, ti, path, - start, len, mode, result); + return __table_get_device(ti->table, ti, path, mode, result); } diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h index d4c9c0b88ad..1381cd97b4e 100644 --- a/include/linux/device-mapper.h +++ b/include/linux/device-mapper.h @@ -118,10 +118,9 @@ struct dm_dev { /* * Constructors should call these functions to ensure destination devices * are opened/closed correctly. - * FIXME: too many arguments. */ -int dm_get_device(struct dm_target *ti, const char *path, sector_t start, - sector_t len, fmode_t mode, struct dm_dev **result); +int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode, + struct dm_dev **result); void dm_put_device(struct dm_target *ti, struct dm_dev *d); /* -- cgit v1.2.3-70-g09d2