Re: [PATCH v4 23/28] scsi: sd: add multipath disk class

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Empty device release callback `sd_mpath_disk_release` bypasses driver core checks and is a known anti-pattern.
- [Low] Indentation of `#ifdef` preprocessor directives inside a struct definition.
--

--- Patch [23]: [PATCH v4 23/28] scsi: sd: add multipath disk class ---
commit 99b2103695f69e8e73bd9a9db23c590d7c822e26
Author: John Garry <john.g.garry@oracle.com>

scsi: sd: add multipath disk class
    
This commit introduces sd_mpath_disk_class, a new multipath version of the
scsi_disk class, and the sd_mpath_disk structure to manage multipath
gendisks.

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 599e75f333343..242a15bc2c5bb 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
[ ... ]
> @@ -113,6 +114,39 @@ static mempool_t *sd_page_pool;
>  static mempool_t *sd_large_page_pool;
>  static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
>  static struct lock_class_key sd_bio_compl_lkclass;
> +#ifdef CONFIG_SCSI_MULTIPATH
> +struct sd_mpath_disk {
> +	struct scsi_mpath_head		*scsi_mpath_head;
> +};
> +
> +static void sd_mpath_disk_release(struct device *dev)
> +{
> +}

[Severity: Low]
Does this empty release callback intentionally bypass the driver core checks?

While this is currently unreachable as no devices are registered to this
class yet, providing an empty release function just to silence the driver
core's missing-release warning is a known anti-pattern. This will cause a
permanent memory leak when devices are eventually assigned to this class and
need to be freed.

> +
> +static const struct class sd_mpath_disk_class = {
> +	.name = "scsi_mpath_disk",
> +	.dev_release = sd_mpath_disk_release,
> +};
[ ... ]
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 574af82430169..304b24644d942 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -83,6 +83,9 @@ struct zoned_disk_info {
>  
>  struct scsi_disk {
>  	struct scsi_device *device;
> +	#ifdef CONFIG_SCSI_MULTIPATH
> +	struct sd_mpath_disk *sd_mpath_disk;
> +	#endif

[Severity: Low]
This isn't a bug, but should these preprocessor directives start at column 0?

Indenting preprocessor directives inside the struct definition doesn't align
with the standard kernel coding style guidelines.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723093627.2327456-1-john.g.garry@oracle.com?part=23