Re: [PATCH v6 1/6] ata: libata-scsi: add atapi_max_lun module parameter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Logic and documentation mismatch in `atapi_max_lun` leading to an off-by-one configuration error.
--

commit 62d1cf766cb69ddd660120719652195586b409e8
Author: Phil Pemberton <philpem@philpem.me.uk>

ata: libata-scsi: add atapi_max_lun module parameter

This commit introduces a libata module parameter atapi_max_lun to control
the upper bound of the per-host SCSI LUN scan for ATAPI devices. The
default is 1 to preserve existing behavior, but can be clamped up to
ATAPI_MAX_LUN (8) to support multi-LUN ATAPI devices.

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index e76d15411e2ae..4408b1fb48c77 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c

[ ... ]

> @@ -122,6 +122,11 @@ int atapi_passthru16 = 1;
>  module_param(atapi_passthru16, int, 0444);
>  MODULE_PARM_DESC(atapi_passthru16, "Enable ATA_16 passthru for ATAPI devices (0=off, 1=on [default])");
>  
> +int atapi_max_lun = 1;
> +module_param(atapi_max_lun, int, 0444);
> +MODULE_PARM_DESC(atapi_max_lun,
> +	"Maximum LUN to scan on ATAPI devices flagged BLIST_FORCELUN (1 [default] .. 7)");

[Severity: Medium]
Is this documentation correct about the upper limit being 7?

The SCSI core treats shost->max_lun as an exclusive upper bound, meaning it
represents the total count of LUNs to scan. If a user sets atapi_max_lun to 7
based on this description, only LUNs 0 through 6 will be scanned, missing
LUN 7.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index f44612e269a4d..32c6a0e497cf9 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c

[ ... ]

> @@ -4627,7 +4627,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
>  		shost->transportt = &ata_scsi_transportt;
>  		shost->unique_id = ap->print_id;
>  		shost->max_id = 16;
> -		shost->max_lun = 1;
> +		shost->max_lun = clamp(atapi_max_lun, 1, ATAPI_MAX_LUN);

[Severity: Medium]
Since ATAPI_MAX_LUN is 8, the clamp allows a value of 8. To correctly scan up
to LUN 7, should the module parameter description be updated to indicate the
maximum value is 8? Or does atapi_max_lun require a +1 adjustment when
assigning it to shost->max_lun in ata_scsi_add_hosts()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608213443.2296614-1-philpem@philpem.me.uk?part=1