RE: [PATCH 1/3] scsi: storvsc: Fix max_outstanding_req_per_channel for Win8 and newer

[Michael Kelley]

From: Andrea Parri (Microsoft) <parri.andrea@xxxxxxxxx> Sent: Thursday, December 17, 2020 12:33 PM
> 
> Current code overestimates the value of max_outstanding_req_per_channel
> for Win8 and newer hosts, since vmscsi_size_delta is set to the initial
> value of sizeof(vmscsi_win8_extension) rather than zero.  This may lead
> to wrong decisions when using ring_avail_percent_lowater equals to zero.
> The estimate of max_outstanding_req_per_channel is 'exact' for Win7 and
> older hosts.  A better choice, keeping the algorithm for the estimation
> simple, is to err the other way around, i.e., to underestimate for Win7
> and older but to use the exact value for Win8 and newer.
> 
> Suggested-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@xxxxxxxxx>
> Cc: "James E.J. Bottomley" <jejb@xxxxxxxxxxxxx>
> Cc: "Martin K. Petersen" <martin.petersen@xxxxxxxxxx>
> Cc: linux-scsi@xxxxxxxxxxxxxxx
> ---
>  drivers/scsi/storvsc_drv.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index ded00a89bfc4e..64298aa2f151e 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -2141,12 +2141,15 @@ static int __init storvsc_drv_init(void)
>  	 * than the ring buffer size since that page is reserved for
>  	 * the ring buffer indices) by the max request size (which is
>  	 * vmbus_channel_packet_multipage_buffer + struct vstor_packet + u64)
> +	 *
> +	 * The computation underestimates max_outstanding_req_per_channel
> +	 * for Win7 and older hosts because it does not take into account
> +	 * the vmscsi_size_delta correction to the max request size.
>  	 */
>  	max_outstanding_req_per_channel =
>  		((storvsc_ringbuffer_size - PAGE_SIZE) /
>  		ALIGN(MAX_MULTIPAGE_BUFFER_PACKET +
> -		sizeof(struct vstor_packet) + sizeof(u64) -
> -		vmscsi_size_delta,
> +		sizeof(struct vstor_packet) + sizeof(u64),
>  		sizeof(u64)));
> 
>  #if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
> --
> 2.25.1

Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>