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

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

[Bart Van Assche]

Measurements for one particular UFS controller + UFS device show a 25%
higher read bandwidth if the maximum data buffer size is increased from
512 KiB to 1 MiB. Hence increase the maximum size of the data buffer
associated with a single request from SCSI_DEFAULT_MAX_SECTORS (1024)
* 512 bytes = 512 KiB to 1 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>
---
Changes compared to v2: changed maximum transfer size 255 MiB to 1 MiB.
Changes compared to v1: changed maximum transfer size from 1 GiB to 255 MiB.

 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..678bc8d6d6aa 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		= (1 << 20) / SECTOR_SIZE, /* 1 MiB */
 	.max_host_blocked	= 1,
 	.track_queue_depth	= 1,
 	.sdev_groups		= ufshcd_driver_groups,

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

[Avri Altman]

> Measurements for one particular UFS controller + UFS device show a 25%
> higher read bandwidth if the maximum data buffer size is increased from
> 512 KiB to 1 MiB. Hence increase the maximum size of the data buffer
> associated with a single request from SCSI_DEFAULT_MAX_SECTORS (1024)
> * 512 bytes = 512 KiB to 1 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>
Acked-by: Avri Altman <avri.altman@wdc.com>
Tested-by: Avri Altman <avri.altman@wdc.com>

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

[Martin K. Petersen]

Bart,

> Measurements for one particular UFS controller + UFS device show a 25%
> higher read bandwidth if the maximum data buffer size is increased from
> 512 KiB to 1 MiB. Hence increase the maximum size of the data buffer
> associated with a single request from SCSI_DEFAULT_MAX_SECTORS (1024)
> * 512 bytes = 512 KiB to 1 MiB.

Applied to 5.20/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

[정요한(JOUNG YOHAN) Mobile SE]

Hi bart

Is it possible by adding only max_sector to increase the data buffer size?
I think the data buffer will split to 512 KiB, because the sg_table size is SG_ALL

Thanks,
yohan

> -----Original Message-----
> From: Bart Van Assche <bvanassche@acm.org>
> Sent: Wednesday, July 27, 2022 7:52 AM
> To: Martin K . Petersen <martin.petersen@oracle.com>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>; linux-scsi@vger.kernel.org; Adrian
> Hunter <adrian.hunter@intel.com>; Bart Van Assche <bvanassche@acm.org>;
> Avri Altman <avri.altman@wdc.com>; Bean Huo <beanhuo@micron.com>; Stanley
> Chu <stanley.chu@mediatek.com>; James E.J. Bottomley <jejb@linux.ibm.com>;
> Matthias Brugger <matthias.bgg@gmail.com>
> Subject: [PATCH v3] scsi: ufs: Increase the maximum data buffer size
> 
> Measurements for one particular UFS controller + UFS device show a 25%
> higher read bandwidth if the maximum data buffer size is increased from
> 512 KiB to 1 MiB. Hence increase the maximum size of the data buffer
> associated with a single request from SCSI_DEFAULT_MAX_SECTORS (1024)
> * 512 bytes = 512 KiB to 1 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>
> ---
> Changes compared to v2: changed maximum transfer size 255 MiB to 1 MiB.
> Changes compared to v1: changed maximum transfer size from 1 GiB to 255 MiB.
> 
>  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..678bc8d6d6aa 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		= (1 << 20) / SECTOR_SIZE, /* 1 MiB */
>  	.max_host_blocked	= 1,
>  	.track_queue_depth	= 1,
>  	.sdev_groups		= ufshcd_driver_groups,

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

[Bart Van Assche]

On 8/2/22 16:40, yohan.joung@sk.com wrote:
> Is it possible by adding only max_sector to increase the data buffer size?

Yes.

> I think the data buffer will split to 512 KiB, because the sg_table size is SG_ALL

I don't think so. With this patch applied, the limits supported by the 
UFS driver are as follows:

	.sg_tablesize		= SG_ALL,                   /* 128 */
  	.max_segment_size	= PRDT_DATA_BYTE_COUNT_MAX, /* 256 KiB*/
	.max_sectors		= (1 << 20) / SECTOR_SIZE,  /* 1 MiB */

So the maximum data buffer size is min(max_sectors * 512, sg_tablesize * 
max_segment_size) = min(1 MiB, 128 * 256 KiB) = 1 MiB. On a system with 
4 KiB pages, the data buffer size will be 128 * 4 KiB = 512 MiB if none 
of the pages involved in the I/O are contiguous.

Bart.

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

[정요한(JOUNG YOHAN) Mobile SE]

> On 8/2/22 16:40, yohan.joung@sk.com wrote:
> > Is it possible by adding only max_sector to increase the data buffer size?
> 
> Yes.
> 
> > I think the data buffer will split to 512 KiB, because the sg_table
> > size is SG_ALL
> 
> I don't think so. With this patch applied, the limits supported by the UFS
> driver are as follows:
> 
> 	.sg_tablesize		= SG_ALL,                   /* 128 */
>   	.max_segment_size	= PRDT_DATA_BYTE_COUNT_MAX, /* 256 KiB*/
> 	.max_sectors		= (1 << 20) / SECTOR_SIZE,  /* 1 MiB */
> 
> So the maximum data buffer size is min(max_sectors * 512, sg_tablesize *
> max_segment_size) = min(1 MiB, 128 * 256 KiB) = 1 MiB. On a system with
> 4 KiB pages, the data buffer size will be 128 * 4 KiB = 512 MiB if none of
> the pages involved in the I/O are contiguous.
In block layer, max_segment_size is obtained from get_max_segment_size.
seg_boundary_mask is set to PAGE_SIZE - 1 in the ufs driver.
The segment size is the PAGE size, and the max buffer size is
segment size * max segment count ( PAGE SIZE * 128 ) = 512 KiB  in block layer
Right? 
>
> Bart.

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

[Bart Van Assche]

On 8/3/22 18:50, yohan.joung@sk.com wrote:
> In block layer, max_segment_size is obtained from get_max_segment_size.
> seg_boundary_mask is set to PAGE_SIZE - 1 in the ufs driver.
> The segment size is the PAGE size, and the max buffer size is
> segment size * max segment count ( PAGE SIZE * 128 ) = 512 KiB  in block layer
> Right?

Thanks for having reported this. I had overlooked that the UFS host 
controller driver sets the dma_boundary member of the host_template 
field. Is my understanding correct that UFS host controllers should 
support DMA segments that consist of multiple physical pages?

Thanks,

Bart.

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

[정요한(JOUNG YOHAN) Mobile SE]

> > In block layer, max_segment_size is obtained from get_max_segment_size.
> > seg_boundary_mask is set to PAGE_SIZE - 1 in the ufs driver.
> > The segment size is the PAGE size, and the max buffer size is segment
> > size * max segment count ( PAGE SIZE * 128 ) = 512 KiB  in block layer
> > Right?
> 
> Thanks for having reported this. I had overlooked that the UFS host
> controller driver sets the dma_boundary member of the host_template field.
> Is my understanding correct that UFS host controllers should support DMA
> segments that consist of multiple physical pages?
max value of data byte count is 256kb in PRDT. 
256kb (data byte count) * 128 (max segments) = 32768kb
If physical pages are continuous, it seems that IO can be delivered up to 32mb.
performance checks are required, but we don't have to consider saturation.
because the device vendor can set opt_xfer_blocks .
Thanks, 
yohan
> 
> Thanks,
> 
> Bart.

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

[Bean Huo]

On Wed, 2022-08-03 at 09:23 -0700, Bart Van Assche wrote:
> On 8/2/22 16:40, yohan.joung@sk.com wrote:
> > Is it possible by adding only max_sector to increase the data
> > buffer size?
> 
> Yes.
> 
> > I think the data buffer will split to 512 KiB, because the sg_table
> > size is SG_ALL
> 
> I don't think so. With this patch applied, the limits supported by
> the 
> UFS driver are as follows:
> 
>         .sg_tablesize           = SG_ALL,                   /* 128 */
>         .max_segment_size       = PRDT_DATA_BYTE_COUNT_MAX, /* 256
> KiB*/
>         .max_sectors            = (1 << 20) / SECTOR_SIZE,  /* 1 MiB
> */
> 
> So the maximum data buffer size is min(max_sectors * 512,
> sg_tablesize * 
> max_segment_size) = min(1 MiB, 128 * 256 KiB) = 1 MiB. On a system
> with 
> 4 KiB pages, the data buffer size will be 128 * 4 KiB = 512 MiB if
> none 
> of the pages involved in the I/O are contiguous.
> 
> Bart.


Bart, 

This change just increases the shost->max_sectors limit from 501KB to
1Mb, but the final value will be overridden by the optimal transfer
length defined in the VPD, right?

Kind regards,
Bean

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

[Bart Van Assche]

On 9/2/22 07:52, Bean Huo wrote:
> On Wed, 2022-08-03 at 09:23 -0700, Bart Van Assche wrote:
>> On 8/2/22 16:40, yohan.joung@sk.com wrote:
>>> Is it possible by adding only max_sector to increase the data
>>> buffer size?
>>
>> Yes.
>>
>>> I think the data buffer will split to 512 KiB, because the sg_table
>>> size is SG_ALL
>>
>> I don't think so. With this patch applied, the limits supported by
>> the
>> UFS driver are as follows:
>>
>>          .sg_tablesize           = SG_ALL,                   /* 128 */
>>          .max_segment_size       = PRDT_DATA_BYTE_COUNT_MAX, /* 256
>> KiB*/
>>          .max_sectors            = (1 << 20) / SECTOR_SIZE,  /* 1 MiB
>> */
>>
>> So the maximum data buffer size is min(max_sectors * 512,
>> sg_tablesize *
>> max_segment_size) = min(1 MiB, 128 * 256 KiB) = 1 MiB. On a system
>> with
>> 4 KiB pages, the data buffer size will be 128 * 4 KiB = 512 MiB if
>> none
>> of the pages involved in the I/O are contiguous.
>
> This change just increases the shost->max_sectors limit from 501KB to
> 1Mb, but the final value will be overridden by the optimal transfer
> length defined in the VPD, right?

Hi Bean,

It seems to me that the block layer only uses the optimal transfer size 
(io_opt) to determine how much data to read ahead during sequential 
reads? See also disk_update_readahead().

The above patch increases max_sectors but is not sufficient to increase 
the maximum transfer size: SG_ALL (128) * 4 KiB (dma_boundary + 1) = 512 
KiB. To increase the maximum transfer size, the dma_boundary parameter 
would have to be modified. I have not yet submitted a patch that 
modifies that parameter since on my test setup (Exynos host controller) 
the current value is the largest value supported.

Thanks,

Bart.

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

[Bean Huo]

On Tue, 2022-09-06 at 11:04 -0700, Bart Van Assche wrote:
> 
> Hi Bean,
> 
> It seems to me that the block layer only uses the optimal transfer
> size 
> (io_opt) to determine how much data to read ahead during sequential 
> reads? See also disk_update_readahead().
> 

Bart,
Sorry for later reply, Didn't notice a question here. 

In the upstream standard Linux kernel, if the device supports a valid
VPD, I think, your this change will also not change the read-ahead
window.

....

sdkp->opt_xfer_blocks = get_unaligned_be32(&vpd->data[12]);


sd_revalidate_disk() {

...
 if (sd_validate_opt_xfer_size(sdkp, dev_max)) {
     q->limits.io_opt = logical_to_bytes(sdp, sdkp->opt_xfer_blocks);
     rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
 }	
...
}


device_add_disk()
	 disk_update_readahead(disk) {
		disk->bdi->ra_pages =
		max(queue_io_opt(q) * 2 / PAGE_SIZE,
VM_READAHEAD_PAGES); 
	}

e.g., if device reports opt 512KB, and the maximum transfer length will
be 512KB, and readahead window will be 1MB.


Kind regards,
Bean