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

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

[Bart Van Assche]

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.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Bean Huo <beanhuo@micron.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 36b7212e9cb5..a6fddf4c546e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8365,6 +8365,7 @@ static struct scsi_host_template ufshcd_driver_template = {
 	.cmd_per_lun		= UFSHCD_CMD_PER_LUN,
 	.can_queue		= UFSHCD_CAN_QUEUE,
 	.max_segment_size	= PRDT_DATA_BYTE_COUNT_MAX,
+	.max_sectors		= 65535 * 4096 / SECTOR_SIZE, /* 255 MiB */
 	.max_host_blocked	= 1,
 	.track_queue_depth	= 1,
 	.sdev_groups		= ufshcd_driver_groups,

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

[Avri Altman]

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

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

[Bart Van Assche]

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.