Re: [PATCH v9 04/11] scsi: sr: initialize logical block size before registering the disk

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: sr: physical_block_size permanently stuck at 2048 after hot-plugging 512-byte media
--

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

scsi: sr: initialize logical block size before registering the disk

This commit updates the logical block size of the request queue to match
the driver's default sector size (2048 bytes) if the drive is empty during
probe. This attempts to prevent mismatched block sizes when reading from
media inserted later.

> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index c36c54ecd354b..268f15a4fdb88 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c

[ ... ]

> @@ -701,6 +702,16 @@ static int sr_probe(struct scsi_device *sdev)
>  	if (error)
>  		goto unregister_cdrom;
>  
> +	/* If no medium was ready, match the queue to the driver's guess. */
> +	if (queue_logical_block_size(sdev->request_queue) != sdev->sector_size) {
> +		lim = queue_limits_start_update(sdev->request_queue);
> +		lim.logical_block_size = sdev->sector_size;
> +		error = queue_limits_commit_update_frozen(sdev->request_queue,
> +							  &lim);
> +		if (error)
> +			goto unregister_cdrom;
> +	}
> +

[Severity: High]
Does this logic introduce a regression where physical_block_size becomes
permanently stuck at 2048 when a 512-byte medium is inserted later?

If a drive is empty during sr_probe(), this code promotes the
logical_block_size to 2048. Because the block layer one-way promotes
physical_block_size to match logical_block_size, blk_validate_limits()
will automatically latch physical_block_size to 2048:

block/blk-settings.c:blk_validate_limits() {
        ...
        if (lim->physical_block_size < lim->logical_block_size) {
                lim->physical_block_size = lim->logical_block_size;
        ...
}

When a 512-byte medium is later hot-plugged, sr_revalidate_disk() shrinks
logical_block_size back to 512:

drivers/scsi/sr.c:sr_revalidate_disk() {
        ...
        lim = queue_limits_start_update(q);
        lim.logical_block_size = sector_size;
        lim.features |= BLK_FEAT_ROTATIONAL;
        return queue_limits_commit_update_frozen(q, &lim);
        ...
}

However, blk_validate_limits() never automatically shrinks
physical_block_size. Could this leave physical_block_size and io_min
permanently at 2048, continuing to cause mismatched block size issues?

>  	error = device_add_disk(&sdev->sdev_gendev, disk, NULL);
>  	if (error)
>  		goto unregister_cdrom;

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