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

[Nilay Shroff <nilay@linux.ibm.com>]

On 2/25/26 9:02 PM, John Garry wrote:
> 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 | 15 +++++++
>   lib/multipath.c           | 92 ++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/multipath.h b/include/linux/multipath.h
> index c964a1aba9c42..d557fb9bab4c9 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;
> @@ -40,10 +41,12 @@ struct mpath_device {
>   };
>   
>   struct mpath_head_template {
> +	bool (*available_path)(struct mpath_device *, bool *);
>   	bool (*is_disabled)(struct mpath_device *);
>   	bool (*is_optimized)(struct mpath_device *);
>   	enum mpath_access_state (*get_access_state)(struct mpath_device *);
>   	enum mpath_iopolicy_e (*get_iopolicy)(struct mpath_head *);
> +	struct bio *(*clone_bio)(struct bio *);
>   	const struct attribute_group **device_groups;
>   };
>   
> @@ -56,12 +59,23 @@ struct mpath_head {
>   
>   	struct kref		ref;
>   
> +	struct bio_list		requeue_list; /* list for requeing bio */
> +	spinlock_t		requeue_lock;
> +	struct work_struct	requeue_work; /* work struct for requeue */
> +
>   	unsigned long		flags;
>   	struct mpath_device __rcu 		*current_path[MAX_NUMNODES];
>   	const struct mpath_head_template	*mpdt;
>   	void			*drvdata;
>   };
>   
> +#define REQ_MPATH		REQ_DRV
> +
> +static inline bool is_mpath_request(struct request *req)
> +{
> +	return req->cmd_flags & REQ_MPATH;
> +}
> +
>   static inline struct mpath_disk *mpath_bd_device_to_disk(struct device *dev)
>   {
>   	return dev_get_drvdata(dev);
> @@ -82,6 +96,7 @@ int mpath_set_iopolicy(const char *val, int *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);
> +void mpath_requeue_work(struct work_struct *work);
>   struct mpath_head *mpath_alloc_head(void);
>   void mpath_put_disk(struct mpath_disk *mpath_disk);
>   void mpath_remove_disk(struct mpath_disk *mpath_disk);
> diff --git a/lib/multipath.c b/lib/multipath.c
> index 65a0d2d2bf524..b494b35e8dccc 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);
>   
> @@ -227,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 =
> @@ -243,6 +243,66 @@ 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)) {
> +		bool available = false;
> +
> +		if (!mpath_head->mpdt->available_path(mpath_device,
> +				&available))
> +			continue;
> +		if (available)
> +			return true;
> +	}
> +
> +	return false;
> +}

IMO, we may further simplify the callback ->available_path() to return 
true or false instead of passing the result in a separate @available 
argument.

Thanks,
--Nilay