Re: [PATCH v6 12/27] scsi-multipath: introduce scsi_device head structure

On 29/07/2026 14:46, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] The struct `mpath_head_template smpdt` is defined completely empty, lacking logically required callbacks, leading to NULL pointer dereferences.
> - [Medium] The patch omits the logic to create sysfs links to member `scsi_device`'s, contradicting the stated commit message.
> - [Low] The function `scsi_multipath_sysfs_group_visible` and the macro `DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(scsi_multipath_sysfs)` generate unused static code, causing a `-Wunused-function` warning.
> --
> 
> --- Patch [12]: [PATCH v6 12/27] scsi-multipath: introduce scsi_device head structure ---
> commit 41b0248171de9edac7171ac10282e0c5b3f1056c
> Author: John Garry <[email protected]>
> 
> scsi-multipath: introduce scsi_device head structure
> 
> Introduce a scsi_mpath_head structure to manage multipathing for a
> scsi_device. A list of scsi_mpath_head structures is managed to lookup for
> matching multipathed scsi_device's.
> 
>> The sysfs device folder will have links to the scsi_device's so, it will
>> be possible to lookup the member scsi_device's.
> 
> [Severity: Medium]
> I notice the patch doesn't seem to contain the code to create these sysfs links
> to the member scsi_device structures. Is this implementation missing, or is it
> planned for a future patch?

future patch

> 
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index 5559e2e8d64a8..30a2d01be8aaf 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
> 
> [ ... ]
> 
>> +static const struct attribute_group scsi_mpath_device_attrs_group = {
>> +	.attrs = scsi_mpath_device_attrs,
>> +};
>> +
>> +static bool scsi_multipath_sysfs_group_visible(struct kobject *kobj)
>> +{
>> +	return true;
>> +}
>> +DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(scsi_multipath_sysfs)
> 
> [Severity: Low]
> Is this function and macro used anywhere in this patch?
> 
> It looks like DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE generates a static function
> that isn't assigned to any attribute_group.is_visible callback, which might
> cause a unused-function compiler warning.

yes, it is used later. For me, the compiler does not complain as 
DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE uses inline in the function declaration

> 
>> +
>> +static const struct attribute_group *scsi_mpath_device_groups[] = {
>> +	&scsi_mpath_device_attrs_group,
>> +	NULL
>> +};
> 
> [ ... ]
> 
>>   static int scsi_multipath_sdev_init(struct scsi_device *sdev)
>>   {
>>   	struct Scsi_Host *shost = sdev->host;
>> @@ -91,6 +144,74 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
>>   	return 0;
>>   }
>>   
>> +static struct mpath_head_template smpdt = {
>> +};
> 
> [Severity: High]
> Will this cause a null pointer dereference when the generic multipath core
> attempts to evaluate I/O paths?
> 
> Since smpdt is defined completely empty without any callback assignments,
> function pointers like mpath_head->mpdt->is_disabled will be null when
> dereferenced by the framework.

it is filled in later

> 
>> +
>> +static struct scsi_mpath_head *scsi_mpath_alloc_head(char *vpd_id)
>> +{
>> +	struct scsi_mpath_head *scsi_mpath_head;
> 
> [ ... ]
>