On 10/03/2026 02:40, Benjamin Marzinski wrote:
+static int sd_mpath_probe(struct scsi_disk *sdkp) +{ + struct scsi_device *sdp = sdkp->device; + struct scsi_mpath_device *scsi_mpath_dev = sdp->scsi_mpath_dev; + struct device *dma_dev = sdp->host->dma_dev; + struct scsi_mpath_head *scsi_mpath_head = + scsi_mpath_dev->scsi_mpath_head; + struct sd_mpath_disk *sd_mpath_disk; + struct mpath_head *mpath_head = scsi_mpath_head->mpath_head; + struct queue_limits lim; + struct gendisk *disk; + int error; + + /* + * sd_mpath_disks_list is kept locked if no disk found. + * Otherwise an extra reference is taken. + */Again, I personally think the logic is easier to follow when all the locking isn't split over multiple functions.
Sure, but I am considering removing the mpath_disk structure, so things may change here anyway. Removing mpath_disk should simplify things for the nvme driver transition.
+ sd_mpath_disk = sd_mpath_find_disk(sdp); + if (sd_mpath_disk) { + mutex_lock(&sd_mpath_disk->lock); + sd_mpath_disk->disk_count++; + mutex_unlock(&sd_mpath_disk->lock); + goto found; + } + + sd_mpath_disk = kzalloc(sizeof(*sd_mpath_disk), GFP_KERNEL); + if (!sd_mpath_disk) { + error = -ENOMEM; + goto out_unlock; + } + + sd_mpath_disk->scsi_mpath_head = scsi_mpath_head; + device_initialize(&sd_mpath_disk->dev); + mutex_init(&sd_mpath_disk->lock); + sd_mpath_disk->dev.class = &sd_mpath_disk_class; + + blk_set_stacking_limits(&lim); + lim.dma_alignment = 3; + lim.features |= BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT | + BLK_FEAT_POLL | BLK_FEAT_ATOMIC_WRITES; + + sd_mpath_disk->mpath_disk = mpath_alloc_head_disk(&lim, + dev_to_node(dma_dev)); + if (!sd_mpath_disk->mpath_disk) { + error = -ENOMEM; + goto out_free_disk; + } + disk = sd_mpath_disk->mpath_disk->disk; + mpath_get_head(mpath_head); /* undone in mpath_free_disk() */ + + sd_mpath_disk->mpath_disk->mpath_head = mpath_head; + sd_mpath_disk->mpath_disk->parent = &sd_mpath_disk->dev; + + error = ida_alloc(&sd_index_ida, GFP_KERNEL); + if (error < 0) { + sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n"); + goto out_put_disk; + } + sd_mpath_disk->disk_index = error; + error = sd_format_disk_name("sd", sd_mpath_disk->disk_index, + disk->disk_name, DISK_NAME_LEN); + if (error) + goto out_free_index; + + error = dev_set_name(&sd_mpath_disk->dev, "%s", + dev_name(&scsi_mpath_head->dev)); + if (error) + goto out_free_index; + + /* undone in sd_mpath_disk_release() */ + scsi_mpath_get_head(scsi_mpath_head); + + error = device_add(&sd_mpath_disk->dev); + if (error) { + put_device(&sd_mpath_disk->dev); + goto out_unlock;We should clean up when we fail here, instead of just unlocking without fully setting things up.
I think that the release function is called from put_device(), which should do the class tidy up. Something similar is done in sd_probe() for the disk_dev.
Thanks, John
