| Message ID | 1656590892-42307-5-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | DMA mapping changes for SCSI core | expand |
On 6/30/22 21:08, 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. > > For performance reasons set the request queue max_sectors from > dma_opt_mapping_size(), which knows this mapping limit. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > drivers/scsi/scsi_transport_sas.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c > index 12bff64dade6..1b45248748e0 100644 > --- a/drivers/scsi/scsi_transport_sas.c > +++ b/drivers/scsi/scsi_transport_sas.c > @@ -225,6 +225,7 @@ static int sas_host_setup(struct transport_container *tc, struct device *dev, > { > struct Scsi_Host *shost = dev_to_shost(dev); > struct sas_host_attrs *sas_host = to_sas_host_attrs(shost); > + struct device *dma_dev = shost->dma_dev; > > INIT_LIST_HEAD(&sas_host->rphy_list); > mutex_init(&sas_host->lock); > @@ -236,6 +237,11 @@ static int sas_host_setup(struct transport_container *tc, struct device *dev, > dev_printk(KERN_ERR, dev, "fail to a bsg device %d\n", > shost->host_no); > > + if (dma_dev) { > + shost->max_sectors = min_t(unsigned int, shost->max_sectors, > + dma_opt_mapping_size(dma_dev) >> SECTOR_SHIFT); > + } Hmm... shost->max_sectors becomes the max_hw_sectors limit for the block dev. So using dma_max_mapping_size(dma_dev) for that limit makes sense. Shouldn't dma_opt_mapping_size(dma_dev) be used to limit only the default "soft" limit (queue max_sectors limit) instead of the hard limit ? > + > return 0; > } >
On 01/07/2022 00:49, Damien Le Moal wrote: >> >> + if (dma_dev) { >> + shost->max_sectors = min_t(unsigned int, shost->max_sectors, >> + dma_opt_mapping_size(dma_dev) >> SECTOR_SHIFT); >> + } Hi Damien, > Hmm... shost->max_sectors becomes the max_hw_sectors limit for the block > dev. So using dma_max_mapping_size(dma_dev) for that limit makes sense. > Shouldn't dma_opt_mapping_size(dma_dev) be used to limit only the default > "soft" limit (queue max_sectors limit) instead of the hard limit ? > Sure, it would sensible to use dma_opt_mapping_size() to limit the default queue max sectors limit, while dma_max_mapping_size() limits the host max sectors. But I didn't see in practice how limiting the shost max sectors to dma_opt_mapping_size() makes a difference: - block queue max_hw_sectors_kb file is read-only, so we cannot change the queue max sectors from there - And no SAS driver actually tries to modify upwards from the default. I do note that USB storage driver as an example of a scsi driver which does (modify from shost max sectors): see scsiglue.c::slave_configure() Finally there is no common method to limit the default request queue max sectors for those SAS drivers - I would need to add this limit in each of their slave_configure callbacks, and I didn't think that its worth it. Thanks, John
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c index 12bff64dade6..1b45248748e0 100644 --- a/drivers/scsi/scsi_transport_sas.c +++ b/drivers/scsi/scsi_transport_sas.c @@ -225,6 +225,7 @@ static int sas_host_setup(struct transport_container *tc, struct device *dev, { struct Scsi_Host *shost = dev_to_shost(dev); struct sas_host_attrs *sas_host = to_sas_host_attrs(shost); + struct device *dma_dev = shost->dma_dev; INIT_LIST_HEAD(&sas_host->rphy_list); mutex_init(&sas_host->lock); @@ -236,6 +237,11 @@ static int sas_host_setup(struct transport_container *tc, struct device *dev, dev_printk(KERN_ERR, dev, "fail to a bsg device %d\n", shost->host_no); + if (dma_dev) { + shost->max_sectors = min_t(unsigned int, shost->max_sectors, + dma_opt_mapping_size(dma_dev) >> SECTOR_SHIFT); + } + return 0; }
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. For performance reasons set the request queue max_sectors from dma_opt_mapping_size(), which knows this mapping limit. Signed-off-by: John Garry <john.garry@huawei.com> --- drivers/scsi/scsi_transport_sas.c | 6 ++++++ 1 file changed, 6 insertions(+)