Re: [PATCH v5 27/27] scsi: sd: add mpath_queue_depth dev attribute

[John Garry <john.g.garry@oracle.com>]

On 27/07/2026 16:28, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The sysfs show function `sd_mpath_queue_depth_show` returns 0 when the iopolicy is not QD, causing reads to return EOF (0 bytes).
> --
> 
> --- Patch [27]: [PATCH v5 27/27] scsi: sd: add mpath_queue_depth dev attribute ---
> commit a1c72efbc115f45170fc3510049d2e925ee24a95
> Author: John Garry <john.garry@linux.dev>
> 
> scsi: sd: add mpath_queue_depth dev attribute
> 
> Add a queue_depth file so that the multipath dynamic queue depth can be
> looked up from per-path gendisk (scsi_disk) directory.
> 
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index 6b6aef361ae22..fbf9da686f25d 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -4159,9 +4159,28 @@ static ssize_t sd_mpath_numa_nodes_show(struct device *dev,
>>   }
>>   static DEVICE_ATTR(mpath_numa_nodes, 0444, sd_mpath_numa_nodes_show, NULL);
>>   
>> +static ssize_t sd_mpath_queue_depth_show(struct device *dev,
>> +		struct device_attribute *attr, char *buf)
>> +{
>> +	struct gendisk *gd = dev_to_disk(dev);
>> +	struct scsi_disk *sdkp = gd->private_data;
>> +	struct scsi_device *sdev = sdkp->device;
>> +	struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
>> +	struct scsi_mpath_head *scsi_mpath_head =
>> +				scsi_mpath_dev->scsi_mpath_head;
>> +	struct Scsi_Host *shost = sdev->host;
>> +
>> +	if (!mpath_qd_iopolicy(&scsi_mpath_head->iopolicy))
>> +		return 0;
> 
> [Severity: Medium]
> Should this return a standard negative error code like -EOPNOTSUPP instead of 0?
> 
> Returning 0 from a sysfs show callback means zero bytes were written to the
> output buffer, causing user-space reads to receive an immediate EOF and see a
> completely empty file instead of parsing an integer or receiving an error.

No, this is same as what NVMe does

> 
>> +
>> +	return sysfs_emit(buf, "%d\n", atomic_read(&shost->mpath_nr_active));
>> +}
>