Re: [PATCH v6 4/6] scsi: sd: Allow max_sectors be capped at DMA optimal size limit

Damien Le Moal
2022-07-18T10:47:37Z

On 7/14/22 20:15, John Garry wrote:
> Streaming DMA mappings may be considerably slower when mappings go through
> an IOMMU and the total mapping length is somewhat long. This is because the
> IOMMU IOVA code allocates and free an IOVA for each mapping, which may
> affect performance.
>
> New member Scsi_Host.opt_sectors is added, which is the optimal host
> max_sectors, and use this value to cap the request queue max_sectors when
> set.
>
> It could be considered to have request queues io_opt value initially
> set at Scsi_Host.opt_sectors in __scsi_init_queue(), but that is not
> really the purpose of io_opt.
>
> Finally, even though Scsi_Host.opt_sectors value should never be greater
> than the request queue max_hw_sectors value, continue to limit to this
> value for safety.
>
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
> drivers/scsi/sd.c | 2 ++
> include/scsi/scsi_host.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index a1a2ac09066f..3eaee1f7aaca 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3296,6 +3296,8 @@ static int sd_revalidate_disk(struct gendisk *disk)
> (sector_t)BLK_DEF_MAX_SECTORS);
> }
>
> + rw_max = min_not_zero(rw_max, sdp->host->opt_sectors);
> +

Adding a comment explaining what the cap is would be nice.

> /* Do not exceed controller limit */
> rw_max = min(rw_max, queue_max_hw_sectors(q));
>
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 667d889b92b5..d32a84b2bb40 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -607,6 +607,7 @@ struct Scsi_Host {
> short unsigned int sg_tablesize;
> short unsigned int sg_prot_tablesize;
> unsigned int max_sectors;
> + unsigned int opt_sectors;
> unsigned int max_segment_size;
> unsigned long dma_boundary;
> unsigned long virt_boundary_mask;

Otherwise, looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

--
Damien Le Moal
Western Digital Research