Re: [PATCH v2] scsi: sd: Set a default optimal IO size if one is not defined

[Damien Le Moal <dlemoal@kernel.org>]

On 6/12/25 00:35, Bart Van Assche wrote:
> On 6/11/25 5:29 AM, Damien Le Moal wrote:
>> +static void sd_set_io_opt(struct scsi_disk *sdkp, unsigned int dev_max,
>> +			  struct queue_limits *lim)
>> +{
>> +	struct scsi_device *sdp = sdkp->device;
>> +	struct Scsi_Host *shost = sdp->host;
>> +
>> +	lim->io_opt = shost->opt_sectors << SECTOR_SHIFT;
>> +	if (sd_validate_opt_xfer_size(sdkp, dev_max))
>> +		lim->io_opt = min_not_zero(lim->io_opt,
>> +				logical_to_bytes(sdp, sdkp->opt_xfer_blocks));
>> +	if (!lim->io_opt) {
>> +		lim->io_opt = ALIGN_DOWN(lim->max_sectors << SECTOR_SHIFT,
>> +					 sdkp->physical_block_size - 1);
>> +		sd_first_printk(KERN_INFO, sdkp,
>> +			"Using default optimal transfer size of %u bytes\n",
>> +			lim->io_opt);
>> +	}
>> +}
> 
> shost->opt_sectors and lim->max_sectors both have type unsigned int so
> when shifting these left there is a risk of overflow. As an example,
> from the scsi_debug driver:
> 
> 	.max_sectors =		-1U,

The code already was like that for opt_sectors and no-one complained.

> Shouldn't integer overflows be prevented instead of letting these
> happen?

Sure, can do that.


-- 
Damien Le Moal
Western Digital Research