RE: [PATCH v2] scsi: core: Fix shost->cmd_per_lun calculation in scsi_add_host_with_dma()

[Dexuan Cui]

> From: John Garry <john.garry@xxxxxxxxxx>
> Sent: Thursday, October 7, 2021 2:42 PM
> 
> On 07/10/2021 18:49, Dexuan Cui wrote:
> > After commit ea2f0f77538c, a 416-CPU VM running on Hyper-V hangs during
> > boot because scsi_add_host_with_dma() sets shost->cmd_per_lun to a
> > negative number (the below numbers may differ in different kernel versions):
> > in drivers/scsi/storvsc_drv.c, 	storvsc_drv_init() sets
> > 'max_outstanding_req_per_channel' to 352, and storvsc_probe() sets
> > 'max_sub_channels' to (416 - 1) / 4 = 103 and sets scsi_driver.can_queue to
> > 352 * (103 + 1) * (100 - 10) / 100 = 32947, which exceeds SHRT_MAX.
> 
> I think that you just need to mention that if can_queue exceeds
> SHRT_MAX, then there is a data truncation issue.

I just hoped the explanation how the too big 'can_queue' value is generated is helpful.

OK, I think I can change the commit log to:
 
After commit ea2f0f77538c, a 416-CPU VM running on Hyper-V hangs during
boot because the hv_storvsc driver sets scsi_driver.can_queue to an "int"
value that exceeds SHRT_MAX, and hence scsi_add_host_with_dma() sets
shost->cmd_per_lun to a negative "short" number. 

Use min_t(int, ...) to fix the issue.

> It looks ok, I'd just like to test it a bit more.
> 
> Thanks,
> John

Thanks! I'll post v3 with the above commit log, and I look forward to your review/test.

> > v2 directly fixes the scsi core change instead as Michael Kelley and
> > John Garry suggested (refer to the above link).
> 
> To be fair, it was Michael's suggestion
 
Yeah. Michael always gives good suggstions when reviewing patches. :-)