Re: [PATCH v2] scsi: ufs: Increase the maximum data buffer size

[Bart Van Assche <bvanassche@acm.org>]

On 7/26/22 05:54, Avri Altman wrote:
> Bart hi,
> 
>> Measurements have shown that for some UFS devices the maximum sequential
>> I/O throughput is achieved with a transfer size above 512 KiB. Hence
>> increase the maximum size of the data buffer associated with a single
>> request from SCSI_DEFAULT_MAX_SECTORS (1024) * 512 bytes = 512 KiB into
>> 255 MiB.
>>
>> Notes:
>> - The maximum data buffer size supported by the UFSHCI specification
>>    is 65535 * 256 KiB or about 16 GiB.
>> - The maximum data buffer size for READ(10) commands is 65535 logical
>>    blocks. To transfer more than 65535 * 4096 bytes = 255 MiB with a
>>    single SCSI command, the READ(16) command is required. Support for
>>    READ(16) is optional in the UFS 3.1 and UFS 4.0 standards.
> I still have concerns of a negative impact of a too-large-max-sectors.
> 
> I replicate your fio measurements using galaxy S22 - one of the most advanced production platform currently in the market.
> See the results below: fio bs vs max-sectors.  data is the read BW[MiB/s].
> Given that:
> a) there isn't that much of a difference among the various max-sectors
> b) max-sectors is configurable via max_sectors_kb sysfs entry, and
> c) going from the benchmark world into the real world, a large write (255MiB),
>      interleaved by many small reads (4k), may cause high latency, up to even a timeout (!?)
> 
> I would leave the max-sectors threshold as it is, or use the moderate 1MB limit.
> Unless you can back the  "Measurements have shown" statement with a more concrete data.
> 
> Thanks,
> Avri
> 
> 
> 	|	1MB		|	2MB		|	128MB		|	255MB
> --------------------------------------------------------------------------------------------------------------------
> 4KiB	|	14.7		|	13.3		|	15.7		|	19.5
> 8KiB	|	26.7		|	27.4		|	25.8		|	30.0
> 16KiB	|	44		|	53.2		|	46.3		|	61.2
> 64KiB	|	179		|	207		|	171		|	184
> 32KiB	|	100		|	120		|	105		|	119
> 128KiB	|	275		|	322		|	271		|	256
> 256KiB	|	440		|	452		|	417		|	378
> 512KiB	|	535		|	525		|	483		|	471
> 1MiB	|	592		|	584		|	531		|	408
> 2MiB	|	349		|	361		|	375		|	297
> 4MiB	|	502		|	492		|	515		|	441
> 8MiB	|	645		|	663		|	682		|	650
> 16MiB	|	752		|	749		|	752		|	762
> 32MiB	|	817		|	790		|	822		|	791
> 64MiB	|	1710		|	1703		|	1700		|	1692
> 128MiB	|	1733		|	1730		|	1724		|	1712
> 256MiB	|	1752		|	1748		|	1738		|	1732

Hi Avri,

Thank you for having run these measurements. Regarding (b): the 
max_sectors_kb sysfs entry can be used to reduce the value set by a SCSI 
LLD but not to increase that value. From block/blk-sysfs.c:

	if (max_sectors_kb > max_hw_sectors_kb)
		return -EINVAL;

I will post a new version of this patch with the max_sectors limit set 
to 1 MiB.

Thanks,

Bart.