Commit 27f204c215a0 for kernel
commit 27f204c215a0f5d91a963a16adb10432fa4ca0e9
Author: Benjamin Marzinski <bmarzins@redhat.com>
Date: Fri Nov 21 18:49:52 2025 -0500
dm-mpath: Simplify the setup_scsi_dh code
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 <bmarzins@redhat.com>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
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: