There's no point to the MPATHF_RETAIN_ATTACHED_HW_HANDLER flag any more. The way setup_scsi_dh() worked, if that flag wasn't set, it would attempt to attach any passed in hardware handler. This would always fail if a different hardware handler was attached, which caused setup_scsi_dh() to rerun as if the flag was set. So the code would already retain any attached handler, because attaching a different one would always fail.
Also, the code had a bug. If attached_handler_name was NULL but there was a scsi device handler attached (because either scsi_dh_attached_handler_name failed() to allocate a name, a handler got attached after it was called) the code would loop endlessly. Instead, ignore MPATHF_RETAIN_ATTACHED_HW_HANDLER, and always free the passed in handler if *attached_handler_name is set. This simplifies the code, and avoids the endless loop bug, while keeping the functionality the same. Signed-off-by: Benjamin Marzinski <[email protected]> --- drivers/md/dm-mpath.c | 61 ++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 38 deletions(-) diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 5dd90b2cdb9b..c18358271618 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -131,7 +131,7 @@ static void queue_if_no_path_timeout_work(struct timer_list *t); #define MPATHF_QUEUE_IO 0 /* Must we queue all I/O? */ #define MPATHF_QUEUE_IF_NO_PATH 1 /* Queue I/O if last path fails? */ #define MPATHF_SAVED_QUEUE_IF_NO_PATH 2 /* Saved state during suspension */ -#define MPATHF_RETAIN_ATTACHED_HW_HANDLER 3 /* If there's already a hw_handler present, don't change it. */ +/* MPATHF_RETAIN_ATTACHED_HW_HANDLER no longer has any effect */ #define MPATHF_PG_INIT_DISABLED 4 /* pg_init is not currently allowed */ #define MPATHF_PG_INIT_REQUIRED 5 /* pg_init needs calling? */ #define MPATHF_PG_INIT_DELAY_RETRY 6 /* Delay pg_init retry? */ @@ -237,16 +237,10 @@ static struct multipath *alloc_multipath(struct dm_target *ti) static int alloc_multipath_stage2(struct dm_target *ti, struct multipath *m) { - if (m->queue_mode == DM_TYPE_NONE) { + if (m->queue_mode == DM_TYPE_NONE) m->queue_mode = DM_TYPE_REQUEST_BASED; - } else if (m->queue_mode == DM_TYPE_BIO_BASED) { + else if (m->queue_mode == DM_TYPE_BIO_BASED) INIT_WORK(&m->process_queued_bios, process_queued_bios); - /* - * bio-based doesn't support any direct scsi_dh management; - * it just discovers if a scsi_dh is attached. - */ - set_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, &m->flags); - } dm_table_set_type(ti->table, m->queue_mode); @@ -887,36 +881,30 @@ static int setup_scsi_dh(struct block_device *bdev, struct multipath *m, struct request_queue *q = bdev_get_queue(bdev); int r; - if (mpath_double_check_test_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, m)) { -retain: - if (*attached_handler_name) { - /* - * Clear any hw_handler_params associated with a - * handler that isn't already attached. - */ - if (m->hw_handler_name && strcmp(*attached_handler_name, m->hw_handler_name)) { - kfree(m->hw_handler_params); - m->hw_handler_params = NULL; - } - - /* - * Reset hw_handler_name to match the attached handler - * - * NB. This modifies the table line to show the actual - * handler instead of the original table passed in. - */ - kfree(m->hw_handler_name); - m->hw_handler_name = *attached_handler_name; - *attached_handler_name = NULL; + if (*attached_handler_name) { + /* + * Clear any hw_handler_params associated with a + * handler that isn't already attached. + */ + if (m->hw_handler_name && strcmp(*attached_handler_name, + m->hw_handler_name)) { + kfree(m->hw_handler_params); + m->hw_handler_params = NULL; } + + /* + * Reset hw_handler_name to match the attached handler + * + * NB. This modifies the table line to show the actual + * handler instead of the original table passed in. + */ + kfree(m->hw_handler_name); + m->hw_handler_name = *attached_handler_name; + *attached_handler_name = NULL; } if (m->hw_handler_name) { r = scsi_dh_attach(q, m->hw_handler_name); - if (r == -EBUSY) { - DMINFO("retaining handler on device %pg", bdev); - goto retain; - } if (r < 0) { *error = "error attaching hardware handler"; return r; @@ -1138,7 +1126,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m) } if (!strcasecmp(arg_name, "retain_attached_hw_handler")) { - set_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, &m->flags); + /* no longer has any effect */ continue; } @@ -1823,7 +1811,6 @@ static void multipath_status(struct dm_target *ti, status_type_t type, DMEMIT("%u ", test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags) + (m->pg_init_retries > 0) * 2 + (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 + - test_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, &m->flags) + (m->queue_mode != DM_TYPE_REQUEST_BASED) * 2); if (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) @@ -1832,8 +1819,6 @@ static void multipath_status(struct dm_target *ti, status_type_t type, DMEMIT("pg_init_retries %u ", m->pg_init_retries); if (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs); - if (test_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, &m->flags)) - DMEMIT("retain_attached_hw_handler "); if (m->queue_mode != DM_TYPE_REQUEST_BASED) { switch (m->queue_mode) { case DM_TYPE_BIO_BASED: -- 2.50.1
