From cfae5c9bb66325cd32d5f2ee41f14749f062a53c Mon Sep 17 00:00:00 2001 From: Chandra Seetharaman Date: Thu, 1 May 2008 14:50:11 -0700 Subject: [SCSI] scsi_dh: Use SCSI device handler in dm-multipath This patch converts dm-mpath to use scsi device handlers instead of dm's hardware handlers. This patch does not add any new functionality. Old behaviors remain and userspace tools work as is except that arguments supplied with hardware handler are ignored. One behavioral exception is: Activation of a path is synchronous in this patch, opposed to the older behavior of being asynchronous (changed in patch 07: scsi_dh: Add a single threaded workqueue for initializing a path) Note: There is no need to get a reference for the device handler module (as it was done in the dm hardware handler case) here as the reference is held when the device was first found. Instead we check and make sure that support for the specified device is present at table load time. Signed-off-by: Chandra Seetharaman Signed-off-by: Mike Christie Acked-by: Alasdair G Kergon Signed-off-by: James Bottomley --- drivers/md/dm-mpath.c | 131 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 80 insertions(+), 51 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index e7ee59e655d5..e54ff372d711 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #define DM_MSG_PREFIX "multipath" @@ -61,7 +62,7 @@ struct multipath { spinlock_t lock; - struct hw_handler hw_handler; + const char *hw_handler_name; unsigned nr_priority_groups; struct list_head priority_groups; unsigned pg_init_required; /* pg_init needs calling? */ @@ -109,6 +110,7 @@ static struct kmem_cache *_mpio_cache; static struct workqueue_struct *kmultipathd; static void process_queued_ios(struct work_struct *work); static void trigger_event(struct work_struct *work); +static void pg_init_done(struct dm_path *, int); /*----------------------------------------------- @@ -193,18 +195,13 @@ static struct multipath *alloc_multipath(struct dm_target *ti) static void free_multipath(struct multipath *m) { struct priority_group *pg, *tmp; - struct hw_handler *hwh = &m->hw_handler; list_for_each_entry_safe(pg, tmp, &m->priority_groups, list) { list_del(&pg->list); free_priority_group(pg, m->ti); } - if (hwh->type) { - hwh->type->destroy(hwh); - dm_put_hw_handler(hwh->type); - } - + kfree(m->hw_handler_name); mempool_destroy(m->mpio_pool); kfree(m); } @@ -216,12 +213,10 @@ static void free_multipath(struct multipath *m) static void __switch_pg(struct multipath *m, struct pgpath *pgpath) { - struct hw_handler *hwh = &m->hw_handler; - m->current_pg = pgpath->pg; /* Must we initialise the PG first, and queue I/O till it's ready? */ - if (hwh->type && hwh->type->pg_init) { + if (m->hw_handler_name) { m->pg_init_required = 1; m->queue_io = 1; } else { @@ -409,7 +404,6 @@ static void process_queued_ios(struct work_struct *work) { struct multipath *m = container_of(work, struct multipath, process_queued_ios); - struct hw_handler *hwh = &m->hw_handler; struct pgpath *pgpath = NULL; unsigned init_required = 0, must_queue = 1; unsigned long flags; @@ -438,8 +432,11 @@ static void process_queued_ios(struct work_struct *work) out: spin_unlock_irqrestore(&m->lock, flags); - if (init_required) - hwh->type->pg_init(hwh, pgpath->pg->bypassed, &pgpath->path); + if (init_required) { + struct dm_path *path = &pgpath->path; + int ret = scsi_dh_activate(bdev_get_queue(path->dev->bdev)); + pg_init_done(path, ret); + } if (!must_queue) dispatch_queued_ios(m); @@ -652,8 +649,6 @@ static struct priority_group *parse_priority_group(struct arg_set *as, static int parse_hw_handler(struct arg_set *as, struct multipath *m) { - int r; - struct hw_handler_type *hwht; unsigned hw_argc; struct dm_target *ti = m->ti; @@ -661,30 +656,18 @@ static int parse_hw_handler(struct arg_set *as, struct multipath *m) {0, 1024, "invalid number of hardware handler args"}, }; - r = read_param(_params, shift(as), &hw_argc, &ti->error); - if (r) + if (read_param(_params, shift(as), &hw_argc, &ti->error)) return -EINVAL; if (!hw_argc) return 0; - hwht = dm_get_hw_handler(shift(as)); - if (!hwht) { + m->hw_handler_name = kstrdup(shift(as), GFP_KERNEL); + request_module("scsi_dh_%s", m->hw_handler_name); + if (scsi_dh_handler_exist(m->hw_handler_name) == 0) { ti->error = "unknown hardware handler type"; return -EINVAL; } - - m->hw_handler.md = dm_table_get_md(ti->table); - dm_put(m->hw_handler.md); - - r = hwht->create(&m->hw_handler, hw_argc - 1, as->argv); - if (r) { - dm_put_hw_handler(hwht); - ti->error = "hardware handler constructor failed"; - return r; - } - - m->hw_handler.type = hwht; consume(as, hw_argc - 1); return 0; @@ -1063,14 +1046,74 @@ void dm_pg_init_complete(struct dm_path *path, unsigned err_flags) spin_unlock_irqrestore(&m->lock, flags); } +static void pg_init_done(struct dm_path *path, int errors) +{ + struct pgpath *pgpath = path_to_pgpath(path); + struct priority_group *pg = pgpath->pg; + struct multipath *m = pg->m; + unsigned long flags; + + /* device or driver problems */ + switch (errors) { + case SCSI_DH_OK: + break; + case SCSI_DH_NOSYS: + if (!m->hw_handler_name) { + errors = 0; + break; + } + DMERR("Cannot failover device because scsi_dh_%s was not " + "loaded.", m->hw_handler_name); + /* + * Fail path for now, so we do not ping pong + */ + fail_path(pgpath); + break; + case SCSI_DH_DEV_TEMP_BUSY: + /* + * Probably doing something like FW upgrade on the + * controller so try the other pg. + */ + bypass_pg(m, pg, 1); + break; + /* TODO: For SCSI_DH_RETRY we should wait a couple seconds */ + case SCSI_DH_RETRY: + case SCSI_DH_IMM_RETRY: + case SCSI_DH_RES_TEMP_UNAVAIL: + if (pg_init_limit_reached(m, pgpath)) + fail_path(pgpath); + errors = 0; + break; + default: + /* + * We probably do not want to fail the path for a device + * error, but this is what the old dm did. In future + * patches we can do more advanced handling. + */ + fail_path(pgpath); + } + + spin_lock_irqsave(&m->lock, flags); + if (errors) { + DMERR("Could not failover device. Error %d.", errors); + m->current_pgpath = NULL; + m->current_pg = NULL; + } else if (!m->pg_init_required) { + m->queue_io = 0; + pg->bypassed = 0; + } + + m->pg_init_in_progress = 0; + queue_work(kmultipathd, &m->process_queued_ios); + spin_unlock_irqrestore(&m->lock, flags); +} + /* * end_io handling */ static int do_end_io(struct multipath *m, struct bio *bio, int error, struct dm_mpath_io *mpio) { - struct hw_handler *hwh = &m->hw_handler; - unsigned err_flags = MP_FAIL_PATH; /* Default behavior */ unsigned long flags; if (!error) @@ -1097,19 +1140,8 @@ static int do_end_io(struct multipath *m, struct bio *bio, } spin_unlock_irqrestore(&m->lock, flags); - if (hwh->type && hwh->type->error) - err_flags = hwh->type->error(hwh, bio); - - if (mpio->pgpath) { - if (err_flags & MP_FAIL_PATH) - fail_path(mpio->pgpath); - - if (err_flags & MP_BYPASS_PG) - bypass_pg(m, mpio->pgpath->pg, 1); - } - - if (err_flags & MP_ERROR_IO) - return -EIO; + if (mpio->pgpath) + fail_path(mpio->pgpath); requeue: dm_bio_restore(&mpio->details, bio); @@ -1194,7 +1226,6 @@ static int multipath_status(struct dm_target *ti, status_type_t type, int sz = 0; unsigned long flags; struct multipath *m = (struct multipath *) ti->private; - struct hw_handler *hwh = &m->hw_handler; struct priority_group *pg; struct pgpath *p; unsigned pg_num; @@ -1214,12 +1245,10 @@ static int multipath_status(struct dm_target *ti, status_type_t type, DMEMIT("pg_init_retries %u ", m->pg_init_retries); } - if (hwh->type && hwh->type->status) - sz += hwh->type->status(hwh, type, result + sz, maxlen - sz); - else if (!hwh->type || type == STATUSTYPE_INFO) + if (!m->hw_handler_name || type == STATUSTYPE_INFO) DMEMIT("0 "); else - DMEMIT("1 %s ", hwh->type->name); + DMEMIT("1 %s ", m->hw_handler_name); DMEMIT("%u ", m->nr_priority_groups); -- cgit v1.2.3-70-g09d2 From bab7cfc733f4453a502b7491b9ee37b091440ec4 Mon Sep 17 00:00:00 2001 From: Chandra Seetharaman Date: Thu, 1 May 2008 14:50:22 -0700 Subject: [SCSI] scsi_dh: Add a single threaded workqueue for initializing paths Before this patch set (SCSI hardware handlers), initialization of a path was done asynchronously. Doing that requires a workqueue in each device/hardware handler module and leads to unneccessary complication in the device handler code, making it difficult to read the code and follow the state diagram. Moving that workqueue to this level makes the device handler code simpler. Hence, the workqueue is moved to dm level. A new workqueue is added instead of adding it to the existing workqueue (kmpathd) for the following reasons: 1. Device activation has to happen faster, stacking them along with the other workqueue might lead to unnecessary delay in the activation of the path. 2. The effect could be felt the other way too. i.e the current events that are handled by the existing workqueue might get a delayed response. Signed-off-by: Chandra Seetharaman Acked-by: Alasdair G Kergon Signed-off-by: James Bottomley --- drivers/md/dm-mpath.c | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index e54ff372d711..9b16788118d2 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -63,6 +63,7 @@ struct multipath { spinlock_t lock; const char *hw_handler_name; + struct work_struct activate_path; unsigned nr_priority_groups; struct list_head priority_groups; unsigned pg_init_required; /* pg_init needs calling? */ @@ -107,10 +108,10 @@ typedef int (*action_fn) (struct pgpath *pgpath); static struct kmem_cache *_mpio_cache; -static struct workqueue_struct *kmultipathd; +static struct workqueue_struct *kmultipathd, *kmpath_handlerd; static void process_queued_ios(struct work_struct *work); static void trigger_event(struct work_struct *work); -static void pg_init_done(struct dm_path *, int); +static void activate_path(struct work_struct *work); /*----------------------------------------------- @@ -180,6 +181,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_WORK(&m->activate_path, activate_path); m->mpio_pool = mempool_create_slab_pool(MIN_IOS, _mpio_cache); if (!m->mpio_pool) { kfree(m); @@ -432,11 +434,8 @@ static void process_queued_ios(struct work_struct *work) out: spin_unlock_irqrestore(&m->lock, flags); - if (init_required) { - struct dm_path *path = &pgpath->path; - int ret = scsi_dh_activate(bdev_get_queue(path->dev->bdev)); - pg_init_done(path, ret); - } + if (init_required) + queue_work(kmpath_handlerd, &m->activate_path); if (!must_queue) dispatch_queued_ios(m); @@ -791,6 +790,7 @@ static void multipath_dtr(struct dm_target *ti) { struct multipath *m = (struct multipath *) ti->private; + flush_workqueue(kmpath_handlerd); flush_workqueue(kmultipathd); free_multipath(m); } @@ -1108,6 +1108,17 @@ static void pg_init_done(struct dm_path *path, int errors) spin_unlock_irqrestore(&m->lock, flags); } +static void activate_path(struct work_struct *work) +{ + int ret; + struct multipath *m = + container_of(work, struct multipath, activate_path); + struct dm_path *path = &m->current_pgpath->path; + + ret = scsi_dh_activate(bdev_get_queue(path->dev->bdev)); + pg_init_done(path, ret); +} + /* * end_io handling */ @@ -1451,6 +1462,21 @@ static int __init dm_multipath_init(void) return -ENOMEM; } + /* + * A separate workqueue is used to handle the device handlers + * to avoid overloading existing workqueue. Overloading the + * old workqueue would also create a bottleneck in the + * path of the storage hardware device activation. + */ + kmpath_handlerd = create_singlethread_workqueue("kmpath_handlerd"); + if (!kmpath_handlerd) { + DMERR("failed to create workqueue kmpath_handlerd"); + destroy_workqueue(kmultipathd); + dm_unregister_target(&multipath_target); + kmem_cache_destroy(_mpio_cache); + return -ENOMEM; + } + DMINFO("version %u.%u.%u loaded", multipath_target.version[0], multipath_target.version[1], multipath_target.version[2]); @@ -1462,6 +1488,7 @@ static void __exit dm_multipath_exit(void) { int r; + destroy_workqueue(kmpath_handlerd); destroy_workqueue(kmultipathd); r = dm_unregister_target(&multipath_target); -- cgit v1.2.3-70-g09d2 From 2651f5d7d3bc5120a439e498f131e4d731f99b3e Mon Sep 17 00:00:00 2001 From: Chandra Seetharaman Date: Thu, 1 May 2008 14:50:28 -0700 Subject: [SCSI] scsi_dh: Remove dm_pg_init_complete This patch just removes the dm layer's path initialization completion routine. This is separated from the other patch(scsi_dh: Use SCSI device handler in dm-multipath) Just to make that patch more readable. Signed-off-by: Chandra Seetharaman Acked-by: Alasdair G Kergon Signed-off-by: James Bottomley --- drivers/md/dm-mpath.c | 41 ----------------------------------------- 1 file changed, 41 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 9b16788118d2..e8f704aa46f2 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -7,7 +7,6 @@ #include "dm.h" #include "dm-path-selector.h" -#include "dm-hw-handler.h" #include "dm-bio-list.h" #include "dm-bio-record.h" #include "dm-uevent.h" @@ -1008,44 +1007,6 @@ static int pg_init_limit_reached(struct multipath *m, struct pgpath *pgpath) return limit_reached; } -/* - * pg_init must call this when it has completed its initialisation - */ -void dm_pg_init_complete(struct dm_path *path, unsigned err_flags) -{ - struct pgpath *pgpath = path_to_pgpath(path); - struct priority_group *pg = pgpath->pg; - struct multipath *m = pg->m; - unsigned long flags; - - /* - * If requested, retry pg_init until maximum number of retries exceeded. - * If retry not requested and PG already bypassed, always fail the path. - */ - if (err_flags & MP_RETRY) { - if (pg_init_limit_reached(m, pgpath)) - err_flags |= MP_FAIL_PATH; - } else if (err_flags && pg->bypassed) - err_flags |= MP_FAIL_PATH; - - if (err_flags & MP_FAIL_PATH) - fail_path(pgpath); - - if (err_flags & MP_BYPASS_PG) - bypass_pg(m, pg, 1); - - spin_lock_irqsave(&m->lock, flags); - if (err_flags & ~MP_RETRY) { - m->current_pgpath = NULL; - m->current_pg = NULL; - } else if (!m->pg_init_required) - m->queue_io = 0; - - m->pg_init_in_progress = 0; - queue_work(kmultipathd, &m->process_queued_ios); - spin_unlock_irqrestore(&m->lock, flags); -} - static void pg_init_done(struct dm_path *path, int errors) { struct pgpath *pgpath = path_to_pgpath(path); @@ -1497,8 +1458,6 @@ static void __exit dm_multipath_exit(void) kmem_cache_destroy(_mpio_cache); } -EXPORT_SYMBOL_GPL(dm_pg_init_complete); - module_init(dm_multipath_init); module_exit(dm_multipath_exit); -- cgit v1.2.3-70-g09d2 From fe9233fb6914a0eb20166c967e3020f7f0fba2c9 Mon Sep 17 00:00:00 2001 From: Chandra Seetharaman Date: Fri, 23 May 2008 18:16:40 -0700 Subject: [SCSI] scsi_dh: fix kconfig related build errors Do not automatically "select" SCSI_DH for dm-multipath. If SCSI_DH doesn't exist,just do not allow hardware handlers to be used. Handle SCSI_DH being a module also. Make sure it doesn't allow DM_MULTIPATH to be compiled in when SCSI_DH is a module. [jejb: added comment for Kconfig syntax] Signed-off-by: Chandra Seetharaman Reported-by: Randy Dunlap Reported-by: Andrew Morton Signed-off-by: James Bottomley --- drivers/md/Kconfig | 6 +++++- drivers/md/dm-mpath.c | 2 ++ include/scsi/scsi_dh.h | 12 +++++++++++- 3 files changed, 18 insertions(+), 2 deletions(-) (limited to 'drivers/md/dm-mpath.c') diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig index b4a3c7d1451d..07d92c11b5d8 100644 --- a/drivers/md/Kconfig +++ b/drivers/md/Kconfig @@ -252,7 +252,11 @@ config DM_ZERO config DM_MULTIPATH tristate "Multipath target" depends on BLK_DEV_DM - select SCSI_DH + # nasty syntax but means make DM_MULTIPATH independent + # of SCSI_DH if the latter isn't defined but if + # it is, DM_MULTIPATH must depend on it. We get a build + # error if SCSI_DH=m and DM_MULTIPATH=y + depends on SCSI_DH || !SCSI_DH ---help--- Allow volume managers to support multipath hardware. diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index e8f704aa46f2..9f7302d4878d 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -664,6 +664,8 @@ static int parse_hw_handler(struct arg_set *as, struct multipath *m) request_module("scsi_dh_%s", m->hw_handler_name); if (scsi_dh_handler_exist(m->hw_handler_name) == 0) { ti->error = "unknown hardware handler type"; + kfree(m->hw_handler_name); + m->hw_handler_name = NULL; return -EINVAL; } consume(as, hw_argc - 1); diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h index 04d0d8495c83..3ad2303d1a16 100644 --- a/include/scsi/scsi_dh.h +++ b/include/scsi/scsi_dh.h @@ -54,6 +54,16 @@ enum { SCSI_DH_NOSYS, SCSI_DH_DRIVER_MAX, }; - +#if defined(CONFIG_SCSI_DH) || defined(CONFIG_SCSI_DH_MODULE) extern int scsi_dh_activate(struct request_queue *); extern int scsi_dh_handler_exist(const char *); +#else +static inline int scsi_dh_activate(struct request_queue *req) +{ + return 0; +} +static inline int scsi_dh_handler_exist(const char *name) +{ + return 0; +} +#endif -- cgit v1.2.3-70-g09d2