| Message ID | 20220725173528.849643-1-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] scsi: ufs: Increase the maximum data buffer size | expand |
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
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.
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,
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(+)