On 02/03/2026 02:50, Benjamin Marzinski wrote:
On Wed, Feb 25, 2026 at 03:36:06PM +0000, John Garry wrote:
Introduce a scsi_device head structure - scsi_mpath_head - to manage
multipathing for a scsi_device. This is similar to nvme_ns_head structure.
There is no reference in scsi_mpath_head to any disk, as this would be
mananged by the scsi_disk driver.
A list of scsi_mpath_head structures is managed to lookup for matching
multipathed scsi_device's. Matching is done through the scsi_device
unique id.
Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/scsi_multipath.c | 147 ++++++++++++++++++++++++++++++++++
drivers/scsi/scsi_sysfs.c | 3 +
include/scsi/scsi_multipath.h | 29 +++++++
3 files changed, 179 insertions(+)
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index 04e0bad3d9204..49316269fad8e 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -107,6 +178,7 @@ static void scsi_multipath_sdev_uninit(struct scsi_device
*sdev)
int scsi_mpath_dev_alloc(struct scsi_device *sdev)
{
+ struct scsi_mpath_head *scsi_mpath_head;
int ret;
if (!scsi_multipath)
@@ -127,13 +199,75 @@ int scsi_mpath_dev_alloc(struct scsi_device *sdev)
goto out_uninit;
}
+ scsi_mpath_head = scsi_mpath_find_head(sdev->scsi_mpath_dev);
+ if (scsi_mpath_head)
+ goto found;
+ /* scsi_mpath_disks_list lock held */
Typo. It should be "scsi_mpath_heads_list lock still held".
Yes
Also, why
split the locking between this function and scsi_mpath_find_head()? It
seems like it would be clearer if you did in all here.
Maybe that is better - I'll consider it further.
+ scsi_mpath_head = scsi_mpath_alloc_head();
+ if (!scsi_mpath_head)
+ goto out_uninit;
It seems resonable to failback to treating the device as non-multipathed
if you can't setup the multipathing resources. But you should probably
warn if that happens.
OK, I can use pr_err or pr_warn if that happens
+
+ strcpy(scsi_mpath_head->wwid, sdev->scsi_mpath_dev->device_id_str);
+
+ ret = device_add(&scsi_mpath_head->dev);
+ if (ret)
+ goto out_put_head;
+
+ list_add_tail(&scsi_mpath_head->entry, &scsi_mpath_heads_list);
+
+ mutex_unlock(&scsi_mpath_heads_lock);
+ sdev->scsi_mpath_dev->scsi_mpath_head = scsi_mpath_head;
You already set sdev->scsi_mpath_dev->scsi_mpath_head right before you
return.
ok, I'll drop this duplicated code
+
+found:
+ sdev->scsi_mpath_dev->index = ida_alloc(&scsi_mpath_head->ida,
GFP_KERNEL);
+ if (sdev->scsi_mpath_dev->index < 0) {
+ ret = sdev->scsi_mpath_dev->index;
+ goto out_put_head;
&scsi_mpath_heads_lock is already unlocked here, but it will get
unlocked again in out_uninit
Yes, I will fix the locking/unlocking
+ }
+
+ mutex_lock(&scsi_mpath_head->lock);
+ scsi_mpath_head->dev_count++;
+ mutex_unlock(&scsi_mpath_head->lock);
+
+ sdev->scsi_mpath_dev->scsi_mpath_head = scsi_mpath_head;
return 0;
+out_put_head:
+ scsi_mpath_put_head(scsi_mpath_head);
out_uninit:
+ mutex_unlock(&scsi_mpath_heads_lock);
scsi_multipath_sdev_uninit(sdev);
return ret;
}
+static void scsi_mpath_remove_head(struct scsi_mpath_device *scsi_mpath_dev)
+{
+ struct scsi_mpath_head *scsi_mpath_head =
+ scsi_mpath_dev->scsi_mpath_head;
+ bool last_path = false;
+
+ mutex_lock(&scsi_mpath_head->lock);
+ scsi_mpath_head->dev_count--;
+ if (scsi_mpath_head->dev_count == 0)
+ last_path = true;
+ mutex_unlock(&scsi_mpath_head->lock);
The locking of scsi_mpath_head->lock makes it appear that
scsi_mpath_remove_head() and scsi_mpath_dev_alloc() can both happen at
the same time. I didn't check enough to verify if that's actually the
case, but if it's not, then the lock is unnecessary.
It should be possible for different sdevs
If they can run at
the same time, then I don't see anything keeping scsi_mpath_dev_alloc()
from calling scsi_mpath_find_head() and finding a scsi_mpath_head that
is just about to have its device deleted by
device_del(&scsi_mpath_head->dev). If this happens, the device won't
get re-added.
Yeah, I think that there might be a problem here. Let me check it further.
Thanks!
