[PATCH v3 00/13] libmultipath: a generic multipath lib for block drivers

[PATCH v3 00/13] libmultipath: a generic multipath lib for block drivers

[John Garry]

This series introduces libmultipath. It is essentially a refactoring of
NVME multipath support, so we can have a common library to also support
native SCSI multipath.

Much of the code is taken directly from the NVMe multipath code. However,
NVMe specifics are removed. A template structure is provided so the driver
may provide callbacks for driver specifics, like ANA support for NVMe.

Important new structures introduced include:

- mpath_head
These contain much of the multipath-specific functionality from
nvme_ns_head, including a pointer to the gendisk structure and
a path SRCU-based array.

- mpath_device
This is the per-path structure, and contains much the same
multipath-specific functionality in nvme_ns

libmultipath provides functionality for path management, path selection,
data path, and failover handling.

Since the NVMe driver has some code in the sysfs and ioctl handling
which iterate all multipath NSes, functions like mpath_call_for_device()
are added to do the same per-path iteration.

Full series also available at:
https://github.com/johnpgarry/linux/tree/scsi-multipath-v7.2-v3

Differences to v2:
- Make mpath_head embeddable in driver head structure (Sagi)
- remove get_nr_active and get_iopolicy driver callbacks
- rebase

Differences to v1:
- put current_path[] at end of struct mpath_head (Nilay)
- drop struct mpath_disk and keep nvme_remove_head() (Nilay)
- don't pass iopolicy from mpath_find_path() (Benjamin)
- change mpath_access_state names (Nilay)
- fix for setting mpath_device.nr_active and .numa_node (Nilay)
- fix uninit'ed pointers in __mpath_find_path() (Nilay)
- simplify mpath_head_template.available_path (Nilay, Benjamin)
- use DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE (Benjamin)
- check mpath_bdev_submit_bio() -> .clone_bio() for errors (Benjamin)
- drop struct mpath_pr_ops (Keith)
- drop mpath_head_template.bdev_ioctl
- drop mpath_head_template.get_unique_id
- drop mpath_head_template.report_zones
- drop mpath_head_template.get_access_state
- add mpath_head_template.ioctl_{begin, finish} and drop
mpath_head_read_unlock()
- add mpath_device.access_state
- add mpath_head_devices_empty()
- make mpath_delete_device() return a bool

John Garry (13):
  libmultipath: Add initial framework
  libmultipath: Add basic gendisk support
  libmultipath: Add path selection support
  libmultipath: Add bio handling
  libmultipath: Add support for mpath_device management
  libmultipath: Add delayed removal support
  libmultipath: Add cdev support
  libmultipath: Add sysfs helpers
  libmultipath: Add PR support
  libmultipath: Add mpath_bdev_report_zones()
  libmultipath: Add support for block device IOCTL
  libmultipath: Add mpath_bdev_getgeo()
  libmultipath: Add mpath_bdev_get_unique_id()

 include/linux/multipath.h |  182 +++++
 lib/Kconfig               |    6 +
 lib/Makefile              |    2 +
 lib/multipath.c           | 1318 +++++++++++++++++++++++++++++++++++++
 4 files changed, 1508 insertions(+)
 create mode 100644 include/linux/multipath.h
 create mode 100644 lib/multipath.c

-- 
2.43.7

[PATCH v3 01/13] libmultipath: Add initial framework

[John Garry]

Add initial framework for libmultipath. libmultipath is a library for
multipath-capable block drivers, such as NVMe. The main function is to
support path management, path selection, and failover handling.

Basic support to add and remove the head structure - mpath_head - is
included.

This main purpose of this structure is to manage available paths and path
selection. It is quite similar to the multipath functionality in
nvme_ns_head. It also manages the multipath gendisk.

Each path is represented by the mpath_device structure. It should hold a
pointer to the per-path gendisk and also a list element for all siblings
of paths. For NVMe, there would be a mpath_device per nvme_ns.

All the libmultipath code is more or less taken from
drivers/nvme/host/multipath.c, which was originally authored by Christoph
Hellwig <hch@lst.de>.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/multipath.h | 28 +++++++++++++++
 lib/Kconfig               |  6 ++++
 lib/Makefile              |  2 ++
 lib/multipath.c           | 74 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 110 insertions(+)
 create mode 100644 include/linux/multipath.h
 create mode 100644 lib/multipath.c

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
new file mode 100644
index 0000000000000..e98b4b241020a
--- /dev/null
+++ b/include/linux/multipath.h
@@ -0,0 +1,28 @@
+
+#ifndef _LIBMULTIPATH_H
+#define _LIBMULTIPATH_H
+
+#include <linux/blkdev.h>
+#include <linux/srcu.h>
+
+struct mpath_device {
+	struct list_head	siblings;
+	struct gendisk		*disk;
+};
+
+struct mpath_head {
+	struct srcu_struct	srcu;
+	struct list_head	dev_list;	/* list of all mpath_devs */
+	struct mutex		lock;
+
+	refcount_t		refcount;
+
+	struct mpath_device __rcu 		*current_path[MAX_NUMNODES];
+};
+
+int mpath_get_head(struct mpath_head *mpath_head);
+void mpath_put_head(struct mpath_head *mpath_head);
+int mpath_head_init(struct mpath_head *mpath_head);
+void mpath_head_uninit(struct mpath_head *mpath_head);
+
+#endif // _LIBMULTIPATH_H
diff --git a/lib/Kconfig b/lib/Kconfig
index 55748b68714e0..d0258bef374a1 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -636,3 +636,9 @@ config UNION_FIND
 
 config MIN_HEAP
 	bool
+
+config LIBMULTIPATH
+	bool "MULTIPATH BLOCK DRIVER LIBRARY"
+	depends on BLOCK
+	help
+	  If you say yes here then you get a multipath lib for block drivers
diff --git a/lib/Makefile b/lib/Makefile
index 7f75cc6edf94a..7ba5e13be4171 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -334,3 +334,5 @@ CONTEXT_ANALYSIS_test_context-analysis.o := y
 obj-$(CONFIG_CONTEXT_ANALYSIS_TEST) += test_context-analysis.o
 
 subdir-$(CONFIG_FORTIFY_SOURCE) += test_fortify
+
+obj-$(CONFIG_LIBMULTIPATH)	+= multipath.o
diff --git a/lib/multipath.c b/lib/multipath.c
new file mode 100644
index 0000000000000..009d4bb875c6f
--- /dev/null
+++ b/lib/multipath.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2017-2018 Christoph Hellwig.
+ * Copyright (c) 2026 Oracle and/or its affiliates.
+ */
+#include <linux/module.h>
+#include <linux/multipath.h>
+
+static struct workqueue_struct *mpath_wq;
+
+int mpath_get_head(struct mpath_head *mpath_head)
+{
+	if (!refcount_inc_not_zero(&mpath_head->refcount))
+		return -ENXIO;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mpath_get_head);
+
+static void mpath_head_cleanup(struct mpath_head *mpath_head)
+{
+	cleanup_srcu_struct(&mpath_head->srcu);
+}
+
+void mpath_put_head(struct mpath_head *mpath_head)
+{
+	refcount_t *refcount = &mpath_head->refcount;
+
+	if (refcount_dec_and_test(refcount)) {
+		mpath_head_cleanup(mpath_head);
+		wake_up_var(refcount);
+	}
+}
+EXPORT_SYMBOL_GPL(mpath_put_head);
+
+void mpath_head_uninit(struct mpath_head *mpath_head)
+{
+	refcount_t *refcount = &mpath_head->refcount;
+
+	if (refcount_dec_and_test(refcount)) {
+		mpath_head_cleanup(mpath_head);
+	} else {
+		wait_var_event(refcount, !refcount_read(refcount));
+	}
+}
+EXPORT_SYMBOL_GPL(mpath_head_uninit);
+
+int mpath_head_init(struct mpath_head *mpath_head)
+{
+	INIT_LIST_HEAD(&mpath_head->dev_list);
+	mutex_init(&mpath_head->lock);
+	refcount_set(&mpath_head->refcount, 1);
+
+	return init_srcu_struct(&mpath_head->srcu);
+}
+EXPORT_SYMBOL_GPL(mpath_head_init);
+
+static int __init mpath_init(void)
+{
+	mpath_wq = alloc_workqueue("mpath-wq",
+			WQ_UNBOUND | WQ_MEM_RECLAIM | WQ_SYSFS, 0);
+	if (!mpath_wq)
+		return -ENOMEM;
+	return 0;
+}
+
+static void __exit mpath_exit(void)
+{
+	destroy_workqueue(mpath_wq);
+}
+
+module_init(mpath_init);
+module_exit(mpath_exit);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("libmultipath");
-- 
2.43.7

[PATCH v3 02/13] libmultipath: Add basic gendisk support

[John Garry]

Add support to allocate and free a multipath gendisk.

NVMe has almost like-for-like equivalents here:
- mpath_alloc_head_disk() -> nvme_mpath_alloc_disk()
- multipath_partition_scan_work() -> nvme_partition_scan_work()
- mpath_remove_disk() -> nvme_remove_head()
- mpath_device_set_live() -> nvme_mpath_set_live()

struct mpath_head_template is introduced as a method for drivers to
provide custom multipath functionality.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/multipath.h | 38 +++++++++++++++
 lib/multipath.c           | 99 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 137 insertions(+)

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
index e98b4b241020a..ca8589d5cd8b2 100644
--- a/include/linux/multipath.h
+++ b/include/linux/multipath.h
@@ -5,11 +5,19 @@
 #include <linux/blkdev.h>
 #include <linux/srcu.h>
 
+extern const struct block_device_operations mpath_ops;
+
 struct mpath_device {
+	struct mpath_head	*mpath_head;
 	struct list_head	siblings;
 	struct gendisk		*disk;
 };
 
+struct mpath_head_template {
+};
+
+#define MPATH_HEAD_DISK_LIVE 			0
+
 struct mpath_head {
 	struct srcu_struct	srcu;
 	struct list_head	dev_list;	/* list of all mpath_devs */
@@ -17,12 +25,42 @@ struct mpath_head {
 
 	refcount_t		refcount;
 
+	unsigned long		flags;
+	struct gendisk		*disk;
+	struct work_struct	partition_scan_work;
+	struct device		*parent;
+	const struct attribute_group 		**disk_groups;
+	const struct mpath_head_template	*mpdt;
 	struct mpath_device __rcu 		*current_path[MAX_NUMNODES];
 };
 
+static inline struct mpath_head *mpath_bd_device_to_head(struct device *dev)
+{
+	return dev_get_drvdata(dev);
+}
+
+static inline struct mpath_head *mpath_gendisk_to_head(struct gendisk *disk)
+{
+	return mpath_bd_device_to_head(disk_to_dev(disk));
+}
+
 int mpath_get_head(struct mpath_head *mpath_head);
 void mpath_put_head(struct mpath_head *mpath_head);
 int mpath_head_init(struct mpath_head *mpath_head);
 void mpath_head_uninit(struct mpath_head *mpath_head);
 
+void mpath_put_disk(struct mpath_head *mpath_head);
+void mpath_remove_disk(struct mpath_head *mpath_head);
+int mpath_alloc_head_disk(struct mpath_head *mpath_head,
+			struct queue_limits *lim, int numa_node);
+void mpath_device_set_live(struct mpath_device *mpath_device);
+
+static inline bool is_mpath_disk(struct gendisk *disk)
+{
+	#if IS_ENABLED(CONFIG_LIBMULTIPATH)
+	return disk->fops == &mpath_ops;
+	#else
+	return false;
+	#endif
+}
 #endif // _LIBMULTIPATH_H
diff --git a/lib/multipath.c b/lib/multipath.c
index 009d4bb875c6f..79be84d3d4f75 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -44,12 +44,111 @@ void mpath_head_uninit(struct mpath_head *mpath_head)
 }
 EXPORT_SYMBOL_GPL(mpath_head_uninit);
 
+static int mpath_bdev_open(struct gendisk *disk, blk_mode_t mode)
+{
+	struct mpath_head *mpath_head = disk->private_data;
+
+	return mpath_get_head(mpath_head);
+}
+
+static void mpath_bdev_release(struct gendisk *disk)
+{
+	struct mpath_head *mpath_head = disk->private_data;
+
+	mpath_put_head(mpath_head);
+}
+
+const struct block_device_operations mpath_ops = {
+	.owner          = THIS_MODULE,
+	.open		= mpath_bdev_open,
+	.release	= mpath_bdev_release,
+};
+EXPORT_SYMBOL_GPL(mpath_ops);
+
+static void multipath_partition_scan_work(struct work_struct *work)
+{
+	struct mpath_head *mpath_head =
+		container_of(work, struct mpath_head, partition_scan_work);
+
+	if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
+					     &mpath_head->disk->state)))
+		return;
+
+	mutex_lock(&mpath_head->disk->open_mutex);
+	bdev_disk_changed(mpath_head->disk, false);
+	mutex_unlock(&mpath_head->disk->open_mutex);
+}
+
+void mpath_remove_disk(struct mpath_head *mpath_head)
+{
+	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
+		struct gendisk *disk = mpath_head->disk;
+
+		del_gendisk(disk);
+	}
+}
+EXPORT_SYMBOL_GPL(mpath_remove_disk);
+
+void mpath_put_disk(struct mpath_head *mpath_head)
+{
+	if (!mpath_head->disk)
+		return;
+
+	/* make sure all pending bios are cleaned up */
+	flush_work(&mpath_head->partition_scan_work);
+	put_disk(mpath_head->disk);
+}
+EXPORT_SYMBOL_GPL(mpath_put_disk);
+
+int mpath_alloc_head_disk(struct mpath_head *mpath_head,
+			struct queue_limits *lim, int numa_node)
+{
+	if (!mpath_head->disk_groups || !mpath_head->parent)
+		return -EINVAL;
+
+	mpath_head->disk = blk_alloc_disk(lim, numa_node);
+	if (IS_ERR(mpath_head->disk))
+		return PTR_ERR(mpath_head->disk);
+
+	mpath_head->disk->private_data = mpath_head;
+	mpath_head->disk->fops = &mpath_ops;
+
+	set_bit(GD_SUPPRESS_PART_SCAN, &mpath_head->disk->state);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mpath_alloc_head_disk);
+
+void mpath_device_set_live(struct mpath_device *mpath_device)
+{
+	struct mpath_head *mpath_head = mpath_device->mpath_head;
+	int ret;
+
+	if (!mpath_head->disk)
+		return;
+
+	if (!test_and_set_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
+		dev_set_drvdata(disk_to_dev(mpath_head->disk), mpath_head);
+		ret = device_add_disk(mpath_head->parent, mpath_head->disk,
+				mpath_head->disk_groups);
+		if (ret) {
+			clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags);
+			return;
+		}
+		queue_work(mpath_wq, &mpath_head->partition_scan_work);
+	}
+}
+EXPORT_SYMBOL_GPL(mpath_device_set_live);
+
 int mpath_head_init(struct mpath_head *mpath_head)
 {
 	INIT_LIST_HEAD(&mpath_head->dev_list);
 	mutex_init(&mpath_head->lock);
 	refcount_set(&mpath_head->refcount, 1);
 
+	INIT_WORK(&mpath_head->partition_scan_work,
+		multipath_partition_scan_work);
+
 	return init_srcu_struct(&mpath_head->srcu);
 }
 EXPORT_SYMBOL_GPL(mpath_head_init);
-- 
2.43.7

[PATCH v3 03/13] libmultipath: Add path selection support

[John Garry]

Add code for path selection.

NVMe ANA is abstracted into enum mpath_access_state. The motivation here is
so that SCSI ALUA can be used. Callbacks .is_disabled, .is_optimized,
.get_access_state are added to get the path access state.

Path selection modes round-robin, NUMA, and queue-depth are added, same
as NVMe supports.

NVMe has almost like-for-like equivalents here:
- __mpath_find_path() -> __nvme_find_path()
- mpath_find_path() -> nvme_find_path()

and similar for all introduced callee functions.

Functions mpath_set_iopolicy() and mpath_get_iopolicy() are added for
setting default iopolicy.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/multipath.h |  33 +++++
 lib/multipath.c           | 252 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 284 insertions(+), 1 deletion(-)

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
index ca8589d5cd8b2..6c75654c12f8f 100644
--- a/include/linux/multipath.h
+++ b/include/linux/multipath.h
@@ -7,13 +7,31 @@
 
 extern const struct block_device_operations mpath_ops;
 
+enum mpath_iopolicy_e {
+	MPATH_IOPOLICY_NUMA,
+	MPATH_IOPOLICY_RR,
+	MPATH_IOPOLICY_QD,
+};
+
+enum mpath_access_state {
+	MPATH_STATE_OPTIMIZED,
+	MPATH_STATE_NONOPTIMIZED,
+	MPATH_STATE_OTHER
+};
+
 struct mpath_device {
 	struct mpath_head	*mpath_head;
 	struct list_head	siblings;
 	struct gendisk		*disk;
+	int			numa_node;
+	atomic_t		*nr_active;
+	enum mpath_access_state access_state;
 };
 
 struct mpath_head_template {
+	bool (*is_disabled)(struct mpath_device *);
+	bool (*is_optimized)(struct mpath_device *);
+	const struct attribute_group **device_groups;
 };
 
 #define MPATH_HEAD_DISK_LIVE 			0
@@ -25,6 +43,7 @@ struct mpath_head {
 
 	refcount_t		refcount;
 
+	enum mpath_iopolicy_e	*iopolicy;
 	unsigned long		flags;
 	struct gendisk		*disk;
 	struct work_struct	partition_scan_work;
@@ -44,6 +63,14 @@ static inline struct mpath_head *mpath_gendisk_to_head(struct gendisk *disk)
 	return mpath_bd_device_to_head(disk_to_dev(disk));
 }
 
+static inline enum mpath_iopolicy_e mpath_read_iopolicy(
+			struct mpath_head *mpath_head)
+{
+	return READ_ONCE(*mpath_head->iopolicy);
+}
+void mpath_synchronize(struct mpath_head *mpath_head);
+int mpath_set_iopolicy(const char *str, enum mpath_iopolicy_e *iopolicy);
+int mpath_get_iopolicy(char *buf, int iopolicy);
 int mpath_get_head(struct mpath_head *mpath_head);
 void mpath_put_head(struct mpath_head *mpath_head);
 int mpath_head_init(struct mpath_head *mpath_head);
@@ -63,4 +90,10 @@ static inline bool is_mpath_disk(struct gendisk *disk)
 	return false;
 	#endif
 }
+
+static inline bool mpath_qd_iopolicy(enum mpath_iopolicy_e *iopolicy)
+{
+	return READ_ONCE(*iopolicy) == MPATH_IOPOLICY_QD;
+}
+
 #endif // _LIBMULTIPATH_H
diff --git a/lib/multipath.c b/lib/multipath.c
index 79be84d3d4f75..21f7ffdb22d60 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -6,8 +6,243 @@
 #include <linux/module.h>
 #include <linux/multipath.h>
 
+static struct mpath_device *mpath_find_path(struct mpath_head *mpath_head);
+
 static struct workqueue_struct *mpath_wq;
 
+static const char * const mpath_iopolicy_names[] = {
+	[MPATH_IOPOLICY_NUMA]	= "numa",
+	[MPATH_IOPOLICY_RR]	= "round-robin",
+	[MPATH_IOPOLICY_QD]	= "queue-depth",
+};
+
+static int mpath_iopolicy_parse(const char *str)
+{
+	return __sysfs_match_string(mpath_iopolicy_names,
+		ARRAY_SIZE(mpath_iopolicy_names), str);
+}
+
+int mpath_set_iopolicy(const char *str, enum mpath_iopolicy_e *iopolicy)
+{
+	int policy;
+
+	if (!str)
+		return -EINVAL;
+	policy = mpath_iopolicy_parse(str);
+	if (policy < 0)
+		return policy;
+	*iopolicy = policy;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mpath_set_iopolicy);
+
+int mpath_get_iopolicy(char *buf, int iopolicy)
+{
+	return sprintf(buf, "%s\n", mpath_iopolicy_names[iopolicy]);
+}
+EXPORT_SYMBOL_GPL(mpath_get_iopolicy);
+
+
+void mpath_synchronize(struct mpath_head *mpath_head)
+{
+	synchronize_srcu(&mpath_head->srcu);
+}
+EXPORT_SYMBOL_GPL(mpath_synchronize);
+
+static bool mpath_path_is_disabled(struct mpath_head *mpath_head,
+				struct mpath_device *mpath_device)
+{
+	return mpath_head->mpdt->is_disabled(mpath_device);
+}
+
+static struct mpath_device *__mpath_find_path(struct mpath_head *mpath_head,
+					int node)
+{
+	int found_distance = INT_MAX, fallback_distance = INT_MAX, distance;
+	struct mpath_device *found = NULL, *fallback = NULL, *mpath_device;
+
+	list_for_each_entry_srcu(mpath_device, &mpath_head->dev_list, siblings,
+		srcu_read_lock_held(&mpath_head->srcu)) {
+		if (mpath_path_is_disabled(mpath_head, mpath_device))
+			continue;
+
+		if (mpath_device->numa_node != NUMA_NO_NODE &&
+		    (mpath_read_iopolicy(mpath_head) ==
+			MPATH_IOPOLICY_NUMA))
+			distance = node_distance(node,
+					mpath_device->numa_node);
+		else
+			distance = LOCAL_DISTANCE;
+
+		switch(mpath_device->access_state) {
+		case MPATH_STATE_OPTIMIZED:
+		    if (distance < found_distance) {
+			    found_distance = distance;
+			    found = mpath_device;
+		    }
+		    break;
+		case MPATH_STATE_NONOPTIMIZED:
+		    if (distance < fallback_distance) {
+			    fallback_distance = distance;
+			    fallback = mpath_device;
+		    }
+		    break;
+		default:
+		    break;
+		}
+	}
+
+	if (!found)
+		found = fallback;
+
+	if (found)
+		rcu_assign_pointer(mpath_head->current_path[node], found);
+
+	return found;
+}
+
+static struct mpath_device *mpath_next_dev(struct mpath_head *mpath_head,
+				struct mpath_device *mpath_dev)
+{
+	mpath_dev = list_next_or_null_rcu(&mpath_head->dev_list,
+			&mpath_dev->siblings, struct mpath_device,
+			siblings);
+
+	if (mpath_dev)
+		return mpath_dev;
+	return list_first_or_null_rcu(&mpath_head->dev_list,
+				struct mpath_device, siblings);
+}
+
+static struct mpath_device *mpath_round_robin_path(
+				struct mpath_head *mpath_head)
+{
+	struct mpath_device *mpath_device, *found = NULL;
+	int node = numa_node_id();
+	enum mpath_access_state access_state_old;
+	struct mpath_device *old =
+			srcu_dereference(mpath_head->current_path[node],
+				&mpath_head->srcu);
+
+	if (unlikely(!old))
+		return __mpath_find_path(mpath_head, node);
+
+	if (list_is_singular(&mpath_head->dev_list)) {
+		if (mpath_path_is_disabled(mpath_head, old))
+			return NULL;
+		return old;
+	}
+
+	for (mpath_device = mpath_next_dev(mpath_head, old);
+	    mpath_device && mpath_device != old;
+	    mpath_device = mpath_next_dev(mpath_head, mpath_device)) {
+
+		if (mpath_path_is_disabled(mpath_head, mpath_device))
+			continue;
+		if (mpath_device->access_state == MPATH_STATE_OPTIMIZED) {
+			found = mpath_device;
+			goto out;
+		}
+		if (mpath_device->access_state == MPATH_STATE_NONOPTIMIZED)
+			found = mpath_device;
+	}
+
+	/*
+	 * The loop above skips the current path for round-robin semantics.
+	 * Fall back to the current path if either:
+	 *  - no other optimized path found and current is optimized,
+	 *  - no other usable path found and current is usable.
+	 */
+	access_state_old = old->access_state;
+	if (!mpath_path_is_disabled(mpath_head, old) &&
+	    (access_state_old == MPATH_STATE_OPTIMIZED ||
+	    (!found && access_state_old == MPATH_STATE_NONOPTIMIZED)))
+		return old;
+
+	if (!found)
+		return NULL;
+out:
+	rcu_assign_pointer(mpath_head->current_path[node], found);
+
+	return found;
+}
+
+static struct mpath_device *mpath_queue_depth_path(
+				struct mpath_head *mpath_head)
+{
+	struct mpath_device *best_opt = NULL, *mpath_device;
+	struct mpath_device *best_nonopt = NULL;
+	unsigned int min_depth_opt = UINT_MAX, min_depth_nonopt = UINT_MAX;
+	unsigned int depth;
+
+	list_for_each_entry_srcu(mpath_device, &mpath_head->dev_list, siblings,
+				 srcu_read_lock_held(&mpath_head->srcu)) {
+
+		if (mpath_path_is_disabled(mpath_head, mpath_device))
+			continue;
+
+		depth = atomic_read(mpath_device->nr_active);
+
+		switch (mpath_device->access_state) {
+		case MPATH_STATE_OPTIMIZED:
+			if (depth < min_depth_opt) {
+				min_depth_opt = depth;
+				best_opt = mpath_device;
+			}
+			break;
+		case MPATH_STATE_NONOPTIMIZED:
+			if (depth < min_depth_nonopt) {
+				min_depth_nonopt = depth;
+				best_nonopt = mpath_device;
+			}
+			break;
+		default:
+			break;
+		}
+
+		if (min_depth_opt == 0)
+			return best_opt;
+	}
+
+	return best_opt ? best_opt : best_nonopt;
+}
+
+static inline bool mpath_path_is_optimized(struct mpath_head *mpath_head,
+				struct mpath_device *mpath_device)
+{
+	return mpath_head->mpdt->is_optimized(mpath_device);
+}
+
+static struct mpath_device *mpath_numa_path(struct mpath_head *mpath_head)
+{
+	int node = numa_node_id();
+	struct mpath_device *mpath_device;
+
+	mpath_device = srcu_dereference(mpath_head->current_path[node],
+					&mpath_head->srcu);
+	if (unlikely(!mpath_device))
+		return __mpath_find_path(mpath_head, node);
+	if (unlikely(!mpath_path_is_optimized(mpath_head, mpath_device)))
+		return __mpath_find_path(mpath_head, node);
+	return mpath_device;
+}
+
+__maybe_unused
+static struct mpath_device *mpath_find_path(struct mpath_head *mpath_head)
+{
+	enum mpath_iopolicy_e iopolicy = mpath_read_iopolicy(mpath_head);
+
+	switch (iopolicy) {
+	case MPATH_IOPOLICY_QD:
+		return mpath_queue_depth_path(mpath_head);
+	case MPATH_IOPOLICY_RR:
+		return mpath_round_robin_path(mpath_head);
+	default:
+		return mpath_numa_path(mpath_head);
+	}
+}
+
 int mpath_get_head(struct mpath_head *mpath_head)
 {
 	if (!refcount_inc_not_zero(&mpath_head->refcount))
@@ -84,6 +319,7 @@ void mpath_remove_disk(struct mpath_head *mpath_head)
 	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
 		struct gendisk *disk = mpath_head->disk;
 
+		mpath_synchronize(mpath_head);
 		del_gendisk(disk);
 	}
 }
@@ -103,7 +339,8 @@ EXPORT_SYMBOL_GPL(mpath_put_disk);
 int mpath_alloc_head_disk(struct mpath_head *mpath_head,
 			struct queue_limits *lim, int numa_node)
 {
-	if (!mpath_head->disk_groups || !mpath_head->parent)
+	if (!mpath_head->disk_groups || !mpath_head->parent ||
+	    !mpath_head->iopolicy)
 		return -EINVAL;
 
 	mpath_head->disk = blk_alloc_disk(lim, numa_node);
@@ -137,6 +374,19 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
 		}
 		queue_work(mpath_wq, &mpath_head->partition_scan_work);
 	}
+
+	mutex_lock(&mpath_head->lock);
+	if (mpath_path_is_optimized(mpath_head, mpath_device)) {
+		int node, srcu_idx;
+
+		srcu_idx = srcu_read_lock(&mpath_head->srcu);
+		for_each_online_node(node)
+			__mpath_find_path(mpath_head, node);
+		srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+	}
+	mutex_unlock(&mpath_head->lock);
+
+	mpath_synchronize(mpath_head);
 }
 EXPORT_SYMBOL_GPL(mpath_device_set_live);
 
-- 
2.43.7

[PATCH v3 04/13] libmultipath: Add bio handling

[John Garry]

Add support to submit a bio per-path. In addition, for failover, add
support to requeue a failed bio.

NVMe has almost like-for-like equivalents here:
    - nvme_available_path() -> mpath_available_path()
    - nvme_requeue_work() -> mpath_requeue_work()
    - nvme_ns_head_submit_bio() -> mpath_bdev_submit_bio()

For failover, a driver may want to re-submit a bio, so add support to
clone a bio prior to submission.

A bio which is submitted to a per-path device has flag REQ_MPATH set,
same as what is done for NVMe with REQ_NVME_MPATH.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/multipath.h |  22 ++++++++
 lib/multipath.c           | 108 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 128 insertions(+), 2 deletions(-)

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
index 6c75654c12f8f..57d13eb1450cd 100644
--- a/include/linux/multipath.h
+++ b/include/linux/multipath.h
@@ -3,6 +3,7 @@
 #define _LIBMULTIPATH_H
 
 #include <linux/blkdev.h>
+#include <linux/blk-mq.h>
 #include <linux/srcu.h>
 
 extern const struct block_device_operations mpath_ops;
@@ -29,8 +30,10 @@ struct mpath_device {
 };
 
 struct mpath_head_template {
+	bool (*available_path)(struct mpath_device *);
 	bool (*is_disabled)(struct mpath_device *);
 	bool (*is_optimized)(struct mpath_device *);
+	struct bio *(*clone_bio)(struct bio *);
 	const struct attribute_group **device_groups;
 };
 
@@ -44,6 +47,14 @@ struct mpath_head {
 	refcount_t		refcount;
 
 	enum mpath_iopolicy_e	*iopolicy;
+
+	struct bio_list		requeue_list; /* list for requeing bio */
+	spinlock_t		requeue_lock;
+	struct work_struct	requeue_work; /* work struct for requeue */
+
+	atomic_long_t		requeue_no_usable_path_cnt;
+	atomic_long_t		fail_no_avail_path_cnt;
+
 	unsigned long		flags;
 	struct gendisk		*disk;
 	struct work_struct	partition_scan_work;
@@ -53,6 +64,13 @@ struct mpath_head {
 	struct mpath_device __rcu 		*current_path[MAX_NUMNODES];
 };
 
+#define REQ_MPATH		REQ_DRV
+
+static inline bool is_mpath_request(struct request *req)
+{
+	return req->cmd_flags & REQ_MPATH;
+}
+
 static inline struct mpath_head *mpath_bd_device_to_head(struct device *dev)
 {
 	return dev_get_drvdata(dev);
@@ -96,4 +114,8 @@ static inline bool mpath_qd_iopolicy(enum mpath_iopolicy_e *iopolicy)
 	return READ_ONCE(*iopolicy) == MPATH_IOPOLICY_QD;
 }
 
+static inline void mpath_schedule_requeue_work(struct mpath_head *mpath_head)
+{
+	kblockd_schedule_work(&mpath_head->requeue_work);
+}
 #endif // _LIBMULTIPATH_H
diff --git a/lib/multipath.c b/lib/multipath.c
index 21f7ffdb22d60..81e737c1ce469 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -5,6 +5,7 @@
  */
 #include <linux/module.h>
 #include <linux/multipath.h>
+#include <trace/events/block.h>
 
 static struct mpath_device *mpath_find_path(struct mpath_head *mpath_head);
 
@@ -43,7 +44,6 @@ int mpath_get_iopolicy(char *buf, int iopolicy)
 }
 EXPORT_SYMBOL_GPL(mpath_get_iopolicy);
 
-
 void mpath_synchronize(struct mpath_head *mpath_head)
 {
 	synchronize_srcu(&mpath_head->srcu);
@@ -228,7 +228,6 @@ static struct mpath_device *mpath_numa_path(struct mpath_head *mpath_head)
 	return mpath_device;
 }
 
-__maybe_unused
 static struct mpath_device *mpath_find_path(struct mpath_head *mpath_head)
 {
 	enum mpath_iopolicy_e iopolicy = mpath_read_iopolicy(mpath_head);
@@ -243,6 +242,81 @@ static struct mpath_device *mpath_find_path(struct mpath_head *mpath_head)
 	}
 }
 
+static bool mpath_available_path(struct mpath_head *mpath_head)
+{
+	struct mpath_device *mpath_device;
+
+	if (!test_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags))
+		return false;
+
+	list_for_each_entry_srcu(mpath_device, &mpath_head->dev_list, siblings,
+				 srcu_read_lock_held(&mpath_head->srcu)) {
+		if (mpath_head->mpdt->available_path(mpath_device))
+			return true;
+	}
+
+	return false;
+}
+
+static void mpath_bdev_submit_bio(struct bio *bio)
+{
+	struct mpath_head *mpath_head = bio->bi_bdev->bd_disk->private_data;
+	struct device *dev = mpath_head->parent;
+	struct mpath_device *mpath_device;
+	int srcu_idx;
+
+	/*
+	 * The mpath_device might be going away and the bio might be moved to a
+	 * different queue in failover, so we need to use the bio_split
+	 * pool from the original queue to allocate the bvecs from.
+	 */
+	bio = bio_split_to_limits(bio);
+	if (!bio)
+		return;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+
+	if (likely(mpath_device)) {
+		if (mpath_head->mpdt->clone_bio) {
+			struct bio *orig = bio;
+
+			bio = mpath_head->mpdt->clone_bio(bio);
+			if (!bio) {
+				bio_io_error(orig);
+				goto out;
+			}
+		}
+
+		bio_set_dev(bio, mpath_device->disk->part0);
+		/*
+		 * Use BIO_REMAPPED to skip bio_check_eod() when this bio
+		 * enters submit_bio_noacct() for the per-path device. The EOD
+		 * check already passed on the multipath head.
+		 */
+		bio_set_flag(bio, BIO_REMAPPED);
+		bio->bi_opf |= REQ_MPATH;
+		trace_block_bio_remap(bio, disk_devt(mpath_device->disk),
+				      bio->bi_iter.bi_sector);
+		submit_bio_noacct(bio);
+	} else if (mpath_available_path(mpath_head)) {
+		dev_warn_ratelimited(dev, "no usable path - requeuing I/O\n");
+
+		spin_lock_irq(&mpath_head->requeue_lock);
+		bio_list_add(&mpath_head->requeue_list, bio);
+		spin_unlock_irq(&mpath_head->requeue_lock);
+		atomic_long_inc(&mpath_head->requeue_no_usable_path_cnt);
+	} else {
+		dev_warn_ratelimited(dev, "no available path - failing I/O\n");
+
+		bio_io_error(bio);
+		atomic_long_inc(&mpath_head->fail_no_avail_path_cnt);
+	}
+
+out:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+}
+
 int mpath_get_head(struct mpath_head *mpath_head)
 {
 	if (!refcount_inc_not_zero(&mpath_head->refcount))
@@ -297,6 +371,7 @@ const struct block_device_operations mpath_ops = {
 	.owner          = THIS_MODULE,
 	.open		= mpath_bdev_open,
 	.release	= mpath_bdev_release,
+	.submit_bio	= mpath_bdev_submit_bio,
 };
 EXPORT_SYMBOL_GPL(mpath_ops);
 
@@ -314,11 +389,34 @@ static void multipath_partition_scan_work(struct work_struct *work)
 	mutex_unlock(&mpath_head->disk->open_mutex);
 }
 
+static void mpath_requeue_work(struct work_struct *work)
+{
+	struct mpath_head *mpath_head =
+	    container_of(work, struct mpath_head, requeue_work);
+	struct bio *bio, *next;
+
+	spin_lock_irq(&mpath_head->requeue_lock);
+	next = bio_list_get(&mpath_head->requeue_list);
+	spin_unlock_irq(&mpath_head->requeue_lock);
+
+	while ((bio = next) != NULL) {
+		next = bio->bi_next;
+		bio->bi_next = NULL;
+		submit_bio_noacct(bio);
+	}
+}
+
 void mpath_remove_disk(struct mpath_head *mpath_head)
 {
 	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
 		struct gendisk *disk = mpath_head->disk;
 
+		/*
+		 * requeue I/O after MPATH_HEAD_DISK_LIVE has been cleared
+		 * to allow multipath to fail all I/O.
+		 */
+		mpath_schedule_requeue_work(mpath_head);
+
 		mpath_synchronize(mpath_head);
 		del_gendisk(disk);
 	}
@@ -331,6 +429,8 @@ void mpath_put_disk(struct mpath_head *mpath_head)
 		return;
 
 	/* make sure all pending bios are cleaned up */
+	kblockd_schedule_work(&mpath_head->requeue_work);
+	flush_work(&mpath_head->requeue_work);
 	flush_work(&mpath_head->partition_scan_work);
 	put_disk(mpath_head->disk);
 }
@@ -387,6 +487,7 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
 	mutex_unlock(&mpath_head->lock);
 
 	mpath_synchronize(mpath_head);
+	mpath_schedule_requeue_work(mpath_head);
 }
 EXPORT_SYMBOL_GPL(mpath_device_set_live);
 
@@ -398,6 +499,9 @@ int mpath_head_init(struct mpath_head *mpath_head)
 
 	INIT_WORK(&mpath_head->partition_scan_work,
 		multipath_partition_scan_work);
+	INIT_WORK(&mpath_head->requeue_work, mpath_requeue_work);
+	spin_lock_init(&mpath_head->requeue_lock);
+	bio_list_init(&mpath_head->requeue_list);
 
 	return init_srcu_struct(&mpath_head->srcu);
 }
-- 
2.43.7

[PATCH v3 05/13] libmultipath: Add support for mpath_device management

[John Garry]

Add support to add or remove a mpath_device as a path.

NVMe has almost like-for-like equivalents here:
- nvme_mpath_clear_current_path() -> mpath_clear_current_path()
- nvme_mpath_add_sysfs_link() -> mpath_add_sysfs_link()
- nvme_mpath_remove_sysfs_link() -> mpath_remove_sysfs_link()
- nvme_mpath_revalidate_paths() -> mpath_revalidate_paths()

mpath_revalidate_paths() has a CB arg for NVMe specific handling.

The functionality in mpath_clear_paths() and mpath_synchronize() have the
same pattern which is frequently used in the NVMe code.

Helper mpath_call_for_device() is added to allow a driver run a callback
on any path available. It is intended to be used for occasions when the
NVMe drivers accesses the list of paths outside its multipath code, like
NVMe sysfs.c

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/multipath.h |  17 ++++
 lib/multipath.c           | 185 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 202 insertions(+)

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
index 57d13eb1450cd..ec4325b77cf8c 100644
--- a/include/linux/multipath.h
+++ b/include/linux/multipath.h
@@ -20,10 +20,13 @@ enum mpath_access_state {
 	MPATH_STATE_OTHER
 };
 
+#define MPATH_DEVICE_SYSFS_ATTR_LINK      0
+
 struct mpath_device {
 	struct mpath_head	*mpath_head;
 	struct list_head	siblings;
 	struct gendisk		*disk;
+	unsigned long		flags;
 	int			numa_node;
 	atomic_t		*nr_active;
 	enum mpath_access_state access_state;
@@ -89,6 +92,20 @@ static inline enum mpath_iopolicy_e mpath_read_iopolicy(
 void mpath_synchronize(struct mpath_head *mpath_head);
 int mpath_set_iopolicy(const char *str, enum mpath_iopolicy_e *iopolicy);
 int mpath_get_iopolicy(char *buf, int iopolicy);
+bool mpath_clear_current_path(struct mpath_device *mpath_device);
+void mpath_synchronize(struct mpath_head *mpath_head);
+void mpath_add_device(struct mpath_device *mpath_device,
+		struct mpath_head *mpath_head, struct gendisk *disk,
+		int numa_node, atomic_t *nr_active);
+bool mpath_delete_device(struct mpath_device *mpath_device);
+bool mpath_head_devices_empty(struct mpath_head *mpath_head);
+int mpath_call_for_device(struct mpath_head *mpath_head,
+			int (*cb)(struct mpath_device *mpath_device));
+void mpath_clear_paths(struct mpath_head *mpath_head);
+void mpath_revalidate_paths(struct mpath_head *mpath_head,
+	void (*not_ready_cb)(struct mpath_device *mpath_device));
+void mpath_add_sysfs_link(struct mpath_head *mpath_head);
+void mpath_remove_sysfs_link(struct mpath_device *mpath_device);
 int mpath_get_head(struct mpath_head *mpath_head);
 void mpath_put_head(struct mpath_head *mpath_head);
 int mpath_head_init(struct mpath_head *mpath_head);
diff --git a/lib/multipath.c b/lib/multipath.c
index 81e737c1ce469..007aa34796569 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -50,6 +50,118 @@ void mpath_synchronize(struct mpath_head *mpath_head)
 }
 EXPORT_SYMBOL_GPL(mpath_synchronize);
 
+void mpath_add_device(struct mpath_device *mpath_device,
+		struct mpath_head *mpath_head, struct gendisk *disk,
+		int numa_node, atomic_t *nr_active)
+{
+	mpath_device->mpath_head = mpath_head;
+	mpath_device->disk = disk;
+	mpath_device->numa_node = numa_node;
+	mpath_device->nr_active = nr_active;
+	mutex_lock(&mpath_head->lock);
+	list_add_tail_rcu(&mpath_device->siblings, &mpath_head->dev_list);
+	mutex_unlock(&mpath_head->lock);
+}
+EXPORT_SYMBOL_GPL(mpath_add_device);
+
+bool mpath_delete_device(struct mpath_device *mpath_device)
+{
+	bool empty;
+
+	mutex_lock(&mpath_device->mpath_head->lock);
+	list_del_rcu(&mpath_device->siblings);
+	empty = list_empty(&mpath_device->mpath_head->dev_list);
+	mutex_unlock(&mpath_device->mpath_head->lock);
+
+	return empty;
+}
+EXPORT_SYMBOL_GPL(mpath_delete_device);
+
+bool mpath_head_devices_empty(struct mpath_head *mpath_head)
+{
+	bool empty;
+
+	mutex_lock(&mpath_head->lock);
+	empty = list_empty(&mpath_head->dev_list);
+	mutex_unlock(&mpath_head->lock);
+
+	return empty;
+}
+EXPORT_SYMBOL_GPL(mpath_head_devices_empty);
+
+int mpath_call_for_device(struct mpath_head *mpath_head,
+			int (*cb)(struct mpath_device *mpath_device))
+{
+	struct mpath_device *mpath_device;
+	int ret = -EWOULDBLOCK, srcu_idx;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device)
+		ret = cb(mpath_device);
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mpath_call_for_device);
+
+bool mpath_clear_current_path(struct mpath_device *mpath_device)
+{
+	struct mpath_head *mpath_head = mpath_device->mpath_head;
+	bool changed = false;
+	int node;
+
+	for_each_node(node) {
+		if (mpath_device ==
+			rcu_access_pointer(mpath_head->current_path[node])) {
+			rcu_assign_pointer(mpath_head->current_path[node],
+				NULL);
+			changed = true;
+		}
+	}
+
+	return changed;
+}
+EXPORT_SYMBOL_GPL(mpath_clear_current_path);
+
+static void mpath_revalidate_paths_iter(struct mpath_head *mpath_head,
+	void (*not_ready_cb)(struct mpath_device *mpath_device))
+{
+	sector_t capacity = get_capacity(mpath_head->disk);
+	struct mpath_device *mpath_device;
+	int srcu_idx;
+
+	if (!not_ready_cb)
+		return;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	list_for_each_entry_srcu(mpath_device, &mpath_head->dev_list, siblings,
+				 srcu_read_lock_held(&mpath_head->srcu)) {
+		if (capacity != get_capacity(mpath_device->disk))
+			not_ready_cb(mpath_device);
+	}
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+}
+
+void mpath_clear_paths(struct mpath_head *mpath_head)
+{
+	int node;
+
+	for_each_node(node)
+		rcu_assign_pointer(mpath_head->current_path[node], NULL);
+}
+EXPORT_SYMBOL_GPL(mpath_clear_paths);
+
+void mpath_revalidate_paths(struct mpath_head *mpath_head,
+	void (*not_ready_cb)(struct mpath_device *mpath_device))
+{
+	mpath_revalidate_paths_iter(mpath_head, not_ready_cb);
+	mpath_clear_paths(mpath_head);
+
+	mpath_schedule_requeue_work(mpath_head);
+}
+EXPORT_SYMBOL_GPL(mpath_revalidate_paths);
+
 static bool mpath_path_is_disabled(struct mpath_head *mpath_head,
 				struct mpath_device *mpath_device)
 {
@@ -475,6 +587,8 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
 		queue_work(mpath_wq, &mpath_head->partition_scan_work);
 	}
 
+	mpath_add_sysfs_link(mpath_head);
+
 	mutex_lock(&mpath_head->lock);
 	if (mpath_path_is_optimized(mpath_head, mpath_device)) {
 		int node, srcu_idx;
@@ -491,6 +605,77 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
 }
 EXPORT_SYMBOL_GPL(mpath_device_set_live);
 
+void mpath_add_sysfs_link(struct mpath_head *mpath_head)
+{
+	struct device *target;
+	struct device *source;
+	int rc, srcu_idx;
+	struct kobject *mpath_gd_kobj;
+	struct mpath_device *mpath_device;
+
+	/*
+	 * Ensure head disk node is already added otherwise we may get invalid
+	 * kobj for head disk node
+	 */
+	if (!test_bit(GD_ADDED, &mpath_head->disk->state))
+		return;
+
+	mpath_gd_kobj = &disk_to_dev(mpath_head->disk)->kobj;
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+
+	list_for_each_entry_srcu(mpath_device, &mpath_head->dev_list, siblings,
+				 srcu_read_lock_held(&mpath_head->srcu)) {
+		if (!test_bit(GD_ADDED, &mpath_device->disk->state))
+			continue;
+
+		if (test_and_set_bit(MPATH_DEVICE_SYSFS_ATTR_LINK,
+					&mpath_device->flags))
+			continue;
+
+		target = disk_to_dev(mpath_device->disk);
+		source = disk_to_dev(mpath_head->disk);
+		/*
+		 * Create sysfs link from head gendisk kobject @kobj to the
+		 * ns path gendisk kobject @target->kobj.
+		 */
+		rc = sysfs_add_link_to_group(mpath_gd_kobj, "multipath",
+				&target->kobj, dev_name(target));
+
+		if (unlikely(rc)) {
+			dev_err(disk_to_dev(mpath_head->disk),
+					"failed to create link to %s rc=%d\n",
+					dev_name(target), rc);
+			clear_bit(MPATH_DEVICE_SYSFS_ATTR_LINK,
+					&mpath_device->flags);
+		} else {
+			dev_info(source, "Created multipath sysfs link to %s\n",
+					mpath_device->disk->disk_name);
+		}
+	}
+
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+}
+EXPORT_SYMBOL_GPL(mpath_add_sysfs_link);
+
+void mpath_remove_sysfs_link(struct mpath_device *mpath_device)
+{
+	struct device *target;
+	struct kobject *mpath_gd_kobj;
+	struct mpath_head *mpath_head = mpath_device->mpath_head;
+
+	if (!test_bit(MPATH_DEVICE_SYSFS_ATTR_LINK, &mpath_device->flags))
+		return;
+
+	target = disk_to_dev(mpath_device->disk);
+	mpath_gd_kobj = &disk_to_dev(mpath_head->disk)->kobj;
+
+	sysfs_remove_link_from_group(mpath_gd_kobj, "multipath",
+			dev_name(target));
+
+	clear_bit(MPATH_DEVICE_SYSFS_ATTR_LINK, &mpath_device->flags);
+}
+EXPORT_SYMBOL_GPL(mpath_remove_sysfs_link);
+
 int mpath_head_init(struct mpath_head *mpath_head)
 {
 	INIT_LIST_HEAD(&mpath_head->dev_list);
-- 
2.43.7

[PATCH v3 06/13] libmultipath: Add delayed removal support

[John Garry]

Add support for delayed removal, same as exists for NVMe.

The purpose of this feature is to keep the multipath disk and cdev present
for intermittent periods of no available path.

Helpers mpath_delayed_removal_secs_show() and
mpath_delayed_removal_secs_store() may be used in the driver sysfs code.

The driver is responsible for supplying the removal work callback for
the delayed work.

Signed-off-by: John Garry <john.g.garry@oracle.com>=
---
 include/linux/multipath.h | 18 ++++++++
 lib/multipath.c           | 91 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
index ec4325b77cf8c..8e4b3fc197637 100644
--- a/include/linux/multipath.h
+++ b/include/linux/multipath.h
@@ -34,6 +34,7 @@ struct mpath_device {
 
 struct mpath_head_template {
 	bool (*available_path)(struct mpath_device *);
+	void (*remove_head)(struct mpath_head *);
 	bool (*is_disabled)(struct mpath_device *);
 	bool (*is_optimized)(struct mpath_device *);
 	struct bio *(*clone_bio)(struct bio *);
@@ -41,6 +42,7 @@ struct mpath_head_template {
 };
 
 #define MPATH_HEAD_DISK_LIVE 			0
+#define MPATH_HEAD_QUEUE_IF_NO_PATH		1
 
 struct mpath_head {
 	struct srcu_struct	srcu;
@@ -58,6 +60,10 @@ struct mpath_head {
 	atomic_long_t		requeue_no_usable_path_cnt;
 	atomic_long_t		fail_no_avail_path_cnt;
 
+	struct delayed_work	remove_work;
+	unsigned int		delayed_removal_secs;
+	struct module		*drv_module;
+
 	unsigned long		flags;
 	struct gendisk		*disk;
 	struct work_struct	partition_scan_work;
@@ -116,6 +122,11 @@ void mpath_remove_disk(struct mpath_head *mpath_head);
 int mpath_alloc_head_disk(struct mpath_head *mpath_head,
 			struct queue_limits *lim, int numa_node);
 void mpath_device_set_live(struct mpath_device *mpath_device);
+bool mpath_can_remove_head(struct mpath_head *mpath_head);
+ssize_t mpath_delayed_removal_secs_show(struct mpath_head *mpath_head,
+			char *buf);
+ssize_t mpath_delayed_removal_secs_store(struct mpath_head *mpath_head,
+			const char *buf, size_t count);
 
 static inline bool is_mpath_disk(struct gendisk *disk)
 {
@@ -131,6 +142,13 @@ static inline bool mpath_qd_iopolicy(enum mpath_iopolicy_e *iopolicy)
 	return READ_ONCE(*iopolicy) == MPATH_IOPOLICY_QD;
 }
 
+static inline bool mpath_head_queue_if_no_path(struct mpath_head *mpath_head)
+{
+	if (test_bit(MPATH_HEAD_QUEUE_IF_NO_PATH, &mpath_head->flags))
+		return true;
+	return false;
+}
+
 static inline void mpath_schedule_requeue_work(struct mpath_head *mpath_head)
 {
 	kblockd_schedule_work(&mpath_head->requeue_work);
diff --git a/lib/multipath.c b/lib/multipath.c
index 007aa34796569..78f88b0664c78 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -61,6 +61,8 @@ void mpath_add_device(struct mpath_device *mpath_device,
 	mutex_lock(&mpath_head->lock);
 	list_add_tail_rcu(&mpath_device->siblings, &mpath_head->dev_list);
 	mutex_unlock(&mpath_head->lock);
+	if (cancel_delayed_work(&mpath_head->remove_work))
+		module_put(mpath_head->drv_module);
 }
 EXPORT_SYMBOL_GPL(mpath_add_device);
 
@@ -367,7 +369,17 @@ static bool mpath_available_path(struct mpath_head *mpath_head)
 			return true;
 	}
 
-	return false;
+	/*
+	 * If "mpath_head->delayed_removal_secs" is set (i.e., non-zero), do
+	 * not immediately fail I/O. Instead, requeue the I/O for the configured
+	 * duration, anticipating that if there's a transient link failure then
+	 * it may recover within this time window. This parameter is exported to
+	 * userspace via sysfs, and its default value is zero. It is internally
+	 * mapped to MPATH_HEAD_QUEUE_IF_NO_PATH. When delayed_removal_secs is
+	 * non-zero, this flag is set to true. When zero, the flag is cleared.
+	 */
+	return mpath_head_queue_if_no_path(mpath_head);
+
 }
 
 static void mpath_bdev_submit_bio(struct bio *bio)
@@ -518,6 +530,39 @@ static void mpath_requeue_work(struct work_struct *work)
 	}
 }
 
+bool mpath_can_remove_head(struct mpath_head *mpath_head)
+{
+	bool remove = false;
+
+	mutex_lock(&mpath_head->lock);
+	/*
+	 * Ensure that no one could remove this module while the head
+	 * remove work is pending.
+	 */
+	if (mpath_head_queue_if_no_path(mpath_head) &&
+		try_module_get(mpath_head->drv_module)) {
+
+		mod_delayed_work(mpath_wq, &mpath_head->remove_work,
+				mpath_head->delayed_removal_secs * HZ);
+	} else {
+		remove = true;
+	}
+
+	mutex_unlock(&mpath_head->lock);
+	return remove;
+}
+EXPORT_SYMBOL_GPL(mpath_can_remove_head);
+
+static void mpath_remove_head_work(struct work_struct *work)
+{
+	struct mpath_head *mpath_head = container_of(to_delayed_work(work),
+			struct mpath_head, remove_work);
+	struct module *drv_module = mpath_head->drv_module;
+
+	mpath_head->mpdt->remove_head(mpath_head);
+	module_put(drv_module);
+}
+
 void mpath_remove_disk(struct mpath_head *mpath_head)
 {
 	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
@@ -562,6 +607,9 @@ int mpath_alloc_head_disk(struct mpath_head *mpath_head,
 	mpath_head->disk->private_data = mpath_head;
 	mpath_head->disk->fops = &mpath_ops;
 
+	INIT_DELAYED_WORK(&mpath_head->remove_work, mpath_remove_head_work);
+	mpath_head->delayed_removal_secs = 0;
+
 	set_bit(GD_SUPPRESS_PART_SCAN, &mpath_head->disk->state);
 
 	return 0;
@@ -605,6 +653,47 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
 }
 EXPORT_SYMBOL_GPL(mpath_device_set_live);
 
+ssize_t mpath_delayed_removal_secs_show(struct mpath_head *mpath_head,
+					char *buf)
+{
+	int ret;
+
+	mutex_lock(&mpath_head->lock);
+	ret = sysfs_emit(buf, "%u\n", mpath_head->delayed_removal_secs);
+	mutex_unlock(&mpath_head->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mpath_delayed_removal_secs_show);
+
+ssize_t mpath_delayed_removal_secs_store(struct mpath_head *mpath_head,
+			const char *buf, size_t count)
+{
+	ssize_t ret;
+	int sec;
+
+	ret = kstrtouint(buf, 0, &sec);
+	if (ret < 0)
+		return ret;
+
+	mutex_lock(&mpath_head->lock);
+	mpath_head->delayed_removal_secs = sec;
+	if (sec)
+		set_bit(MPATH_HEAD_QUEUE_IF_NO_PATH, &mpath_head->flags);
+	else
+		clear_bit(MPATH_HEAD_QUEUE_IF_NO_PATH, &mpath_head->flags);
+	mutex_unlock(&mpath_head->lock);
+
+	/*
+	 * Ensure that update to MPATH_HEAD_QUEUE_IF_NO_PATH is seen
+	 * by its reader.
+	 */
+	mpath_synchronize(mpath_head);
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(mpath_delayed_removal_secs_store);
+
 void mpath_add_sysfs_link(struct mpath_head *mpath_head)
 {
 	struct device *target;
-- 
2.43.7

[PATCH v3 07/13] libmultipath: Add cdev support

[John Garry]

Add support to create a cdev multipath device. The functionality is much
the same as NVMe, where the cdev is created when a mpath device is set
live.

The driver must provide a mpath_head_template.cdev_ioctl callback to
actually handle the ioctl.

Structure mpath_generic_chr_fops would be used for setting the cdev fops in
the mpath_head_template.add_cdev callback.

NVMe cdev ioctl handler has special handling for NVMe controller commands.
In this case, the SRCU read lock is dropped before executing the ioctl.
For reference, see nvme_ns_head_ctrl_ioctl(). This makes having the SRCU
lock when calling not always possible. To handle this scenario, add template
callbacks .ioctl_begin and .ioctl_finish to be called around the before and
after the ioctl callback - if the .ioctl_begin returns data then we know
to drop the SRCU lock before calling the ioctl callback, and then later
call .ioctl_finish callback with that same data. For NVMe using
libmultipath, we would take a reference to the controller structure and
pass a pointer to the controller structure back in .ioctl_begin callback
and use that same data in the .ioctl_finish callback to put the reference
to the controller.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/multipath.h |  19 ++++++
 lib/multipath.c           | 138 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 157 insertions(+)

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
index 8e4b3fc197637..044061de3ecb9 100644
--- a/include/linux/multipath.h
+++ b/include/linux/multipath.h
@@ -4,8 +4,11 @@
 
 #include <linux/blkdev.h>
 #include <linux/blk-mq.h>
+#include <linux/cdev.h>
 #include <linux/srcu.h>
+#include <linux/io_uring/cmd.h>
 
+extern const struct file_operations mpath_chr_fops;
 extern const struct block_device_operations mpath_ops;
 
 enum mpath_iopolicy_e {
@@ -35,14 +38,27 @@ struct mpath_device {
 struct mpath_head_template {
 	bool (*available_path)(struct mpath_device *);
 	void (*remove_head)(struct mpath_head *);
+	int (*add_cdev)(struct mpath_head *);
+	void (*del_cdev)(struct mpath_head *);
 	bool (*is_disabled)(struct mpath_device *);
 	bool (*is_optimized)(struct mpath_device *);
+	void (*ioctl_begin)(struct mpath_device *, unsigned int cmd, void **);
+	void (*ioctl_finish)(void *opaque);
+	long (*cdev_ioctl)(struct mpath_device *, unsigned int cmd,
+				unsigned long arg, bool open_for_write);
+	int (*chr_uring_cmd)(struct mpath_device *,
+				struct io_uring_cmd *ioucmd,
+				unsigned int issue_flags);
+	int (*chr_uring_cmd_iopoll)(struct io_uring_cmd *ioucmd,
+				 struct io_comp_batch *iob,
+				 unsigned int poll_flags);
 	struct bio *(*clone_bio)(struct bio *);
 	const struct attribute_group **device_groups;
 };
 
 #define MPATH_HEAD_DISK_LIVE 			0
 #define MPATH_HEAD_QUEUE_IF_NO_PATH		1
+#define MPATH_HEAD_CDEV_LIVE			2
 
 struct mpath_head {
 	struct srcu_struct	srcu;
@@ -64,6 +80,9 @@ struct mpath_head {
 	unsigned int		delayed_removal_secs;
 	struct module		*drv_module;
 
+	struct cdev		cdev;
+	struct device		cdev_device;
+
 	unsigned long		flags;
 	struct gendisk		*disk;
 	struct work_struct	partition_scan_work;
diff --git a/lib/multipath.c b/lib/multipath.c
index 78f88b0664c78..6d2e1186a10f8 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -499,6 +499,131 @@ const struct block_device_operations mpath_ops = {
 };
 EXPORT_SYMBOL_GPL(mpath_ops);
 
+static int mpath_chr_open(struct inode *inode, struct file *file)
+{
+	struct cdev *cdev = file_inode(file)->i_cdev;
+	struct mpath_head *mpath_head =
+			container_of(cdev, struct mpath_head, cdev);
+
+	return mpath_get_head(mpath_head);
+}
+
+static int mpath_chr_release(struct inode *inode, struct file *file)
+{
+	struct cdev *cdev = file_inode(file)->i_cdev;
+	struct mpath_head *mpath_head =
+			container_of(cdev, struct mpath_head, cdev);
+
+	mpath_put_head(mpath_head);
+	return 0;
+}
+
+static long mpath_chr_ioctl(struct file *file, unsigned int cmd,
+		unsigned long arg)
+{
+	struct cdev *cdev = file_inode(file)->i_cdev;
+	struct mpath_head *mpath_head =
+			container_of(cdev, struct mpath_head, cdev);
+	struct mpath_device *mpath_device;
+	int srcu_idx, err = -EWOULDBLOCK;
+	void *unlocked_ioctl_data = NULL;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (!mpath_device)
+		goto out_unlock;
+	if (mpath_head->mpdt->ioctl_begin)
+		mpath_head->mpdt->ioctl_begin(mpath_device, cmd,
+					&unlocked_ioctl_data);
+	if (unlocked_ioctl_data)
+		srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+	err = mpath_head->mpdt->cdev_ioctl(mpath_device, cmd, arg,
+					file->f_mode & FMODE_WRITE);
+	if (unlocked_ioctl_data) {
+		mpath_head->mpdt->ioctl_finish(unlocked_ioctl_data);
+		return err;
+	}
+
+out_unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+	return err;
+}
+
+static int mpath_chr_uring_cmd(struct io_uring_cmd *ioucmd,
+		unsigned int issue_flags)
+{
+	struct cdev *cdev = file_inode(ioucmd->file)->i_cdev;
+	struct mpath_head *mpath_head =
+			container_of(cdev, struct mpath_head, cdev);
+	struct mpath_device *mpath_device;
+	/* error code copied from nvme_ns_head_chr_uring_cmd */
+	int srcu_idx, ret = -EINVAL;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+
+	if (!mpath_device)
+		goto out_unlock;
+
+	if (!mpath_head->mpdt->chr_uring_cmd) {
+		ret = -EOPNOTSUPP;
+		goto out_unlock;
+	}
+
+	ret = mpath_head->mpdt->chr_uring_cmd(mpath_device, ioucmd,
+			issue_flags);
+out_unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+	return ret;
+}
+
+static int mpath_chr_uring_cmd_iopoll(struct io_uring_cmd *ioucmd,
+				 struct io_comp_batch *iob,
+				 unsigned int poll_flags)
+{
+	struct cdev *cdev = file_inode(ioucmd->file)->i_cdev;
+	struct mpath_head *mpath_head =
+			container_of(cdev, struct mpath_head, cdev);
+
+	if (!mpath_head->mpdt->chr_uring_cmd_iopoll)
+		return -EOPNOTSUPP;
+
+	return mpath_head->mpdt->chr_uring_cmd_iopoll(ioucmd, iob, poll_flags);
+}
+
+const struct file_operations mpath_chr_fops = {
+	.owner		= THIS_MODULE,
+	.open		= mpath_chr_open,
+	.release	= mpath_chr_release,
+	.unlocked_ioctl	= mpath_chr_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
+	.uring_cmd	= mpath_chr_uring_cmd,
+	.uring_cmd_iopoll = mpath_chr_uring_cmd_iopoll,
+};
+EXPORT_SYMBOL_GPL(mpath_chr_fops);
+
+static void mpath_head_add_cdev(struct mpath_head *mpath_head)
+{
+	if (!mpath_head->mpdt->add_cdev)
+		return;
+
+	if (mpath_head->mpdt->add_cdev(mpath_head)) {
+		dev_err(disk_to_dev(mpath_head->disk),
+			"Unable to create the cdev\n");
+		return;
+	}
+	set_bit(MPATH_HEAD_CDEV_LIVE, &mpath_head->flags);
+}
+
+static void mpath_head_del_cdev(struct mpath_head *mpath_head)
+{
+	if (!mpath_head->mpdt->del_cdev)
+		return;
+
+	if (test_and_clear_bit(MPATH_HEAD_CDEV_LIVE, &mpath_head->flags))
+		mpath_head->mpdt->del_cdev(mpath_head);
+}
+
 static void multipath_partition_scan_work(struct work_struct *work)
 {
 	struct mpath_head *mpath_head =
@@ -574,6 +699,7 @@ void mpath_remove_disk(struct mpath_head *mpath_head)
 		 */
 		mpath_schedule_requeue_work(mpath_head);
 
+		mpath_head_del_cdev(mpath_head);
 		mpath_synchronize(mpath_head);
 		del_gendisk(disk);
 	}
@@ -600,6 +726,16 @@ int mpath_alloc_head_disk(struct mpath_head *mpath_head,
 	    !mpath_head->iopolicy)
 		return -EINVAL;
 
+	/* limited sanity checks on the template */
+	if (!mpath_head->mpdt->ioctl_begin ^ !mpath_head->mpdt->ioctl_finish)
+		return -EINVAL;
+
+	if (!mpath_head->mpdt->add_cdev ^ !mpath_head->mpdt->del_cdev)
+		return -EINVAL;
+
+	if (!mpath_head->mpdt->add_cdev ^ !mpath_head->mpdt->cdev_ioctl)
+		return -EINVAL;
+
 	mpath_head->disk = blk_alloc_disk(lim, numa_node);
 	if (IS_ERR(mpath_head->disk))
 		return PTR_ERR(mpath_head->disk);
@@ -632,6 +768,8 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
 			clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags);
 			return;
 		}
+
+		mpath_head_add_cdev(mpath_head);
 		queue_work(mpath_wq, &mpath_head->partition_scan_work);
 	}
 
-- 
2.43.7

[PATCH v3 08/13] libmultipath: Add sysfs helpers

[John Garry]

Add helpers for driver sysfs code for the following functionality:
- get/set iopolicy with mpath_iopolicy_store() and mpath_iopolicy_show()
- show device path per NUMA node
- "multipath" attribute group, equivalent to nvme_ns_mpath_attr_group
- device groups attribute array, similar to nvme_ns_attr_groups but not
  containing NVMe members.

Note that mpath_iopolicy_store() has a update callback to allow same
functionality as nvme_subsys_iopolicy_update() be run for clearing paths.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/multipath.h |  6 +++
 lib/multipath.c           | 95 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
index 044061de3ecb9..a335ea9885110 100644
--- a/include/linux/multipath.h
+++ b/include/linux/multipath.h
@@ -10,6 +10,8 @@
 
 extern const struct file_operations mpath_chr_fops;
 extern const struct block_device_operations mpath_ops;
+extern const struct attribute_group mpath_attr_group;
+extern const struct attribute_group *mpath_device_groups[];
 
 enum mpath_iopolicy_e {
 	MPATH_IOPOLICY_NUMA,
@@ -142,6 +144,10 @@ int mpath_alloc_head_disk(struct mpath_head *mpath_head,
 			struct queue_limits *lim, int numa_node);
 void mpath_device_set_live(struct mpath_device *mpath_device);
 bool mpath_can_remove_head(struct mpath_head *mpath_head);
+ssize_t mpath_numa_nodes_show(struct mpath_device *mpath_device, char *buf);
+ssize_t mpath_iopolicy_show(enum mpath_iopolicy_e *iopolicy, char *buf);
+bool mpath_iopolicy_store(enum mpath_iopolicy_e *iopolicy,
+			const char *buf, size_t count);
 ssize_t mpath_delayed_removal_secs_show(struct mpath_head *mpath_head,
 			char *buf);
 ssize_t mpath_delayed_removal_secs_store(struct mpath_head *mpath_head,
diff --git a/lib/multipath.c b/lib/multipath.c
index 6d2e1186a10f8..4945f2d847fbf 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -791,6 +791,101 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
 }
 EXPORT_SYMBOL_GPL(mpath_device_set_live);
 
+static struct attribute dummy_attr = {
+	.name = "dummy",
+};
+
+static struct attribute *mpath_attrs[] = {
+	&dummy_attr,
+	NULL
+};
+
+static bool multipath_sysfs_group_visible(struct kobject *kobj)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct gendisk *disk = dev_to_disk(dev);
+
+	return is_mpath_disk(disk);
+}
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(multipath_sysfs)
+
+const struct attribute_group mpath_attr_group = {
+	.name           = "multipath",
+	.attrs		= mpath_attrs,
+	.is_visible     = SYSFS_GROUP_VISIBLE(multipath_sysfs),
+};
+EXPORT_SYMBOL_GPL(mpath_attr_group);
+
+const struct attribute_group *mpath_device_groups[] = {
+	&mpath_attr_group,
+	NULL
+};
+EXPORT_SYMBOL_GPL(mpath_device_groups);
+
+ssize_t mpath_iopolicy_show(enum mpath_iopolicy_e *iopolicy, char *buf)
+{
+	return sysfs_emit(buf, "%s\n",
+		mpath_iopolicy_names[READ_ONCE(*iopolicy)]);
+}
+EXPORT_SYMBOL_GPL(mpath_iopolicy_show);
+
+static void mpath_iopolicy_update(enum mpath_iopolicy_e *iopolicy,
+		int new)
+{
+	int old = READ_ONCE(*iopolicy);
+
+	if (old == new)
+		return;
+
+	WRITE_ONCE(*iopolicy, new);
+
+	pr_info("iopolicy changed from %s to %s\n",
+		mpath_iopolicy_names[old],
+		mpath_iopolicy_names[new]);
+}
+
+bool mpath_iopolicy_store(enum mpath_iopolicy_e *iopolicy,
+				const char *buf, size_t count)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mpath_iopolicy_names); i++) {
+		if (sysfs_streq(buf, mpath_iopolicy_names[i])) {
+			mpath_iopolicy_update(iopolicy, i);
+			return true;
+		}
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(mpath_iopolicy_store);
+
+ssize_t mpath_numa_nodes_show(struct mpath_device *mpath_device, char *buf)
+{
+	struct mpath_head *mpath_head = mpath_device->mpath_head;
+	int node, srcu_idx;
+	nodemask_t numa_nodes;
+	struct mpath_device *current_mpath_dev;
+
+	if (mpath_read_iopolicy(mpath_head) != MPATH_IOPOLICY_NUMA)
+		return 0;
+
+	nodes_clear(numa_nodes);
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	for_each_node(node) {
+		current_mpath_dev =
+			srcu_dereference(mpath_head->current_path[node],
+				&mpath_head->srcu);
+		if (current_mpath_dev == mpath_device)
+			node_set(node, numa_nodes);
+	}
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return sysfs_emit(buf, "%*pbl\n", nodemask_pr_args(&numa_nodes));
+}
+EXPORT_SYMBOL_GPL(mpath_numa_nodes_show);
+
 ssize_t mpath_delayed_removal_secs_show(struct mpath_head *mpath_head,
 					char *buf)
 {
-- 
2.43.7

[PATCH v3 09/13] libmultipath: Add PR support

[John Garry]

Add support for persistent reservations.

Effectively all that is done here is that a multipath version of pr_ops is
created which calls into the bdev fops callback for the mpath_device
selected.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/multipath.h |   1 +
 lib/multipath.c           | 182 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 183 insertions(+)

diff --git a/include/linux/multipath.h b/include/linux/multipath.h
index a335ea9885110..74ff2e4d33e69 100644
--- a/include/linux/multipath.h
+++ b/include/linux/multipath.h
@@ -5,6 +5,7 @@
 #include <linux/blkdev.h>
 #include <linux/blk-mq.h>
 #include <linux/cdev.h>
+#include <linux/pr.h>
 #include <linux/srcu.h>
 #include <linux/io_uring/cmd.h>
 
diff --git a/lib/multipath.c b/lib/multipath.c
index 4945f2d847fbf..cec7047ce7b9b 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -491,11 +491,193 @@ static void mpath_bdev_release(struct gendisk *disk)
 	mpath_put_head(mpath_head);
 }
 
+static int mpath_pr_register(struct block_device *bdev, u64 old_key,
+			u64 new_key, unsigned int flags)
+{
+	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
+	struct mpath_device *mpath_device;
+	int srcu_idx, ret = -EWOULDBLOCK;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		const struct pr_ops *ops = mpath_device->disk->fops->pr_ops;
+
+		if (!ops || !ops->pr_register) {
+			ret = -EOPNOTSUPP;
+			goto unlock;
+		}
+		ret = ops->pr_register(mpath_device->disk->part0,
+				old_key, new_key, flags);
+	}
+unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
+static int mpath_pr_reserve(struct block_device *bdev, u64 key,
+		enum pr_type type, unsigned flags)
+{
+	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
+	struct mpath_device *mpath_device;
+	int srcu_idx, ret = -EWOULDBLOCK;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		const struct pr_ops *ops = mpath_device->disk->fops->pr_ops;
+
+		if (!ops || !ops->pr_reserve) {
+			ret = -EOPNOTSUPP;
+			goto unlock;
+		}
+		ret = ops->pr_reserve(mpath_device->disk->part0, key,
+				type, flags);
+	}
+unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
+static int mpath_pr_release(struct block_device *bdev, u64 key,
+				enum pr_type type)
+{
+	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
+	struct mpath_device *mpath_device;
+	int srcu_idx, ret = -EWOULDBLOCK;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		const struct pr_ops *ops = mpath_device->disk->fops->pr_ops;
+
+		if (!ops || !ops->pr_release) {
+			ret = -EOPNOTSUPP;
+			goto unlock;
+		}
+		ret = ops->pr_release(mpath_device->disk->part0, key, type);
+	}
+unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
+static int mpath_pr_preempt(struct block_device *bdev, u64 old, u64 new,
+		enum pr_type type, bool abort)
+{
+	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
+	struct mpath_device *mpath_device;
+	int srcu_idx, ret = -EWOULDBLOCK;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		const struct pr_ops *ops = mpath_device->disk->fops->pr_ops;
+
+		if (!ops || !ops->pr_preempt) {
+			ret = -EOPNOTSUPP;
+			goto unlock;
+		}
+		ret = ops->pr_preempt(mpath_device->disk->part0, old,
+				new, type, abort);
+	}
+unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
+static int mpath_pr_clear(struct block_device *bdev, u64 key)
+{
+	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
+	struct mpath_device *mpath_device;
+	int srcu_idx, ret = -EWOULDBLOCK;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		const struct pr_ops *ops = mpath_device->disk->fops->pr_ops;
+
+		if (!ops || !ops->pr_clear) {
+			ret = -EOPNOTSUPP;
+			goto unlock;
+		}
+		ret = ops->pr_clear(mpath_device->disk->part0, key);
+	}
+unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
+static int mpath_pr_read_keys(struct block_device *bdev,
+		struct pr_keys *keys_info)
+{
+	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
+	struct mpath_device *mpath_device;
+	int srcu_idx, ret = -EWOULDBLOCK;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		const struct pr_ops *ops = mpath_device->disk->fops->pr_ops;
+
+		if (!ops || !ops->pr_read_keys) {
+			ret = -EOPNOTSUPP;
+			goto unlock;
+		}
+		ret = ops->pr_read_keys(mpath_device->disk->part0, keys_info);
+	}
+unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
+static int mpath_pr_read_reservation(struct block_device *bdev,
+		struct pr_held_reservation *resv)
+{
+	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
+	struct mpath_device *mpath_device;
+	int srcu_idx, ret = -EWOULDBLOCK;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		const struct pr_ops *ops = mpath_device->disk->fops->pr_ops;
+
+		if (!ops || !ops->pr_read_reservation) {
+			ret = -EOPNOTSUPP;
+			goto unlock;
+		}
+		ret = ops->pr_read_reservation(mpath_device->disk->part0,
+				resv);
+	}
+unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
+static const struct pr_ops mpath_pr_ops = {
+	.pr_register	= mpath_pr_register,
+	.pr_reserve	= mpath_pr_reserve,
+	.pr_release	= mpath_pr_release,
+	.pr_preempt	= mpath_pr_preempt,
+	.pr_clear	= mpath_pr_clear,
+	.pr_read_keys	= mpath_pr_read_keys,
+	.pr_read_reservation = mpath_pr_read_reservation,
+};
+
 const struct block_device_operations mpath_ops = {
 	.owner          = THIS_MODULE,
 	.open		= mpath_bdev_open,
 	.release	= mpath_bdev_release,
 	.submit_bio	= mpath_bdev_submit_bio,
+	.pr_ops		= &mpath_pr_ops,
 };
 EXPORT_SYMBOL_GPL(mpath_ops);
 
-- 
2.43.7

[PATCH v3 10/13] libmultipath: Add mpath_bdev_report_zones()

[John Garry]

Add a multipath handler for block_device_operations.report_zones

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 lib/multipath.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/lib/multipath.c b/lib/multipath.c
index cec7047ce7b9b..d335074eb5bcd 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -44,6 +44,30 @@ int mpath_get_iopolicy(char *buf, int iopolicy)
 }
 EXPORT_SYMBOL_GPL(mpath_get_iopolicy);
 
+#ifdef CONFIG_BLK_DEV_ZONED
+static int mpath_bdev_report_zones(struct gendisk *disk, sector_t sector,
+		unsigned int nr_zones, struct blk_report_zones_args *args)
+{
+	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
+	struct mpath_device *mpath_device;
+	int srcu_idx, ret = -EWOULDBLOCK;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		if (mpath_device->disk->fops->report_zones)
+			ret = mpath_device->disk->fops->report_zones
+				(mpath_device->disk, sector, nr_zones, args);
+		else
+			ret = -EOPNOTSUPP;
+	}
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+	return ret;
+}
+#else
+#define mpath_bdev_report_zones	NULL
+#endif /* CONFIG_BLK_DEV_ZONED */
+
 void mpath_synchronize(struct mpath_head *mpath_head)
 {
 	synchronize_srcu(&mpath_head->srcu);
@@ -677,6 +701,7 @@ const struct block_device_operations mpath_ops = {
 	.open		= mpath_bdev_open,
 	.release	= mpath_bdev_release,
 	.submit_bio	= mpath_bdev_submit_bio,
+	.report_zones	= mpath_bdev_report_zones,
 	.pr_ops		= &mpath_pr_ops,
 };
 EXPORT_SYMBOL_GPL(mpath_ops);
-- 
2.43.7

[PATCH v3 11/13] libmultipath: Add support for block device IOCTL

[John Garry]

Add mpath_bdev_ioctl() as a multipath block device IOCTL handler. This
handler calls into the mpath_device bdev fops handler.

Like what is done for cdev IOCTL handler, use .ioctl_begin and
.ioctl_finish methods to know when the until the SRCU read lock - this is
for special NVMe controller IOCTL handling.

The .compat_ioctl handler is given the standard handler.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 lib/multipath.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/lib/multipath.c b/lib/multipath.c
index d335074eb5bcd..4e4b347875500 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -515,6 +515,38 @@ static void mpath_bdev_release(struct gendisk *disk)
 	mpath_put_head(mpath_head);
 }
 
+static int mpath_bdev_ioctl(struct block_device *bdev, blk_mode_t mode,
+		    unsigned int cmd, unsigned long arg)
+{
+	struct gendisk *disk = bdev->bd_disk;
+	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
+	struct mpath_device *mpath_device;
+	int srcu_idx, err;
+	void *unlocked_ioctl_data = NULL;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (!mpath_device) {
+		err = -EWOULDBLOCK;
+		goto out_unlock;
+	}
+
+	if (mpath_head->mpdt->ioctl_begin)
+		mpath_head->mpdt->ioctl_begin(mpath_device, cmd,
+					&unlocked_ioctl_data);
+	if (unlocked_ioctl_data)
+		srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+	err = mpath_device->disk->fops->ioctl(
+			mpath_device->disk->part0, mode, cmd, arg);
+	if (unlocked_ioctl_data) {
+		mpath_head->mpdt->ioctl_finish(unlocked_ioctl_data);
+		return err;
+	}
+out_unlock:
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+	return err;
+}
+
 static int mpath_pr_register(struct block_device *bdev, u64 old_key,
 			u64 new_key, unsigned int flags)
 {
@@ -701,6 +733,8 @@ const struct block_device_operations mpath_ops = {
 	.open		= mpath_bdev_open,
 	.release	= mpath_bdev_release,
 	.submit_bio	= mpath_bdev_submit_bio,
+	.ioctl		= mpath_bdev_ioctl,
+	.compat_ioctl	= blkdev_compat_ptr_ioctl,
 	.report_zones	= mpath_bdev_report_zones,
 	.pr_ops		= &mpath_pr_ops,
 };
-- 
2.43.7

[PATCH v3 12/13] libmultipath: Add mpath_bdev_getgeo()

[John Garry]

Add mpath_bdev_getgeo() as a multipath block device .getgeo handler.

Here we just redirect into the selected mpath_device disk fops->getgeo
handler.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 lib/multipath.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lib/multipath.c b/lib/multipath.c
index 4e4b347875500..26d9b17998d66 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -547,6 +547,26 @@ static int mpath_bdev_ioctl(struct block_device *bdev, blk_mode_t mode,
 	return err;
 }
 
+static int mpath_bdev_getgeo(struct gendisk *disk, struct hd_geometry *geo)
+{
+	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
+	int srcu_idx, ret = -EWOULDBLOCK;
+	struct mpath_device *mpath_device;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		if (mpath_device->disk->fops->getgeo)
+			ret = mpath_device->disk->fops->getgeo(
+					mpath_device->disk, geo);
+		else
+			ret = -ENOTTY; /* See blkdev_getgeo */
+	}
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
 static int mpath_pr_register(struct block_device *bdev, u64 old_key,
 			u64 new_key, unsigned int flags)
 {
@@ -736,6 +756,7 @@ const struct block_device_operations mpath_ops = {
 	.ioctl		= mpath_bdev_ioctl,
 	.compat_ioctl	= blkdev_compat_ptr_ioctl,
 	.report_zones	= mpath_bdev_report_zones,
+	.getgeo		= mpath_bdev_getgeo,
 	.pr_ops		= &mpath_pr_ops,
 };
 EXPORT_SYMBOL_GPL(mpath_ops);
-- 
2.43.7

[PATCH v3 13/13] libmultipath: Add mpath_bdev_get_unique_id()

[John Garry]

Add mpath_bdev_get_unique_id() as a multipath block device .get_unique_id
handler.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 lib/multipath.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/lib/multipath.c b/lib/multipath.c
index 26d9b17998d66..fba1716a165c7 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -515,6 +515,27 @@ static void mpath_bdev_release(struct gendisk *disk)
 	mpath_put_head(mpath_head);
 }
 
+static int mpath_bdev_get_unique_id(struct gendisk *disk, u8 id[16],
+    enum blk_unique_id type)
+{
+	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
+	int srcu_idx, ret = -EWOULDBLOCK;
+	struct mpath_device *mpath_device;
+
+	srcu_idx = srcu_read_lock(&mpath_head->srcu);
+	mpath_device = mpath_find_path(mpath_head);
+	if (mpath_device) {
+		if (mpath_device->disk->fops->get_unique_id)
+			ret = mpath_device->disk->fops->get_unique_id(
+					mpath_device->disk, id, type);
+		else
+			ret = 0; /* referencing __dm_get_unique_id() */
+	}
+	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
+
+	return ret;
+}
+
 static int mpath_bdev_ioctl(struct block_device *bdev, blk_mode_t mode,
 		    unsigned int cmd, unsigned long arg)
 {
@@ -755,6 +776,7 @@ const struct block_device_operations mpath_ops = {
 	.submit_bio	= mpath_bdev_submit_bio,
 	.ioctl		= mpath_bdev_ioctl,
 	.compat_ioctl	= blkdev_compat_ptr_ioctl,
+	.get_unique_id	= mpath_bdev_get_unique_id,
 	.report_zones	= mpath_bdev_report_zones,
 	.getgeo		= mpath_bdev_getgeo,
 	.pr_ops		= &mpath_pr_ops,
-- 
2.43.7

Re: [PATCH v3 01/13] libmultipath: Add initial framework

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A race condition between mpath_put_head() and mpath_head_uninit() leads to a use-after-free vulnerability when accessing mpath_head for cleanup.
--

commit 594f063adb51503b6e7af6741526b8d738fee5ba
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add initial framework

This commit adds the initial framework for libmultipath to support path
management, path selection, and failover handling for multipath-capable
block drivers. It introduces the mpath_head structure to manage available
paths and the multipath gendisk.

> diff --git a/lib/multipath.c b/lib/multipath.c
> new file mode 100644
> index 0000000000000..009d4bb875c6f
> --- /dev/null
> +++ b/lib/multipath.c

[ ... ]

> +void mpath_put_head(struct mpath_head *mpath_head)
> +{
> +	refcount_t *refcount = &mpath_head->refcount;
> +
> +	if (refcount_dec_and_test(refcount)) {

[Severity: High]
Could this introduce a use-after-free regression on the mpath_head structure?

If a teardown thread calls mpath_head_uninit() concurrently, it blocks in
wait_var_event() waiting for the refcount to drop to 0.

Once refcount_dec_and_test() drops the refcount to 0 here, the condition
!refcount_read(refcount) in mpath_head_uninit() can evaluate to true
immediately. The uninit thread could then wake up, return, and free mpath_head
before this thread completes mpath_head_cleanup() and wake_up_var().

> +		mpath_head_cleanup(mpath_head);
> +		wake_up_var(refcount);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(mpath_put_head);
> +
> +void mpath_head_uninit(struct mpath_head *mpath_head)
> +{
> +	refcount_t *refcount = &mpath_head->refcount;
> +
> +	if (refcount_dec_and_test(refcount)) {
> +		mpath_head_cleanup(mpath_head);
> +	} else {
> +		wait_var_event(refcount, !refcount_read(refcount));
> +	}
> +}
> +EXPORT_SYMBOL_GPL(mpath_head_uninit);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=1

Re: [PATCH v3 08/13] libmultipath: Add sysfs helpers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mpath_iopolicy_store() does not clear cached paths when the I/O policy changes, contradicting the commit message and leaving stale I/O routing state.
- [Low] Unused 'count' parameter in mpath_iopolicy_store().
--

commit e168713423f62d0ecfcf0e0b0c9ed72180e1f19b
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add sysfs helpers

This commit adds sysfs helpers for the libmultipath driver sysfs code. It
includes helpers to get/set I/O policies, show device paths per NUMA node,
and handle attribute groups.

>     Note that mpath_iopolicy_store() has a update callback to allow same
>     functionality as nvme_subsys_iopolicy_update() be run for clearing paths.

[Severity: High]
Does this description match the implementation? The code doesn't seem to provide
the callback parameter mentioned here.


> diff --git a/lib/multipath.c b/lib/multipath.c
> index 6d2e1186a10f8..4945f2d847fbf 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
[ ... ]
> +static void mpath_iopolicy_update(enum mpath_iopolicy_e *iopolicy,
> +		int new)
> +{
> +	int old = READ_ONCE(*iopolicy);
> +
> +	if (old == new)
> +		return;
> +
> +	WRITE_ONCE(*iopolicy, new);
> +
> +	pr_info("iopolicy changed from %s to %s\n",
> +		mpath_iopolicy_names[old],
> +		mpath_iopolicy_names[new]);
> +}
> +
> +bool mpath_iopolicy_store(enum mpath_iopolicy_e *iopolicy,
> +				const char *buf, size_t count)
                                                 ^^^^^^^^^^^^

[Severity: Low]
This isn't a bug, but is the count parameter needed here? It doesn't
appear to be used in the function body since sysfs_streq() is used for the
string comparison.

> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mpath_iopolicy_names); i++) {
> +		if (sysfs_streq(buf, mpath_iopolicy_names[i])) {
> +			mpath_iopolicy_update(iopolicy, i);

[Severity: High]
Does changing the I/O policy via sysfs leave stale paths cached under the old
policy rules?

Neither mpath_iopolicy_store() nor mpath_iopolicy_update() accept a callback
function pointer, nor do they clear the paths themselves. As a result, drivers
adopting this API have no mechanism to hook into the change to acquire
subsystem locks or clear active paths.

> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=8

Re: [PATCH v3 06/13] libmultipath: Add delayed removal support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Repeated calls to `mpath_can_remove_head()` leak module references when removal work is already pending.
- [High] Race condition between `mpath_add_device()`, `mpath_can_remove_head()`, and `mpath_remove_head_work()` leads to use-after-free of mpath_head.
- [High] Missing cancellation of delayed removal work on `mpath_head` teardown results in Use-After-Free.
- [High] `mpath_head->drv_module` is never initialized by the subsystem API, leading to a silent failure to pin the driver module.
- [Medium] Disabling delayed removal via sysfs fails to cancel already pending removal work, leading to unexpected device removal.
- [Low] Incompatible pointer type passed to `kstrtouint()` causes compiler warnings.
- [Low] Integer overflow when calculating the delay for `mod_delayed_work()` results in severely shortened delays.
--

commit c00bb95222f826ebc3e8dbd55ff6884abf84858e
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add delayed removal support
    
Add support for delayed removal, same as exists for NVMe.
    
The purpose of this feature is to keep the multipath disk and cdev present
for intermittent periods of no available path.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index 007aa34796569..78f88b0664c78 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -61,6 +61,8 @@ void mpath_add_device(struct mpath_device *mpath_device,
>  	mutex_lock(&mpath_head->lock);
>  	list_add_tail_rcu(&mpath_device->siblings, &mpath_head->dev_list);
>  	mutex_unlock(&mpath_head->lock);
> +	if (cancel_delayed_work(&mpath_head->remove_work))
> +		module_put(mpath_head->drv_module);
>  }

[Severity: High]
Does this code introduce a use-after-free race condition?

If the delayed removal timer is already executing, cancel_delayed_work()
returns false. 

mpath_add_device() will successfully add the device, but the
already-executing mpath_remove_head_work() unconditionally calls
mpath_head->mpdt->remove_head() without checking for newly added devices.

Could the newly added device access the destroyed mpath_head?

[ ... ]

> @@ -518,6 +530,39 @@ static void mpath_requeue_work(struct work_struct *work)
> +bool mpath_can_remove_head(struct mpath_head *mpath_head)
> +{
> +	bool remove = false;
> +
> +	mutex_lock(&mpath_head->lock);
> +	/*
> +	 * Ensure that no one could remove this module while the head
> +	 * remove work is pending.
> +	 */
> +	if (mpath_head_queue_if_no_path(mpath_head) &&
> +		try_module_get(mpath_head->drv_module)) {

[Severity: High]
Is mpath_head->drv_module initialized anywhere in the subsystem API?

If it remains uninitialized (e.g., NULL from zero-allocated structures),
try_module_get(NULL) will silently succeed. 

Could this lead to a failure to pin the driver module, potentially resulting
in a crash if the module is unloaded while the timer is pending?

> +
> +		mod_delayed_work(mpath_wq, &mpath_head->remove_work,
> +				mpath_head->delayed_removal_secs * HZ);

[Severity: High]
Does this unconditionally increment the module reference count even when the
timer is already pending?

If mpath_can_remove_head() is called multiple times, try_module_get()
will increment the reference count on each call. 

However, mod_delayed_work() merely modifies the existing timer. When the
timer eventually fires, mpath_remove_head_work() calls module_put() only
once, leaking the additional module references and preventing unloading.

[Severity: Low]
Does this multiplication overflow on 32-bit systems?

mpath_head->delayed_removal_secs is an unsigned int, and HZ is typically
an integer. 

Could their product overflow 32-bit arithmetic before being promoted to
unsigned long for mod_delayed_work(), leading to shortened delays?

> +	} else {
> +		remove = true;
> +	}
> +
> +	mutex_unlock(&mpath_head->lock);
> +	return remove;
> +}
> +EXPORT_SYMBOL_GPL(mpath_can_remove_head);
> +
> +static void mpath_remove_head_work(struct work_struct *work)
> +{
> +	struct mpath_head *mpath_head = container_of(to_delayed_work(work),
> +			struct mpath_head, remove_work);
> +	struct module *drv_module = mpath_head->drv_module;
> +
> +	mpath_head->mpdt->remove_head(mpath_head);
> +	module_put(drv_module);
> +}

[ ... ]

> @@ -562,6 +607,9 @@ int mpath_alloc_head_disk(struct mpath_head *mpath_head,
>  	mpath_head->disk->private_data = mpath_head;
>  	mpath_head->disk->fops = &mpath_ops;
>  
> +	INIT_DELAYED_WORK(&mpath_head->remove_work, mpath_remove_head_work);
> +	mpath_head->delayed_removal_secs = 0;
> +
>  	set_bit(GD_SUPPRESS_PART_SCAN, &mpath_head->disk->state);

[Severity: High]
Is the delayed removal work reliably canceled during final teardown?

During explicit device teardown (like a driver unbind), functions like
mpath_head_cleanup() are called to free resources. 

Since this patch doesn't add a cancel_delayed_work_sync() call to the
cleanup path, could an active timer fire after the mpath_head has been
freed, leading to a use-after-free?

[ ... ]

> @@ -605,6 +653,47 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
> +ssize_t mpath_delayed_removal_secs_store(struct mpath_head *mpath_head,
> +			const char *buf, size_t count)
> +{
> +	ssize_t ret;
> +	int sec;
> +
> +	ret = kstrtouint(buf, 0, &sec);

[Severity: Low]
Does passing an int pointer to kstrtouint() cause compiler warnings?

sec is declared as an int, but kstrtouint() strictly expects an unsigned
int *. 

Could this cause an incompatible pointer type warning and potential undefined
behavior on architectures with differing int/unsigned int layouts?

> +	if (ret < 0)
> +		return ret;
> +
> +	mutex_lock(&mpath_head->lock);
> +	mpath_head->delayed_removal_secs = sec;
> +	if (sec)
> +		set_bit(MPATH_HEAD_QUEUE_IF_NO_PATH, &mpath_head->flags);
> +	else
> +		clear_bit(MPATH_HEAD_QUEUE_IF_NO_PATH, &mpath_head->flags);

[Severity: Medium]
Should the active timer be canceled when delayed removal is disabled?

When a user writes 0 to disable the feature, this code clears the flag but
misses a call to cancel_delayed_work(&mpath_head->remove_work). 

If the timer is already pending, will it continue to run and eventually
destroy the head, contrary to the user's configuration?

> +	mutex_unlock(&mpath_head->lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=6

Re: [PATCH v3 03/13] libmultipath: Add path selection support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] `mpath_round_robin_path` enters an infinite loop if the current cached path (`old`) is concurrently removed.
- [High] Unsafe usage of `list_is_singular()` on an RCU list leads to data races and routes I/O to a removed path.
- [High] `mpath_numa_path` fails to verify if the cached path is disabled before returning it, potentially routing I/O to a disabled path.
- [High] `mpath_alloc_head_disk` leaves an ERR_PTR in `mpath_head->disk` on failure, causing cleanup paths to crash.
- [Medium] NULL pointer dereference in `mpath_queue_depth_path` if a device does not provide `nr_active`.
- [Medium] The `.get_access_state` callback is missing from `struct mpath_head_template`, violating the documented API intent.
--

commit e941f797884516dd9568e0b668984eb3c895248a
Author: John Garry <john.g.garry@oracle.com>

    libmultipath: Add path selection support
    
    Add code for path selection.

>     NVMe ANA is abstracted into enum mpath_access_state. The motivation here is
>     so that SCSI ALUA can be used. Callbacks .is_disabled, .is_optimized,
>     .get_access_state are added to get the path access state.

[Severity: Medium]
The commit message mentions adding a .get_access_state callback, but it appears
to be missing from struct mpath_head_template. Is this intentional, or should
it be included in the struct definition to avoid requiring drivers to directly
modify the access_state field?

[ ... ]
> diff --git a/lib/multipath.c b/lib/multipath.c
> index 79be84d3d4f75..21f7ffdb22d60 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
[ ... ]
> +static struct mpath_device *mpath_round_robin_path(
> +				struct mpath_head *mpath_head)
> +{
> +	struct mpath_device *mpath_device, *found = NULL;
> +	int node = numa_node_id();
> +	enum mpath_access_state access_state_old;
> +	struct mpath_device *old =
> +			srcu_dereference(mpath_head->current_path[node],
> +				&mpath_head->srcu);
> +
> +	if (unlikely(!old))
> +		return __mpath_find_path(mpath_head, node);
> +
> +	if (list_is_singular(&mpath_head->dev_list)) {
> +		if (mpath_path_is_disabled(mpath_head, old))
> +			return NULL;
> +		return old;
> +	}

[Severity: High]
Could using list_is_singular() here lead to data races? Since this list is
managed with RCU, and list_is_singular() evaluates head->next == head->prev,
it might read inconsistent prev pointers during concurrent updates. Also, if old
was concurrently removed and only one valid path remains, would this return
the removed old path instead of the new remaining path?

> +
> +	for (mpath_device = mpath_next_dev(mpath_head, old);
> +	    mpath_device && mpath_device != old;
> +	    mpath_device = mpath_next_dev(mpath_head, mpath_device)) {

[Severity: High]
Does this loop terminate safely if old has been concurrently removed from the list?
If old is unlinked via list_del_rcu(), mpath_device != old will always be true.
Could this result in an infinite loop wrapping around the active elements
indefinitely, leading to a lockup if no path is MPATH_STATE_OPTIMIZED?

> +
> +		if (mpath_path_is_disabled(mpath_head, mpath_device))
> +			continue;
[ ... ]
> +static struct mpath_device *mpath_queue_depth_path(
> +				struct mpath_head *mpath_head)
> +{
> +	struct mpath_device *best_opt = NULL, *mpath_device;
> +	struct mpath_device *best_nonopt = NULL;
> +	unsigned int min_depth_opt = UINT_MAX, min_depth_nonopt = UINT_MAX;
> +	unsigned int depth;
> +
> +	list_for_each_entry_srcu(mpath_device, &mpath_head->dev_list, siblings,
> +				 srcu_read_lock_held(&mpath_head->srcu)) {
> +
> +		if (mpath_path_is_disabled(mpath_head, mpath_device))
> +			continue;
> +
> +		depth = atomic_read(mpath_device->nr_active);

[Severity: Medium]
Could this dereference a NULL pointer if a device was added without providing
an nr_active atomic counter during mpath_add_device()? If userspace sets the
I/O policy to 'queue-depth' via sysfs, would it crash here?

> +
> +		switch (mpath_device->access_state) {
[ ... ]
> +static struct mpath_device *mpath_numa_path(struct mpath_head *mpath_head)
> +{
> +	int node = numa_node_id();
> +	struct mpath_device *mpath_device;
> +
> +	mpath_device = srcu_dereference(mpath_head->current_path[node],
> +					&mpath_head->srcu);
> +	if (unlikely(!mpath_device))
> +		return __mpath_find_path(mpath_head, node);
> +	if (unlikely(!mpath_path_is_optimized(mpath_head, mpath_device)))
> +		return __mpath_find_path(mpath_head, node);
> +	return mpath_device;
> +}

[Severity: High]
Does this correctly validate the cached path before returning it? Unlike other
path selection functions, this omits checking mpath_path_is_disabled().
If a path transitions to a disabled state but remains marked as optimized, could
mpath_numa_path() incorrectly continue to route I/O to it?

[ ... ]
> @@ -103,7 +339,8 @@ EXPORT_SYMBOL_GPL(mpath_put_disk);
>  int mpath_alloc_head_disk(struct mpath_head *mpath_head,
>  			struct queue_limits *lim, int numa_node)
>  {
> -	if (!mpath_head->disk_groups || !mpath_head->parent)
> +	if (!mpath_head->disk_groups || !mpath_head->parent ||
> +	    !mpath_head->iopolicy)
>  		return -EINVAL;
>  
>  	mpath_head->disk = blk_alloc_disk(lim, numa_node);

[Severity: High]
If blk_alloc_disk() fails, it returns an ERR_PTR. If this is assigned directly
to mpath_head->disk and an error is returned without setting mpath_head->disk
back to NULL, could this cause subsequent cleanup paths (like mpath_put_disk())
to crash when they check if (mpath_head->disk) and attempt to put an ERR_PTR?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=3

Re: [PATCH v3 09/13] libmultipath: Add PR support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Calling Persistent Reservation (PR) operations on a multipath partition causes a NULL pointer dereference.
--

commit ed0ad48ae1e1adac4c8ee8809fa3bba6ef9f4ac5
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add PR support

Add support for persistent reservations.

Effectively all that is done here is that a multipath version of pr_ops is
created which calls into the bdev fops callback for the mpath_device
selected.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index 4945f2d847fbf..cec7047ce7b9b 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -491,11 +491,193 @@ static void mpath_bdev_release(struct gendisk *disk)
>  	mpath_put_head(mpath_head);
>  }
>  
> +static int mpath_pr_register(struct block_device *bdev, u64 old_key,
> +			u64 new_key, unsigned int flags)
> +{
> +	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
> +	struct mpath_device *mpath_device;
> +	int srcu_idx, ret = -EWOULDBLOCK;
> +
> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);

[Severity: Critical]
Will this cause a NULL pointer dereference if the persistent reservation
operation is called on a partition?

If the target_core_iblock backend is configured for PR passthrough on a
partition of a multipath device, bdev will be the partition's block_device.
Because dev_set_drvdata() is only called for the base disk's device,
dev_get_drvdata(&bdev->bd_device) will return NULL for partition devices.

Other functions in this file, like mpath_bdev_ioctl(), seem to handle this by
resolving the disk first using mpath_gendisk_to_head(bdev->bd_disk). Should
the PR callbacks do the same?

[ ... ]

> +static int mpath_pr_reserve(struct block_device *bdev, u64 key,
> +		enum pr_type type, unsigned flags)
> +{
> +	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
> +	struct mpath_device *mpath_device;
> +	int srcu_idx, ret = -EWOULDBLOCK;
> +
> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);

[Severity: Critical]
The same NULL pointer dereference risk applies here for partition devices, as
well as to mpath_pr_release(), mpath_pr_preempt(), mpath_pr_clear(),
mpath_pr_read_keys(), and mpath_pr_read_reservation() introduced below.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=9

Re: [PATCH v3 07/13] libmultipath: Add cdev support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Dropping the SRCU read lock during ioctl execution leaves the `mpath_device` pointer unprotected, causing a use-after-free when passed to `cdev_ioctl` or accessed.
- [High] Decoupled lifetime between the embedded `cdev` and `mpath_head` reference counting leads to Use-After-Free during VFS file open and release.
--

commit 9a6527083c29f0b9d58867aa52ca2ea700cf1168
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add cdev support

Add support to create a cdev multipath device. The functionality is much
the same as NVMe, where the cdev is created when a mpath device is set
live.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index 78f88b0664c78..6d2e1186a10f8 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c

[ ... ]

> +static int mpath_chr_open(struct inode *inode, struct file *file)
> +{
> +	struct cdev *cdev = file_inode(file)->i_cdev;
> +	struct mpath_head *mpath_head =
> +			container_of(cdev, struct mpath_head, cdev);
> +
> +	return mpath_get_head(mpath_head);
> +}

[Severity: High]
Is there a race condition with driver teardown here? 

If teardown occurs after the VFS has looked up the cdev but before this
function calls mpath_get_head() to increment the refcount, mpath_head could
be freed, potentially causing a use-after-free when this accesses the freed
memory.

> +
> +static int mpath_chr_release(struct inode *inode, struct file *file)
> +{
> +	struct cdev *cdev = file_inode(file)->i_cdev;
> +	struct mpath_head *mpath_head =
> +			container_of(cdev, struct mpath_head, cdev);
> +
> +	mpath_put_head(mpath_head);
> +	return 0;
> +}

[Severity: High]
Does this sequence cause the embedded cdev to be freed prematurely? 

When the last file descriptor is closed, VFS __fput() calls
mpath_chr_release(). If mpath_put_head() drops the mpath_head refcount to 0,
the mpath_head structure (and the embedded cdev) is freed. 

When this returns, VFS __fput() will subsequently call
cdev_put(inode->i_cdev). Could this result in a use-after-free on the now
freed cdev->kobj?

> +
> +static long mpath_chr_ioctl(struct file *file, unsigned int cmd,
> +		unsigned long arg)
> +{
> +	struct cdev *cdev = file_inode(file)->i_cdev;
> +	struct mpath_head *mpath_head =
> +			container_of(cdev, struct mpath_head, cdev);
> +	struct mpath_device *mpath_device;
> +	int srcu_idx, err = -EWOULDBLOCK;
> +	void *unlocked_ioctl_data = NULL;
> +
> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
> +	mpath_device = mpath_find_path(mpath_head);
> +	if (!mpath_device)
> +		goto out_unlock;
> +	if (mpath_head->mpdt->ioctl_begin)
> +		mpath_head->mpdt->ioctl_begin(mpath_device, cmd,
> +					&unlocked_ioctl_data);
> +	if (unlocked_ioctl_data)
> +		srcu_read_unlock(&mpath_head->srcu, srcu_idx);
> +	err = mpath_head->mpdt->cdev_ioctl(mpath_device, cmd, arg,
> +					file->f_mode & FMODE_WRITE);

[Severity: Critical]
Are we exposing the mpath_device pointer to a use-after-free here?

If unlocked_ioctl_data is present, the SRCU lock is dropped just before
calling cdev_ioctl(). Since the lock is dropped, a concurrent hot-unplug
removal process calling synchronize_srcu() might not wait for this ioctl to
finish and could free the mpath_device.

This would leave the mpath_device pointer unprotected, which is then
immediately passed into cdev_ioctl(). 

Also, does mpath_bdev_ioctl() suffer from this exact same problem when it
calls mpath_device->disk->fops->ioctl() after potentially dropping its SRCU
lock?

> +	if (unlocked_ioctl_data) {
> +		mpath_head->mpdt->ioctl_finish(unlocked_ioctl_data);
> +		return err;
> +	}
> +
> +out_unlock:
> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
> +	return err;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=7

Re: [PATCH v3 04/13] libmultipath: Add bio handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] TOCTOU race in `mpath_remove_disk` can strand a bio on `requeue_list`, causing `del_gendisk` to deadlock.
- [Medium] The `trace_block_bio_remap` tracepoint is called with the wrong device argument for `old_dev`, causing the trace event to emit identical devices for source and destination.
--

commit 1f99dbf217347b66f1710f734eac617209a4e8c6
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add bio handling

This commit adds per-path bio submission support and failover requeuing
logic for libmultipath. It introduces mechanisms similar to NVMe multipath
to clone bios and submit them with the REQ_MPATH flag.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index 21f7ffdb22d60..81e737c1ce469 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c

[ ... ]

> @@ -243,6 +242,81 @@ static struct mpath_device *mpath_find_path(struct mpath_head *mpath_head)
> +static void mpath_bdev_submit_bio(struct bio *bio)
> +{

[ ... ]

> +		bio_set_dev(bio, mpath_device->disk->part0);
> +		/*
> +		 * Use BIO_REMAPPED to skip bio_check_eod() when this bio
> +		 * enters submit_bio_noacct() for the per-path device. The EOD
> +		 * check already passed on the multipath head.
> +		 */
> +		bio_set_flag(bio, BIO_REMAPPED);
> +		bio->bi_opf |= REQ_MPATH;
> +		trace_block_bio_remap(bio, disk_devt(mpath_device->disk),
> +				      bio->bi_iter.bi_sector);

[Severity: Medium]
Does this tracepoint call incorrectly use the target path device instead of
the original head device? 

By passing disk_devt(mpath_device->disk), it seems the trace event will log
the new path device as both the source and destination, which might break
trace analysis. Should this be disk_devt(mpath_head->disk)?

> +		submit_bio_noacct(bio);
> +	} else if (mpath_available_path(mpath_head)) {
> +		dev_warn_ratelimited(dev, "no usable path - requeuing I/O\n");
> +
> +		spin_lock_irq(&mpath_head->requeue_lock);
> +		bio_list_add(&mpath_head->requeue_list, bio);
> +		spin_unlock_irq(&mpath_head->requeue_lock);
> +		atomic_long_inc(&mpath_head->requeue_no_usable_path_cnt);
> +	} else {

[ ... ]

> @@ -314,11 +389,34 @@ static void multipath_partition_scan_work(struct work_struct *work)
>  void mpath_remove_disk(struct mpath_head *mpath_head)
>  {
>  	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
>  		struct gendisk *disk = mpath_head->disk;
>  
> +		/*
> +		 * requeue I/O after MPATH_HEAD_DISK_LIVE has been cleared
> +		 * to allow multipath to fail all I/O.
> +		 */
> +		mpath_schedule_requeue_work(mpath_head);
> +
>  		mpath_synchronize(mpath_head);

[Severity: High]
Could scheduling the requeue work before synchronizing SRCU cause a deadlock
during device removal?

If mpath_bdev_submit_bio() checks mpath_available_path() and sees the disk
is live, but then gets preempted before taking the requeue_lock,
mpath_remove_disk() might clear MPATH_HEAD_DISK_LIVE and schedule the
requeue worker.

The worker would run and find the requeue_list empty. When the submitter
wakes up and adds the bio, it will be stranded on the requeue_list.

Because the bio holds a q_usage_counter reference, will del_gendisk()
hang forever waiting for the queue to freeze?

>  		del_gendisk(disk);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=4

Re: [PATCH v3 02/13] libmultipath: Add basic gendisk support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] `mpath_alloc_head_disk()` leaves `ERR_PTR` in `mpath_head->disk` on failure, causing a kernel panic during cleanup or state updates.
- [High] A TOCTOU race between `mpath_device_set_live()` and `mpath_remove_disk()` permanently leaks the gendisk.
- [High] Synchronous wait in `mpath_head_uninit()` allows unprivileged openers to block device teardown indefinitely.
--

commit bba05c007c446cdb19dbac857c9ce717ca5f2a08
Author: John Garry <john.g.garry@oracle.com>
Subject: libmultipath: Add basic gendisk support

This patch adds support to allocate and free a multipath gendisk. It also
introduces struct mpath_head_template as a method for drivers to provide
custom multipath functionality.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index 009d4bb875c6f..79be84d3d4f75 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -44,12 +44,111 @@ void mpath_head_uninit(struct mpath_head *mpath_head)
>  }
>  EXPORT_SYMBOL_GPL(mpath_head_uninit);
>  
> +static int mpath_bdev_open(struct gendisk *disk, blk_mode_t mode)
> +{
> +	struct mpath_head *mpath_head = disk->private_data;
> +
> +	return mpath_get_head(mpath_head);
> +}

[Severity: High]
Does this increment the refcount on behalf of userspace openers?

If unprivileged users open the block device, this calls mpath_get_head()
which increments mpath_head->refcount. If the admin later removes the device,
the driver calls mpath_head_uninit() which synchronously waits for the
refcount to hit zero. Since the user can hold the file descriptor open,
couldn't this allow unprivileged openers to permanently hang the kernel
thread handling device removal?

[ ... ]

> +void mpath_remove_disk(struct mpath_head *mpath_head)
> +{
> +	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
> +		struct gendisk *disk = mpath_head->disk;
> +
> +		del_gendisk(disk);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(mpath_remove_disk);

[Severity: High]
Is there a race here with mpath_device_set_live() that could permanently
leak the gendisk?

If Thread A calls mpath_device_set_live() and sets MPATH_HEAD_DISK_LIVE
locklessly via test_and_set_bit(), and Thread B concurrently calls
mpath_remove_disk(), Thread B will clear the flag and call del_gendisk().

Because device_add_disk() in Thread A hasn't completed and set GD_ADDED
yet, del_gendisk() observes the disk is not live and returns early without
doing anything. 

Thread A then completes device_add_disk() making the disk live, but the
flag is now 0. Any subsequent mpath_remove_disk() will do nothing, leaking
the active block device.

> +void mpath_put_disk(struct mpath_head *mpath_head)
> +{
> +	if (!mpath_head->disk)
> +		return;
> +
> +	/* make sure all pending bios are cleaned up */
> +	flush_work(&mpath_head->partition_scan_work);
> +	put_disk(mpath_head->disk);
> +}
> +EXPORT_SYMBOL_GPL(mpath_put_disk);
> +
> +int mpath_alloc_head_disk(struct mpath_head *mpath_head,
> +			struct queue_limits *lim, int numa_node)
> +{
> +	if (!mpath_head->disk_groups || !mpath_head->parent)
> +		return -EINVAL;
> +
> +	mpath_head->disk = blk_alloc_disk(lim, numa_node);
> +	if (IS_ERR(mpath_head->disk))
> +		return PTR_ERR(mpath_head->disk);

[Severity: High]
If blk_alloc_disk() fails, does this leave an ERR_PTR in mpath_head->disk?

When the driver cleans up on error and calls mpath_put_disk(), the
if (!mpath_head->disk) check will pass because the ERR_PTR is not NULL.
Will this result in a kernel panic when put_disk() attempts to dereference
the invalid pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=2

Re: [PATCH v3 12/13] libmultipath: Add mpath_bdev_getgeo()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL check for the optional fops->ioctl callback in mpath_bdev_ioctl
--

commit 5104b9e862d1fb81d42be96c9762fa0a9897083a
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add mpath_bdev_getgeo()

This patch adds mpath_bdev_getgeo() as a multipath block device .getgeo
handler. It redirects the call into the selected mpath_device disk's
fops->getgeo handler.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index 4e4b347875500..26d9b17998d66 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -547,6 +547,26 @@ static int mpath_bdev_ioctl(struct block_device *bdev, blk_mode_t mode,
>  	return err;
>  }
>  
> +static int mpath_bdev_getgeo(struct gendisk *disk, struct hd_geometry *geo)
> +{
> +	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
> +	int srcu_idx, ret = -EWOULDBLOCK;
> +	struct mpath_device *mpath_device;
> +
> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
> +	mpath_device = mpath_find_path(mpath_head);
> +	if (mpath_device) {
> +		if (mpath_device->disk->fops->getgeo)

[Severity: High]
This is a pre-existing issue, but while reviewing this patch I noticed that
the new mpath_bdev_getgeo() correctly checks if the callback is present
before executing it.

However, the pre-existing mpath_bdev_ioctl() function located just above
this code does an unconditional dereference of the ioctl callback:

lib/multipath.c:mpath_bdev_ioctl() {
    ...
    err = mpath_device->disk->fops->ioctl(
            mpath_device->disk->part0, mode, cmd, arg);
    ...
}

If a user-space program issues an ioctl to the multipath block device and
the underlying device driver does not implement the optional ioctl callback,
could this result in a NULL pointer dereference and system panic?

> +			ret = mpath_device->disk->fops->getgeo(
> +					mpath_device->disk, geo);
> +		else
> +			ret = -ENOTTY; /* See blkdev_getgeo */
> +	}
> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=12

Re: [PATCH v3 11/13] libmultipath: Add support for block device IOCTL

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional dereference of optional `fops->ioctl` leads to NULL pointer dereference.
- [High] Bypass of the underlying block device's custom `.compat_ioctl` handler.
--

commit 3f49637890144f68bc3af4119c61a0b330200c0e
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add support for block device IOCTL

Add mpath_bdev_ioctl() as a multipath block device IOCTL handler. This
handler calls into the mpath_device bdev fops handler. Like what is done
for cdev IOCTL handler, use .ioctl_begin and .ioctl_finish methods.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index d335074eb5bcd..4e4b347875500 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -515,6 +515,38 @@ static void mpath_bdev_release(struct gendisk *disk)
>  	mpath_put_head(mpath_head);
>  }
>  
> +static int mpath_bdev_ioctl(struct block_device *bdev, blk_mode_t mode,
> +		    unsigned int cmd, unsigned long arg)
> +{
> +	struct gendisk *disk = bdev->bd_disk;
> +	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
> +	struct mpath_device *mpath_device;
> +	int srcu_idx, err;
> +	void *unlocked_ioctl_data = NULL;
> +
> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
> +	mpath_device = mpath_find_path(mpath_head);
> +	if (!mpath_device) {
> +		err = -EWOULDBLOCK;
> +		goto out_unlock;
> +	}
> +
> +	if (mpath_head->mpdt->ioctl_begin)
> +		mpath_head->mpdt->ioctl_begin(mpath_device, cmd,
> +					&unlocked_ioctl_data);
> +	if (unlocked_ioctl_data)
> +		srcu_read_unlock(&mpath_head->srcu, srcu_idx);
> +	err = mpath_device->disk->fops->ioctl(
> +			mpath_device->disk->part0, mode, cmd, arg);

[Severity: High]
Does this code dereference the underlying device's ioctl function pointer
without checking if it's NULL?

If the active path's block device does not implement the optional .ioctl method
in its block_device_operations, won't this cause a NULL pointer dereference
when user-space issues an ioctl on the multipath device node?

> +	if (unlocked_ioctl_data) {
> +		mpath_head->mpdt->ioctl_finish(unlocked_ioctl_data);
> +		return err;
> +	}
> +out_unlock:
> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
> +	return err;
> +}
> +

[ ... ]

> @@ -701,6 +733,8 @@ const struct block_device_operations mpath_ops = {
>  	.open		= mpath_bdev_open,
>  	.release	= mpath_bdev_release,
>  	.submit_bio	= mpath_bdev_submit_bio,
> +	.ioctl		= mpath_bdev_ioctl,
> +	.compat_ioctl	= blkdev_compat_ptr_ioctl,

[Severity: High]
Does using the generic blkdev_compat_ptr_ioctl wrapper here bypass the
underlying block device's custom .compat_ioctl handler?

If a 32-bit user space task invokes a compat ioctl, the core block layer will
delegate to this generic wrapper, which routes the call to the native
mpath_bdev_ioctl().

That native handler then unconditionally forwards the call to the underlying
path device's native .ioctl method, entirely skipping the underlying device's
.compat_ioctl handler.

Could this lead to 32-bit struct layouts being passed directly into a 64-bit
native .ioctl handler, potentially causing memory corruption or out-of-bounds
reads if the underlying driver requires custom struct repacking?

> +	.report_zones	= mpath_bdev_report_zones,
>  	.pr_ops		= &mpath_pr_ops,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=11

Re: [PATCH v3 01/13] libmultipath: Add initial framework

[John Garry]

On 03/07/2026 11:43, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] A race condition between mpath_put_head() and mpath_head_uninit() leads to a use-after-free vulnerability when accessing mpath_head for cleanup.
> --
> 
> commit 594f063adb51503b6e7af6741526b8d738fee5ba
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add initial framework
> 
> This commit adds the initial framework for libmultipath to support path
> management, path selection, and failover handling for multipath-capable
> block drivers. It introduces the mpath_head structure to manage available
> paths and the multipath gendisk.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> new file mode 100644
>> index 0000000000000..009d4bb875c6f
>> --- /dev/null
>> +++ b/lib/multipath.c
> 
> [ ... ]
> 
>> +void mpath_put_head(struct mpath_head *mpath_head)
>> +{
>> +	refcount_t *refcount = &mpath_head->refcount;
>> +
>> +	if (refcount_dec_and_test(refcount)) {
> 
> [Severity: High]
> Could this introduce a use-after-free regression on the mpath_head structure?
> 
> If a teardown thread calls mpath_head_uninit() concurrently, it blocks in
> wait_var_event() waiting for the refcount to drop to 0.
> 
> Once refcount_dec_and_test() drops the refcount to 0 here, the condition
> !refcount_read(refcount) in mpath_head_uninit() can evaluate to true
> immediately. The uninit thread could then wake up, return, and free mpath_head
> before this thread completes mpath_head_cleanup() and wake_up_var().

I will address this.

> 
>> +		mpath_head_cleanup(mpath_head);
>> +		wake_up_var(refcount);
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(mpath_put_head);
>> +
>> +void mpath_head_uninit(struct mpath_head *mpath_head)
>> +{
>> +	refcount_t *refcount = &mpath_head->refcount;
>> +
>> +	if (refcount_dec_and_test(refcount)) {
>> +		mpath_head_cleanup(mpath_head);
>> +	} else {
>> +		wait_var_event(refcount, !refcount_read(refcount));
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(mpath_head_uninit);
> 

Re: [PATCH v3 02/13] libmultipath: Add basic gendisk support

[John Garry]

On 03/07/2026 11:51, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] `mpath_alloc_head_disk()` leaves `ERR_PTR` in `mpath_head->disk` on failure, causing a kernel panic during cleanup or state updates.
> - [High] A TOCTOU race between `mpath_device_set_live()` and `mpath_remove_disk()` permanently leaks the gendisk.
> - [High] Synchronous wait in `mpath_head_uninit()` allows unprivileged openers to block device teardown indefinitely.
> --
> 
> commit bba05c007c446cdb19dbac857c9ce717ca5f2a08
> Author: John Garry <john.g.garry@oracle.com>
> Subject: libmultipath: Add basic gendisk support
> 
> This patch adds support to allocate and free a multipath gendisk. It also
> introduces struct mpath_head_template as a method for drivers to provide
> custom multipath functionality.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 009d4bb875c6f..79be84d3d4f75 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
>> @@ -44,12 +44,111 @@ void mpath_head_uninit(struct mpath_head *mpath_head)
>>   }
>>   EXPORT_SYMBOL_GPL(mpath_head_uninit);
>>   
>> +static int mpath_bdev_open(struct gendisk *disk, blk_mode_t mode)
>> +{
>> +	struct mpath_head *mpath_head = disk->private_data;
>> +
>> +	return mpath_get_head(mpath_head);
>> +}
> 
> [Severity: High]
> Does this increment the refcount on behalf of userspace openers?
> 
> If unprivileged users open the block device, this calls mpath_get_head()
> which increments mpath_head->refcount. If the admin later removes the device,
> the driver calls mpath_head_uninit() which synchronously waits for the
> refcount to hit zero. Since the user can hold the file descriptor open,
> couldn't this allow unprivileged openers to permanently hang the kernel
> thread handling device removal?

Userspace can still issue a syscall which can reference the mpath_head 
structure, so we need to guard against that.

> 
> [ ... ]
> 
>> +void mpath_remove_disk(struct mpath_head *mpath_head)
>> +{
>> +	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
>> +		struct gendisk *disk = mpath_head->disk;
>> +
>> +		del_gendisk(disk);
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(mpath_remove_disk);
> 
> [Severity: High]
> Is there a race here with mpath_device_set_live() that could permanently
> leak the gendisk?
> 
> If Thread A calls mpath_device_set_live() and sets MPATH_HEAD_DISK_LIVE
> locklessly via test_and_set_bit(), and Thread B concurrently calls
> mpath_remove_disk(), 

That should not happen. The driver needs to take care of concurrent 
calls. In the NVMe example, we remove the NS head structure from the 
list of NS heads before we call mpath_remove_disk(), so no new NS can 
call mpath_device_set_live() for the same mpath_head once the disk is 
being removed.

>Thread B will clear the flag and call del_gendisk().
> 
> Because device_add_disk() in Thread A hasn't completed and set GD_ADDED
> yet, del_gendisk() observes the disk is not live and returns early without
> doing anything.
> 
> Thread A then completes device_add_disk() making the disk live, but the
> flag is now 0. Any subsequent mpath_remove_disk() will do nothing, leaking
> the active block device.
> 
>> +void mpath_put_disk(struct mpath_head *mpath_head)
>> +{
>> +	if (!mpath_head->disk)
>> +		return;
>> +
>> +	/* make sure all pending bios are cleaned up */
>> +	flush_work(&mpath_head->partition_scan_work);
>> +	put_disk(mpath_head->disk);
>> +}
>> +EXPORT_SYMBOL_GPL(mpath_put_disk);
>> +
>> +int mpath_alloc_head_disk(struct mpath_head *mpath_head,
>> +			struct queue_limits *lim, int numa_node)
>> +{
>> +	if (!mpath_head->disk_groups || !mpath_head->parent)
>> +		return -EINVAL;
>> +
>> +	mpath_head->disk = blk_alloc_disk(lim, numa_node);
>> +	if (IS_ERR(mpath_head->disk))
>> +		return PTR_ERR(mpath_head->disk);
> 
> [Severity: High]
> If blk_alloc_disk() fails, does this leave an ERR_PTR in mpath_head->disk?
> 
> When the driver cleans up on error and calls mpath_put_disk(), the
> if (!mpath_head->disk) check will pass because the ERR_PTR is not NULL.
> Will this result in a kernel panic when put_disk() attempts to dereference
> the invalid pointer?

We don't call mpath_put_disk() for a failure there. In this case the 
caller should tear down the mpath_head structure.

> 

Re: [PATCH v3 03/13] libmultipath: Add path selection support

[John Garry]

On 03/07/2026 11:45, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
> - [High] `mpath_round_robin_path` enters an infinite loop if the current cached path (`old`) is concurrently removed.
> - [High] Unsafe usage of `list_is_singular()` on an RCU list leads to data races and routes I/O to a removed path.
> - [High] `mpath_numa_path` fails to verify if the cached path is disabled before returning it, potentially routing I/O to a disabled path.
> - [High] `mpath_alloc_head_disk` leaves an ERR_PTR in `mpath_head->disk` on failure, causing cleanup paths to crash.
> - [Medium] NULL pointer dereference in `mpath_queue_depth_path` if a device does not provide `nr_active`.
> - [Medium] The `.get_access_state` callback is missing from `struct mpath_head_template`, violating the documented API intent.
> --
> 
> commit e941f797884516dd9568e0b668984eb3c895248a
> Author: John Garry <john.g.garry@oracle.com>
> 
>      libmultipath: Add path selection support
>      
>      Add code for path selection.
> 
>>      NVMe ANA is abstracted into enum mpath_access_state. The motivation here is
>>      so that SCSI ALUA can be used. Callbacks .is_disabled, .is_optimized,
>>      .get_access_state are added to get the path access state.
> 
> [Severity: Medium]
> The commit message mentions adding a .get_access_state callback, but it appears
> to be missing from struct mpath_head_template. Is this intentional, or should
> it be included in the struct definition to avoid requiring drivers to directly
> modify the access_state field?

I will fix this. This text was not updated from an earlier series.

> 
> [ ... ]
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 79be84d3d4f75..21f7ffdb22d60 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
> [ ... ]
>> +static struct mpath_device *mpath_round_robin_path(
>> +				struct mpath_head *mpath_head)
>> +{
>> +	struct mpath_device *mpath_device, *found = NULL;
>> +	int node = numa_node_id();
>> +	enum mpath_access_state access_state_old;
>> +	struct mpath_device *old =
>> +			srcu_dereference(mpath_head->current_path[node],
>> +				&mpath_head->srcu);
>> +
>> +	if (unlikely(!old))
>> +		return __mpath_find_path(mpath_head, node);
>> +
>> +	if (list_is_singular(&mpath_head->dev_list)) {
>> +		if (mpath_path_is_disabled(mpath_head, old))
>> +			return NULL;
>> +		return old;
>> +	}
> 
> [Severity: High]
> Could using list_is_singular() here lead to data races? Since this list is
> managed with RCU, and list_is_singular() evaluates head->next == head->prev,
> it might read inconsistent prev pointers during concurrent updates. Also, if old
> was concurrently removed and only one valid path remains, would this return
> the removed old path instead of the new remaining path?

uh, since we hold the RCU read lock for traversal, this should not 
happen. Having said that, since in this patch there are no users of 
mpath_find_path() and the user does the RCU read lock.

So maybe I can merge this patch with another. Or the context analysis 
annotations could help.

> 
>> +
>> +	for (mpath_device = mpath_next_dev(mpath_head, old);
>> +	    mpath_device && mpath_device != old;
>> +	    mpath_device = mpath_next_dev(mpath_head, mpath_device)) {
> 
> [Severity: High]
> Does this loop terminate safely if old has been concurrently removed from the list?
> If old is unlinked via list_del_rcu(), mpath_device != old will always be true.
> Could this result in an infinite loop wrapping around the active elements
> indefinitely, leading to a lockup if no path is MPATH_STATE_OPTIMIZED?

As above

> 
>> +
>> +		if (mpath_path_is_disabled(mpath_head, mpath_device))
>> +			continue;
> [ ... ]
>> +static struct mpath_device *mpath_queue_depth_path(
>> +				struct mpath_head *mpath_head)
>> +{
>> +	struct mpath_device *best_opt = NULL, *mpath_device;
>> +	struct mpath_device *best_nonopt = NULL;
>> +	unsigned int min_depth_opt = UINT_MAX, min_depth_nonopt = UINT_MAX;
>> +	unsigned int depth;
>> +
>> +	list_for_each_entry_srcu(mpath_device, &mpath_head->dev_list, siblings,
>> +				 srcu_read_lock_held(&mpath_head->srcu)) {
>> +
>> +		if (mpath_path_is_disabled(mpath_head, mpath_device))
>> +			continue;
>> +
>> +		depth = atomic_read(mpath_device->nr_active);
> 
> [Severity: Medium]
> Could this dereference a NULL pointer if a device was added without providing
> an nr_active atomic counter during mpath_add_device()? If userspace sets the
> I/O policy to 'queue-depth' via sysfs, would it crash here?

Well the driver needs to ensure that it is valid. However I can add code 
to test this.

> 
>> +
>> +		switch (mpath_device->access_state) {
> [ ... ]
>> +static struct mpath_device *mpath_numa_path(struct mpath_head *mpath_head)
>> +{
>> +	int node = numa_node_id();
>> +	struct mpath_device *mpath_device;
>> +
>> +	mpath_device = srcu_dereference(mpath_head->current_path[node],
>> +					&mpath_head->srcu);
>> +	if (unlikely(!mpath_device))
>> +		return __mpath_find_path(mpath_head, node);
>> +	if (unlikely(!mpath_path_is_optimized(mpath_head, mpath_device)))
>> +		return __mpath_find_path(mpath_head, node);
>> +	return mpath_device;
>> +}
> 
> [Severity: High]
> Does this correctly validate the cached path before returning it? Unlike other
> path selection functions, this omits checking mpath_path_is_disabled().
> If a path transitions to a disabled state but remains marked as optimized, could
> mpath_numa_path() incorrectly continue to route I/O to it?

Yeah, maybe we should check that also to be clear

> 
> [ ... ]
>> @@ -103,7 +339,8 @@ EXPORT_SYMBOL_GPL(mpath_put_disk);
>>   int mpath_alloc_head_disk(struct mpath_head *mpath_head,
>>   			struct queue_limits *lim, int numa_node)
>>   {
>> -	if (!mpath_head->disk_groups || !mpath_head->parent)
>> +	if (!mpath_head->disk_groups || !mpath_head->parent ||
>> +	    !mpath_head->iopolicy)
>>   		return -EINVAL;
>>   
>>   	mpath_head->disk = blk_alloc_disk(lim, numa_node);
> 
> [Severity: High]
> If blk_alloc_disk() fails, it returns an ERR_PTR. If this is assigned directly
> to mpath_head->disk and an error is returned without setting mpath_head->disk
> back to NULL, could this cause subsequent cleanup paths (like mpath_put_disk())
> to crash when they check if (mpath_head->disk) and attempt to put an ERR_PTR?

I already commented on this in the response to another patch review.

> 

Re: [PATCH v3 04/13] libmultipath: Add bio handling

[John Garry]

On 03/07/2026 11:49, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] TOCTOU race in `mpath_remove_disk` can strand a bio on `requeue_list`, causing `del_gendisk` to deadlock.
> - [Medium] The `trace_block_bio_remap` tracepoint is called with the wrong device argument for `old_dev`, causing the trace event to emit identical devices for source and destination.
> --
> 
> commit 1f99dbf217347b66f1710f734eac617209a4e8c6
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add bio handling
> 
> This commit adds per-path bio submission support and failover requeuing
> logic for libmultipath. It introduces mechanisms similar to NVMe multipath
> to clone bios and submit them with the REQ_MPATH flag.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 21f7ffdb22d60..81e737c1ce469 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
> 
> [ ... ]
> 
>> @@ -243,6 +242,81 @@ static struct mpath_device *mpath_find_path(struct mpath_head *mpath_head)
>> +static void mpath_bdev_submit_bio(struct bio *bio)
>> +{
> 
> [ ... ]
> 
>> +		bio_set_dev(bio, mpath_device->disk->part0);
>> +		/*
>> +		 * Use BIO_REMAPPED to skip bio_check_eod() when this bio
>> +		 * enters submit_bio_noacct() for the per-path device. The EOD
>> +		 * check already passed on the multipath head.
>> +		 */
>> +		bio_set_flag(bio, BIO_REMAPPED);
>> +		bio->bi_opf |= REQ_MPATH;
>> +		trace_block_bio_remap(bio, disk_devt(mpath_device->disk),
>> +				      bio->bi_iter.bi_sector);
> 
> [Severity: Medium]
> Does this tracepoint call incorrectly use the target path device instead of
> the original head device?
> 
> By passing disk_devt(mpath_device->disk), it seems the trace event will log
> the new path device as both the source and destination, which might break
> trace analysis. Should this be disk_devt(mpath_head->disk)?

But with bio_set_dev(), the new device is from mpath_device->disk->part0 
and old is the mpath_head disk in mpath_head->disk

> 
>> +		submit_bio_noacct(bio);
>> +	} else if (mpath_available_path(mpath_head)) {
>> +		dev_warn_ratelimited(dev, "no usable path - requeuing I/O\n");
>> +
>> +		spin_lock_irq(&mpath_head->requeue_lock);
>> +		bio_list_add(&mpath_head->requeue_list, bio);
>> +		spin_unlock_irq(&mpath_head->requeue_lock);
>> +		atomic_long_inc(&mpath_head->requeue_no_usable_path_cnt);
>> +	} else {
> 
> [ ... ]
> 
>> @@ -314,11 +389,34 @@ static void multipath_partition_scan_work(struct work_struct *work)
>>   void mpath_remove_disk(struct mpath_head *mpath_head)
>>   {
>>   	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
>>   		struct gendisk *disk = mpath_head->disk;
>>   
>> +		/*
>> +		 * requeue I/O after MPATH_HEAD_DISK_LIVE has been cleared
>> +		 * to allow multipath to fail all I/O.
>> +		 */
>> +		mpath_schedule_requeue_work(mpath_head);
>> +
>>   		mpath_synchronize(mpath_head);
> 
> [Severity: High]
> Could scheduling the requeue work before synchronizing SRCU cause a deadlock
> during device removal?
> 
> If mpath_bdev_submit_bio() checks mpath_available_path() and sees the disk
> is live, but then gets preempted before taking the requeue_lock,
> mpath_remove_disk() might clear MPATH_HEAD_DISK_LIVE and schedule the
> requeue worker.
> 
> The worker would run and find the requeue_list empty. When the submitter
> wakes up and adds the bio, it will be stranded on the requeue_list.
> 
> Because the bio holds a q_usage_counter reference, will del_gendisk()
> hang forever waiting for the queue to freeze?

Yeah, maybe we should do that. This is same as mainline NVMe multipath 
code today.

> 
>>   		del_gendisk(disk);
>>   	}
> 

Re: [PATCH v3 06/13] libmultipath: Add delayed removal support

[John Garry]

On 03/07/2026 11:45, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
> - [High] Repeated calls to `mpath_can_remove_head()` leak module references when removal work is already pending.
> - [High] Race condition between `mpath_add_device()`, `mpath_can_remove_head()`, and `mpath_remove_head_work()` leads to use-after-free of mpath_head.
> - [High] Missing cancellation of delayed removal work on `mpath_head` teardown results in Use-After-Free.
> - [High] `mpath_head->drv_module` is never initialized by the subsystem API, leading to a silent failure to pin the driver module.
> - [Medium] Disabling delayed removal via sysfs fails to cancel already pending removal work, leading to unexpected device removal.
> - [Low] Incompatible pointer type passed to `kstrtouint()` causes compiler warnings.
> - [Low] Integer overflow when calculating the delay for `mod_delayed_work()` results in severely shortened delays.
> --
> 
> commit c00bb95222f826ebc3e8dbd55ff6884abf84858e
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add delayed removal support
>      
> Add support for delayed removal, same as exists for NVMe.
>      
> The purpose of this feature is to keep the multipath disk and cdev present
> for intermittent periods of no available path.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 007aa34796569..78f88b0664c78 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
>> @@ -61,6 +61,8 @@ void mpath_add_device(struct mpath_device *mpath_device,
>>   	mutex_lock(&mpath_head->lock);
>>   	list_add_tail_rcu(&mpath_device->siblings, &mpath_head->dev_list);
>>   	mutex_unlock(&mpath_head->lock);
>> +	if (cancel_delayed_work(&mpath_head->remove_work))
>> +		module_put(mpath_head->drv_module);
>>   }
> 
> [Severity: High]
> Does this code introduce a use-after-free race condition?
> 
> If the delayed removal timer is already executing, cancel_delayed_work()
> returns false.
> 
> mpath_add_device() will successfully add the device, but the
> already-executing mpath_remove_head_work() unconditionally calls
> mpath_head->mpdt->remove_head() without checking for newly added devices.
> 
> Could the newly added device access the destroyed mpath_head?

Ensuring that new devices are not added while the head is being removed 
is the job of the driver.

In the NVMe mainline case, in the nvme_remove_head_work() will 
atomically ensure that no new devices appear and then decide to delete 
the head list entry reference (so no new devices can reference the same 
head)

> 
> [ ... ]
> 
>> @@ -518,6 +530,39 @@ static void mpath_requeue_work(struct work_struct *work)
>> +bool mpath_can_remove_head(struct mpath_head *mpath_head)
>> +{
>> +	bool remove = false;
>> +
>> +	mutex_lock(&mpath_head->lock);
>> +	/*
>> +	 * Ensure that no one could remove this module while the head
>> +	 * remove work is pending.
>> +	 */
>> +	if (mpath_head_queue_if_no_path(mpath_head) &&
>> +		try_module_get(mpath_head->drv_module)) {
> 
> [Severity: High]
> Is mpath_head->drv_module initialized anywhere in the subsystem API?
> 
> If it remains uninitialized (e.g., NULL from zero-allocated structures),
> try_module_get(NULL) will silently succeed.
> 
> Could this lead to a failure to pin the driver module, potentially resulting
> in a crash if the module is unloaded while the timer is pending?

The driver needs to ensure that this is set.

It is hard to add a check, as if the driver is builtin then THIS_MODULE 
is NULL. Maybe that can be improved in the core code.

> 
>> +
>> +		mod_delayed_work(mpath_wq, &mpath_head->remove_work,
>> +				mpath_head->delayed_removal_secs * HZ);
> 
> [Severity: High]
> Does this unconditionally increment the module reference count even when the
> timer is already pending?
> 
> If mpath_can_remove_head() is called multiple times, try_module_get()
> will increment the reference count on each call.

It should not be called multiple times. That is the job of the driver.

> 
> However, mod_delayed_work() merely modifies the existing timer. When the
> timer eventually fires, mpath_remove_head_work() calls module_put() only
> once, leaking the additional module references and preventing unloading.
> 
> [Severity: Low]
> Does this multiplication overflow on 32-bit systems?
> 
> mpath_head->delayed_removal_secs is an unsigned int, and HZ is typically
> an integer.
> 
> Could their product overflow 32-bit arithmetic before being promoted to
> unsigned long for mod_delayed_work(), leading to shortened delays?

I don't think that this is a problem. The delayed removal secs should 
not be so big as to overflow.

> 
>> +	} else {
>> +		remove = true;
>> +	}
>> +
>> +	mutex_unlock(&mpath_head->lock);
>> +	return remove;
>> +}
>> +EXPORT_SYMBOL_GPL(mpath_can_remove_head);
>> +
>> +static void mpath_remove_head_work(struct work_struct *work)
>> +{
>> +	struct mpath_head *mpath_head = container_of(to_delayed_work(work),
>> +			struct mpath_head, remove_work);
>> +	struct module *drv_module = mpath_head->drv_module;
>> +
>> +	mpath_head->mpdt->remove_head(mpath_head);
>> +	module_put(drv_module);
>> +}
> 
> [ ... ]
> 
>> @@ -562,6 +607,9 @@ int mpath_alloc_head_disk(struct mpath_head *mpath_head,
>>   	mpath_head->disk->private_data = mpath_head;
>>   	mpath_head->disk->fops = &mpath_ops;
>>   
>> +	INIT_DELAYED_WORK(&mpath_head->remove_work, mpath_remove_head_work);
>> +	mpath_head->delayed_removal_secs = 0;
>> +
>>   	set_bit(GD_SUPPRESS_PART_SCAN, &mpath_head->disk->state);
> 
> [Severity: High]
> Is the delayed removal work reliably canceled during final teardown?
> 
> During explicit device teardown (like a driver unbind), functions like
> mpath_head_cleanup() are called to free resources.
> 
> Since this patch doesn't add a cancel_delayed_work_sync() call to the
> cleanup path, could an active timer fire after the mpath_head has been
> freed, leading to a use-after-free?

Hmmm ... I don't get it. When the delayed work is active, then only the 
delayed timer callback will remove the mpath_head

> 
> [ ... ]
> 
>> @@ -605,6 +653,47 @@ void mpath_device_set_live(struct mpath_device *mpath_device)
>> +ssize_t mpath_delayed_removal_secs_store(struct mpath_head *mpath_head,
>> +			const char *buf, size_t count)
>> +{
>> +	ssize_t ret;
>> +	int sec;
>> +
>> +	ret = kstrtouint(buf, 0, &sec);
> 
> [Severity: Low]
> Does passing an int pointer to kstrtouint() cause compiler warnings?
> 
> sec is declared as an int, but kstrtouint() strictly expects an unsigned
> int *.

ok, I'll make sec as an unsigned int

> 
> Could this cause an incompatible pointer type warning and potential undefined
> behavior on architectures with differing int/unsigned int layouts?
> 
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	mutex_lock(&mpath_head->lock);
>> +	mpath_head->delayed_removal_secs = sec;
>> +	if (sec)
>> +		set_bit(MPATH_HEAD_QUEUE_IF_NO_PATH, &mpath_head->flags);
>> +	else
>> +		clear_bit(MPATH_HEAD_QUEUE_IF_NO_PATH, &mpath_head->flags);
> 
> [Severity: Medium]
> Should the active timer be canceled when delayed removal is disabled?

unsetting MPATH_HEAD_QUEUE_IF_NO_PATH has that effect

> 
> When a user writes 0 to disable the feature, this code clears the flag but
> misses a call to cancel_delayed_work(&mpath_head->remove_work).
> 
> If the timer is already pending, will it continue to run and eventually
> destroy the head, contrary to the user's configuration?
> 
>> +	mutex_unlock(&mpath_head->lock);
> 

Re: [PATCH v3 07/13] libmultipath: Add cdev support

[John Garry]

On 03/07/2026 11:48, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Dropping the SRCU read lock during ioctl execution leaves the `mpath_device` pointer unprotected, causing a use-after-free when passed to `cdev_ioctl` or accessed.
> - [High] Decoupled lifetime between the embedded `cdev` and `mpath_head` reference counting leads to Use-After-Free during VFS file open and release.
> --
> 
> commit 9a6527083c29f0b9d58867aa52ca2ea700cf1168
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add cdev support
> 
> Add support to create a cdev multipath device. The functionality is much
> the same as NVMe, where the cdev is created when a mpath device is set
> live.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 78f88b0664c78..6d2e1186a10f8 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
> 
> [ ... ]
> 
>> +static int mpath_chr_open(struct inode *inode, struct file *file)
>> +{
>> +	struct cdev *cdev = file_inode(file)->i_cdev;
>> +	struct mpath_head *mpath_head =
>> +			container_of(cdev, struct mpath_head, cdev);
>> +
>> +	return mpath_get_head(mpath_head);
>> +}
> 
> [Severity: High]
> Is there a race condition with driver teardown here?
> 
> If teardown occurs after the VFS has looked up the cdev but before this
> function calls mpath_get_head() to increment the refcount, mpath_head could
> be freed, potentially causing a use-after-free when this accesses the freed
> memory.

Yeah, that does look like a bug and the same issue exists in the NVMe 
driver today.

> 
>> +
>> +static int mpath_chr_release(struct inode *inode, struct file *file)
>> +{
>> +	struct cdev *cdev = file_inode(file)->i_cdev;
>> +	struct mpath_head *mpath_head =
>> +			container_of(cdev, struct mpath_head, cdev);
>> +
>> +	mpath_put_head(mpath_head);
>> +	return 0;
>> +}
> 
> [Severity: High]
> Does this sequence cause the embedded cdev to be freed prematurely?
> 
> When the last file descriptor is closed, VFS __fput() calls
> mpath_chr_release(). If mpath_put_head() drops the mpath_head refcount to 0,
> the mpath_head structure (and the embedded cdev) is freed.
> 
> When this returns, VFS __fput() will subsequently call
> cdev_put(inode->i_cdev). Could this result in a use-after-free on the now
> freed cdev->kobj?

As above

> 
>> +
>> +static long mpath_chr_ioctl(struct file *file, unsigned int cmd,
>> +		unsigned long arg)
>> +{
>> +	struct cdev *cdev = file_inode(file)->i_cdev;
>> +	struct mpath_head *mpath_head =
>> +			container_of(cdev, struct mpath_head, cdev);
>> +	struct mpath_device *mpath_device;
>> +	int srcu_idx, err = -EWOULDBLOCK;
>> +	void *unlocked_ioctl_data = NULL;
>> +
>> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
>> +	mpath_device = mpath_find_path(mpath_head);
>> +	if (!mpath_device)
>> +		goto out_unlock;
>> +	if (mpath_head->mpdt->ioctl_begin)
>> +		mpath_head->mpdt->ioctl_begin(mpath_device, cmd,
>> +					&unlocked_ioctl_data);
>> +	if (unlocked_ioctl_data)
>> +		srcu_read_unlock(&mpath_head->srcu, srcu_idx);
>> +	err = mpath_head->mpdt->cdev_ioctl(mpath_device, cmd, arg,
>> +					file->f_mode & FMODE_WRITE);
> 
> [Severity: Critical]
> Are we exposing the mpath_device pointer to a use-after-free here?

Yeah, this needs to be improved.

> 
> If unlocked_ioctl_data is present, the SRCU lock is dropped just before
> calling cdev_ioctl(). Since the lock is dropped, a concurrent hot-unplug
> removal process calling synchronize_srcu() might not wait for this ioctl to
> finish and could free the mpath_device.
> 
> This would leave the mpath_device pointer unprotected, which is then
> immediately passed into cdev_ioctl().
> 
> Also, does mpath_bdev_ioctl() suffer from this exact same problem when it
> calls mpath_device->disk->fops->ioctl() after potentially dropping its SRCU
> lock?
> 
>> +	if (unlocked_ioctl_data) {
>> +		mpath_head->mpdt->ioctl_finish(unlocked_ioctl_data);
>> +		return err;
>> +	}
>> +
>> +out_unlock:
>> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
>> +	return err;
>> +}
> 

Re: [PATCH v3 08/13] libmultipath: Add sysfs helpers

[John Garry]

On 03/07/2026 11:44, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] mpath_iopolicy_store() does not clear cached paths when the I/O policy changes, contradicting the commit message and leaving stale I/O routing state.
> - [Low] Unused 'count' parameter in mpath_iopolicy_store().
> --
> 
> commit e168713423f62d0ecfcf0e0b0c9ed72180e1f19b
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add sysfs helpers
> 
> This commit adds sysfs helpers for the libmultipath driver sysfs code. It
> includes helpers to get/set I/O policies, show device paths per NUMA node,
> and handle attribute groups.
> 
>>      Note that mpath_iopolicy_store() has a update callback to allow same
>>      functionality as nvme_subsys_iopolicy_update() be run for clearing paths.
> 
> [Severity: High]
> Does this description match the implementation? The code doesn't seem to provide
> the callback parameter mentioned here.

will fix

> 
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 6d2e1186a10f8..4945f2d847fbf 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
> [ ... ]
>> +static void mpath_iopolicy_update(enum mpath_iopolicy_e *iopolicy,
>> +		int new)
>> +{
>> +	int old = READ_ONCE(*iopolicy);
>> +
>> +	if (old == new)
>> +		return;
>> +
>> +	WRITE_ONCE(*iopolicy, new);
>> +
>> +	pr_info("iopolicy changed from %s to %s\n",
>> +		mpath_iopolicy_names[old],
>> +		mpath_iopolicy_names[new]);
>> +}
>> +
>> +bool mpath_iopolicy_store(enum mpath_iopolicy_e *iopolicy,
>> +				const char *buf, size_t count)
>                                                   ^^^^^^^^^^^^
> 
> [Severity: Low]
> This isn't a bug, but is the count parameter needed here? It doesn't
> appear to be used in the function body since sysfs_streq() is used for the
> string comparison.

will fix

> 
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(mpath_iopolicy_names); i++) {
>> +		if (sysfs_streq(buf, mpath_iopolicy_names[i])) {
>> +			mpath_iopolicy_update(iopolicy, i);
> 
> [Severity: High]
> Does changing the I/O policy via sysfs leave stale paths cached under the old
> policy rules?
> 
> Neither mpath_iopolicy_store() nor mpath_iopolicy_update() accept a callback
> function pointer, nor do they clear the paths themselves. As a result, drivers
> adopting this API have no mechanism to hook into the change to acquire
> subsystem locks or clear active paths.

drivers are responsible for clearing paths after calling this function

> 
>> +			return true;
>> +		}
>> +	}
>> +
>> +	return false;
>> +}
> 

Re: [PATCH v3 09/13] libmultipath: Add PR support

[John Garry]

On 03/07/2026 11:48, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Calling Persistent Reservation (PR) operations on a multipath partition causes a NULL pointer dereference.
> --
> 
> commit ed0ad48ae1e1adac4c8ee8809fa3bba6ef9f4ac5
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add PR support
> 
> Add support for persistent reservations.
> 
> Effectively all that is done here is that a multipath version of pr_ops is
> created which calls into the bdev fops callback for the mpath_device
> selected.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 4945f2d847fbf..cec7047ce7b9b 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
>> @@ -491,11 +491,193 @@ static void mpath_bdev_release(struct gendisk *disk)
>>   	mpath_put_head(mpath_head);
>>   }
>>   
>> +static int mpath_pr_register(struct block_device *bdev, u64 old_key,
>> +			u64 new_key, unsigned int flags)
>> +{
>> +	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
>> +	struct mpath_device *mpath_device;
>> +	int srcu_idx, ret = -EWOULDBLOCK;
>> +
>> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
> 
> [Severity: Critical]
> Will this cause a NULL pointer dereference if the persistent reservation
> operation is called on a partition?
> 
> If the target_core_iblock backend is configured for PR passthrough on a
> partition of a multipath device,

PR ops are rejected for partitions, but I can still use 
mpath_gendisk_to_head() which is better

> bdev will be the partition's block_device.
> Because dev_set_drvdata() is only called for the base disk's device,
> dev_get_drvdata(&bdev->bd_device) will return NULL for partition devices.
> 
> Other functions in this file, like mpath_bdev_ioctl(), seem to handle this by
> resolving the disk first using mpath_gendisk_to_head(bdev->bd_disk). Should
> the PR callbacks do the same?
> 
> [ ... ]
> 
>> +static int mpath_pr_reserve(struct block_device *bdev, u64 key,
>> +		enum pr_type type, unsigned flags)
>> +{
>> +	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
>> +	struct mpath_device *mpath_device;
>> +	int srcu_idx, ret = -EWOULDBLOCK;
>> +
>> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
> 
> [Severity: Critical]
> The same NULL pointer dereference risk applies here for partition devices, as
> well as to mpath_pr_release(), mpath_pr_preempt(), mpath_pr_clear(),
> mpath_pr_read_keys(), and mpath_pr_read_reservation() introduced below.
> 
> [ ... ]
> 

Re: [PATCH v3 11/13] libmultipath: Add support for block device IOCTL

[John Garry]

On 03/07/2026 11:56, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Unconditional dereference of optional `fops->ioctl` leads to NULL pointer dereference.
> - [High] Bypass of the underlying block device's custom `.compat_ioctl` handler.
> --
> 
> commit 3f49637890144f68bc3af4119c61a0b330200c0e
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add support for block device IOCTL
> 
> Add mpath_bdev_ioctl() as a multipath block device IOCTL handler. This
> handler calls into the mpath_device bdev fops handler. Like what is done
> for cdev IOCTL handler, use .ioctl_begin and .ioctl_finish methods.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index d335074eb5bcd..4e4b347875500 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
>> @@ -515,6 +515,38 @@ static void mpath_bdev_release(struct gendisk *disk)
>>   	mpath_put_head(mpath_head);
>>   }
>>   
>> +static int mpath_bdev_ioctl(struct block_device *bdev, blk_mode_t mode,
>> +		    unsigned int cmd, unsigned long arg)
>> +{
>> +	struct gendisk *disk = bdev->bd_disk;
>> +	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
>> +	struct mpath_device *mpath_device;
>> +	int srcu_idx, err;
>> +	void *unlocked_ioctl_data = NULL;
>> +
>> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
>> +	mpath_device = mpath_find_path(mpath_head);
>> +	if (!mpath_device) {
>> +		err = -EWOULDBLOCK;
>> +		goto out_unlock;
>> +	}
>> +
>> +	if (mpath_head->mpdt->ioctl_begin)
>> +		mpath_head->mpdt->ioctl_begin(mpath_device, cmd,
>> +					&unlocked_ioctl_data);
>> +	if (unlocked_ioctl_data)
>> +		srcu_read_unlock(&mpath_head->srcu, srcu_idx);
>> +	err = mpath_device->disk->fops->ioctl(
>> +			mpath_device->disk->part0, mode, cmd, arg);
> 
> [Severity: High]
> Does this code dereference the underlying device's ioctl function pointer
> without checking if it's NULL?
> 
> If the active path's block device does not implement the optional .ioctl method
> in its block_device_operations, won't this cause a NULL pointer dereference
> when user-space issues an ioctl on the multipath device node?

ok, I can add that check

> 
>> +	if (unlocked_ioctl_data) {
>> +		mpath_head->mpdt->ioctl_finish(unlocked_ioctl_data);
>> +		return err;
>> +	}
>> +out_unlock:
>> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
>> +	return err;
>> +}
>> +
> 
> [ ... ]
> 
>> @@ -701,6 +733,8 @@ const struct block_device_operations mpath_ops = {
>>   	.open		= mpath_bdev_open,
>>   	.release	= mpath_bdev_release,
>>   	.submit_bio	= mpath_bdev_submit_bio,
>> +	.ioctl		= mpath_bdev_ioctl,
>> +	.compat_ioctl	= blkdev_compat_ptr_ioctl,
> 
> [Severity: High]
> Does using the generic blkdev_compat_ptr_ioctl wrapper here bypass the
> underlying block device's custom .compat_ioctl handler?
> 
> If a 32-bit user space task invokes a compat ioctl, the core block layer will
> delegate to this generic wrapper, which routes the call to the native
> mpath_bdev_ioctl().
> 
> That native handler then unconditionally forwards the call to the underlying
> path device's native .ioctl method, entirely skipping the underlying device's
> .compat_ioctl handler.
> 
> Could this lead to 32-bit struct layouts being passed directly into a 64-bit
> native .ioctl handler, potentially causing memory corruption or out-of-bounds
> reads if the underlying driver requires custom struct repacking?

ok, I can fix improve this for the .compat_ioctl handler to call into 
the underlying block device .compat_ioctl handler

> 
>> +	.report_zones	= mpath_bdev_report_zones,
>>   	.pr_ops		= &mpath_pr_ops,
>>   };
>