Re: [PATCH v5 4/5] scsi: scsi_transport_sas: Cap shost max_sectors according to DMA optimal limit
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
> ---
> 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(_host->rphy_list);
> mutex_init(_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;
> }
>
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/5] scsi: core: Cap shost max_sectors according to DMA limits only once
On 6/30/22 21:08, John Garry wrote:
> The shost->max_sectors is repeatedly capped according to the host DMA
> mapping limit for each sdev in __scsi_init_queue(). This is unnecessary, so
> set only once when adding the host.
>
> Signed-off-by: John Garry
> ---
> drivers/scsi/hosts.c| 5 +
> drivers/scsi/scsi_lib.c | 4
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index 8352f90d997d..d04bd2c7c9f1 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -236,6 +236,11 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost,
> struct device *dev,
>
> shost->dma_dev = dma_dev;
>
> + if (dma_dev->dma_mask) {
> + shost->max_sectors = min_t(unsigned int, shost->max_sectors,
> + dma_max_mapping_size(dma_dev) >> SECTOR_SHIFT);
> + }
Nit: you could remove the curly brackets... But it being a multi-line
statement, having them is OK too I think.
> +
> error = scsi_mq_setup_tags(shost);
> if (error)
> goto fail;
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 6ffc9e4258a8..6ce8acea322a 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1884,10 +1884,6 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct
> request_queue *q)
> blk_queue_max_integrity_segments(q, shost->sg_prot_tablesize);
> }
>
> - if (dev->dma_mask) {
> - shost->max_sectors = min_t(unsigned int, shost->max_sectors,
> - dma_max_mapping_size(dev) >> SECTOR_SHIFT);
> - }
> blk_queue_max_hw_sectors(q, shost->max_sectors);
> blk_queue_segment_boundary(q, shost->dma_boundary);
> dma_set_seg_boundary(dev, shost->dma_boundary);
Looks good.
Reviewed-by: Damien Le Moal
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 5/5] libata-scsi: Cap ata_device->max_sectors according to shost->max_sectors
On 6/29/22 16:43, John Garry wrote: > On 29/06/2022 06:58, Damien Le Moal wrote: >> On 6/29/22 14:40, Christoph Hellwig wrote: >>> On Tue, Jun 28, 2022 at 12:33:58PM +0100, John Garry wrote: Well Christoph originally offered to take this series via the dma-mapping tree. @Christoph, is that still ok with you? If so, would you rather I send this libata patch separately? >>> >>> The offer still stands, and I don't really care where the libata >>> patch is routed. Just tell me what you prefer. > > Cheers. > >> >> If it is 100% independent from the other patches, I can take it. >> Otherwise, feel free to take it ! >> > > I'll just keep the all together - it's easier in case I need to change > anything. Works for me. > > Thanks! -- Damien Le Moal Western Digital Research ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 5/5] libata-scsi: Cap ata_device->max_sectors according to shost->max_sectors
On 6/29/22 14:40, Christoph Hellwig wrote: > On Tue, Jun 28, 2022 at 12:33:58PM +0100, John Garry wrote: >> Well Christoph originally offered to take this series via the dma-mapping >> tree. >> >> @Christoph, is that still ok with you? If so, would you rather I send this >> libata patch separately? > > The offer still stands, and I don't really care where the libata > patch is routed. Just tell me what you prefer. If it is 100% independent from the other patches, I can take it. Otherwise, feel free to take it ! -- Damien Le Moal Western Digital Research ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 5/5] libata-scsi: Cap ata_device->max_sectors according to shost->max_sectors
On 6/28/22 16:54, John Garry wrote: > On 28/06/2022 00:24, Damien Le Moal wrote: >> On 6/28/22 00:25, John Garry wrote: >>> ATA devices (struct ata_device) have a max_sectors field which is >>> configured internally in libata. This is then used to (re)configure the >>> associated sdev request queue max_sectors value from how it is earlier set >>> in __scsi_init_queue(). In __scsi_init_queue() the max_sectors value is set >>> according to shost limits, which includes host DMA mapping limits. >>> >>> Cap the ata_device max_sectors according to shost->max_sectors to respect >>> this shost limit. >>> >>> Signed-off-by: John Garry >>> Acked-by: Damien Le Moal >> Nit: please change the patch title to "ata: libata-scsi: Cap ..." >> > > ok, but it's going to be an even longer title :) > > BTW, this patch has no real dependency on the rest of the series, so > could be taken separately if you prefer. Sure, you can send it separately. Adding it through the scsi tree is fine too. > > Thanks, > John -- Damien Le Moal Western Digital Research ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 5/5] libata-scsi: Cap ata_device->max_sectors according to shost->max_sectors
On 6/28/22 00:25, John Garry wrote:
> ATA devices (struct ata_device) have a max_sectors field which is
> configured internally in libata. This is then used to (re)configure the
> associated sdev request queue max_sectors value from how it is earlier set
> in __scsi_init_queue(). In __scsi_init_queue() the max_sectors value is set
> according to shost limits, which includes host DMA mapping limits.
>
> Cap the ata_device max_sectors according to shost->max_sectors to respect
> this shost limit.
>
> Signed-off-by: John Garry
> Acked-by: Damien Le Moal
Nit: please change the patch title to "ata: libata-scsi: Cap ..."
> ---
> drivers/ata/libata-scsi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 86dbb1cdfabd..24a43d540d9f 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1060,6 +1060,7 @@ int ata_scsi_dev_config(struct scsi_device *sdev,
> struct ata_device *dev)
> dev->flags |= ATA_DFLAG_NO_UNLOAD;
>
> /* configure max sectors */
> + dev->max_sectors = min(dev->max_sectors, sdev->host->max_sectors);
> blk_queue_max_hw_sectors(q, dev->max_sectors);
>
> if (dev->class == ATA_DEV_ATAPI) {
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/4] dma-iommu: Add iommu_dma_opt_mapping_size()
On 2022/05/26 19:28, John Garry wrote:
> Add the IOMMU callback for DMA mapping API dma_opt_mapping_size(), which
> allows the drivers to know the optimal mapping limit and thus limit the
> requested IOVA lengths.
>
> This value is based on the IOVA rcache range limit, as IOVAs allocated
> above this limit must always be newly allocated, which may be quite slow.
>
> Signed-off-by: John Garry
> ---
> drivers/iommu/dma-iommu.c | 6 ++
> drivers/iommu/iova.c | 5 +
> include/linux/iova.h | 2 ++
> 3 files changed, 13 insertions(+)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 09f6e1c0f9c0..f619e41b9172 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1442,6 +1442,11 @@ static unsigned long
> iommu_dma_get_merge_boundary(struct device *dev)
> return (1UL << __ffs(domain->pgsize_bitmap)) - 1;
> }
>
> +static size_t iommu_dma_opt_mapping_size(void)
> +{
> + return iova_rcache_range();
> +}
> +
> static const struct dma_map_ops iommu_dma_ops = {
> .alloc = iommu_dma_alloc,
> .free = iommu_dma_free,
> @@ -1462,6 +1467,7 @@ static const struct dma_map_ops iommu_dma_ops = {
> .map_resource = iommu_dma_map_resource,
> .unmap_resource = iommu_dma_unmap_resource,
> .get_merge_boundary = iommu_dma_get_merge_boundary,
> + .opt_mapping_size = iommu_dma_opt_mapping_size,
> };
>
> /*
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index db77aa675145..9f00b58d546e 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -26,6 +26,11 @@ static unsigned long iova_rcache_get(struct iova_domain
> *iovad,
> static void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain
> *iovad);
> static void free_iova_rcaches(struct iova_domain *iovad);
>
> +unsigned long iova_rcache_range(void)
> +{
> + return PAGE_SIZE << (IOVA_RANGE_CACHE_MAX_SIZE - 1);
> +}
> +
> static int iova_cpuhp_dead(unsigned int cpu, struct hlist_node *node)
> {
> struct iova_domain *iovad;
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index 320a70e40233..c6ba6d95d79c 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h
> @@ -79,6 +79,8 @@ static inline unsigned long iova_pfn(struct iova_domain
> *iovad, dma_addr_t iova)
> int iova_cache_get(void);
> void iova_cache_put(void);
>
> +unsigned long iova_rcache_range(void);
> +
> void free_iova(struct iova_domain *iovad, unsigned long pfn);
> void __free_iova(struct iova_domain *iovad, struct iova *iova);
> struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,
Reviewed-by: Damien Le Moal
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/4] dma-iommu: Add iommu_dma_opt_mapping_size()
On 2022/05/20 17:23, John Garry wrote:
> Add the IOMMU callback for DMA mapping API dma_opt_mapping_size(), which
> allows the drivers to know the optimal mapping limit and thus limit the
> requested IOVA lengths.
>
> This value is based on the IOVA rcache range limit, as IOVAs allocated
> above this limit must always be newly allocated, which may be quite slow.
>
> Signed-off-by: John Garry
> ---
> drivers/iommu/dma-iommu.c | 6 ++
> drivers/iommu/iova.c | 5 +
> include/linux/iova.h | 2 ++
> 3 files changed, 13 insertions(+)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 09f6e1c0f9c0..f619e41b9172 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1442,6 +1442,11 @@ static unsigned long
> iommu_dma_get_merge_boundary(struct device *dev)
> return (1UL << __ffs(domain->pgsize_bitmap)) - 1;
> }
>
> +static size_t iommu_dma_opt_mapping_size(void)
> +{
> + return iova_rcache_range();
> +}
> +
> static const struct dma_map_ops iommu_dma_ops = {
> .alloc = iommu_dma_alloc,
> .free = iommu_dma_free,
> @@ -1462,6 +1467,7 @@ static const struct dma_map_ops iommu_dma_ops = {
> .map_resource = iommu_dma_map_resource,
> .unmap_resource = iommu_dma_unmap_resource,
> .get_merge_boundary = iommu_dma_get_merge_boundary,
> + .opt_mapping_size = iommu_dma_opt_mapping_size,
> };
>
> /*
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index db77aa675145..9f00b58d546e 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -26,6 +26,11 @@ static unsigned long iova_rcache_get(struct iova_domain
> *iovad,
> static void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain
> *iovad);
> static void free_iova_rcaches(struct iova_domain *iovad);
>
> +unsigned long iova_rcache_range(void)
> +{
> + return PAGE_SIZE << (IOVA_RANGE_CACHE_MAX_SIZE - 1);
> +}
> +
> static int iova_cpuhp_dead(unsigned int cpu, struct hlist_node *node)
> {
> struct iova_domain *iovad;
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index 320a70e40233..c6ba6d95d79c 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h
> @@ -79,6 +79,8 @@ static inline unsigned long iova_pfn(struct iova_domain
> *iovad, dma_addr_t iova)
> int iova_cache_get(void);
> void iova_cache_put(void);
>
> +unsigned long iova_rcache_range(void);
> +
> void free_iova(struct iova_domain *iovad, unsigned long pfn);
> void __free_iova(struct iova_domain *iovad, struct iova *iova);
> struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,
Reviewed-by: Damien Le Moal
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] scsi: core: Cap shost max_sectors according to DMA optimum mapping limits
On 2022/05/23 15:53, John Garry wrote:
> On 21/05/2022 00:30, Damien Le Moal wrote:
>>> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
>>> index f69b77cbf538..a3ae6345473b 100644
>>> --- a/drivers/scsi/hosts.c
>>> +++ b/drivers/scsi/hosts.c
>>> @@ -225,6 +225,11 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost,
>>> struct device *dev,
>>> shost->cmd_per_lun = min_t(int, shost->cmd_per_lun,
>>>shost->can_queue);
>>>
>
> Hi Damien,
>
>>> + if (dma_dev->dma_mask) {
>>> + shost->max_sectors = min_t(unsigned int, shost->max_sectors,
>>> + dma_opt_mapping_size(dma_dev) >> SECTOR_SHIFT);
>>> + }
>> Nit: you could drop the curly brackets here.
>
> Some people prefer this style - multi-line statements have curly
> brackets, while single-line statements conform to the official coding
> style (and don't use brackets).
OK.
>
> I'll just stick with what we have unless there is a consensus to change.
>
> Thanks,
> John
>
>>
>>> +
>>> error = scsi_init_sense_cache(shost);
>>> if (error)
>>> goto fail;
>
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/4] dma-iommu: Add iommu_dma_opt_mapping_size()
On 2022/05/23 16:01, John Garry wrote:
> On 21/05/2022 00:33, Damien Le Moal wrote:
>
> Hi Damien,
>
>>> +unsigned long iova_rcache_range(void)
>> Why not a size_t return type ?
>
> The IOVA code generally uses unsigned long for size/range while
> dam-iommu uses size_t as appropiate, so I'm just sticking to that.
OK.
>
>>
>>> +{
>>> + return PAGE_SIZE << (IOVA_RANGE_CACHE_MAX_SIZE - 1);
>>> +}
>>> +
>
> Thanks,
> John
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/4] DMA mapping changes for SCSI core
On 2022/05/22 22:13, Christoph Hellwig wrote: > The whole series looks fine to me. I'll happily queue it up in the > dma-mapping tree if the SCSI and ATA maintainers are ok with that. > Fine with me. I sent an acked-by for the libata bit. -- Damien Le Moal Western Digital Research ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] libata-scsi: Cap ata_device->max_sectors according to shost->max_sectors
On 5/20/22 17:23, John Garry wrote:
> ATA devices (struct ata_device) have a max_sectors field which is
> configured internally in libata. This is then used to (re)configure the
> associated sdev request queue max_sectors value from how it is earlier set
> in __scsi_init_queue(). In __scsi_init_queue() the max_sectors value is set
> according to shost limits, which includes host DMA mapping limits.
>
> Cap the ata_device max_sectors according to shost->max_sectors to respect
> this shost limit.
>
> Signed-off-by: John Garry
> ---
> drivers/ata/libata-scsi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 06c9d90238d9..25fe89791641 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1036,6 +1036,7 @@ int ata_scsi_dev_config(struct scsi_device *sdev,
> struct ata_device *dev)
> dev->flags |= ATA_DFLAG_NO_UNLOAD;
>
> /* configure max sectors */
> + dev->max_sectors = min(dev->max_sectors, sdev->host->max_sectors);
> blk_queue_max_hw_sectors(q, dev->max_sectors);
>
> if (dev->class == ATA_DEV_ATAPI) {
Acked-by: Damien Le Moal
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] dma-mapping: Add dma_opt_mapping_size()
On 5/20/22 17:23, John Garry wrote:
> Streaming DMA mapping involving an IOMMU may be much slower for larger
> total mapping size. This is because every IOMMU DMA mapping requires an
> IOVA to be allocated and freed. IOVA sizes above a certain limit are not
> cached, which can have a big impact on DMA mapping performance.
>
> Provide an API for device drivers to know this "optimal" limit, such that
> they may try to produce mapping which don't exceed it.
>
> Signed-off-by: John Garry
> ---
> Documentation/core-api/dma-api.rst | 9 +
> include/linux/dma-map-ops.h| 1 +
> include/linux/dma-mapping.h| 5 +
> kernel/dma/mapping.c | 12
> 4 files changed, 27 insertions(+)
>
> diff --git a/Documentation/core-api/dma-api.rst
> b/Documentation/core-api/dma-api.rst
> index 6d6d0edd2d27..b3cd9763d28b 100644
> --- a/Documentation/core-api/dma-api.rst
> +++ b/Documentation/core-api/dma-api.rst
> @@ -204,6 +204,15 @@ Returns the maximum size of a mapping for the device.
> The size parameter
> of the mapping functions like dma_map_single(), dma_map_page() and
> others should not be larger than the returned value.
>
> +::
> +
> + size_t
> + dma_opt_mapping_size(struct device *dev);
> +
> +Returns the maximum optimal size of a mapping for the device. Mapping large
> +buffers may take longer so device drivers are advised to limit total DMA
> +streaming mappings length to the returned value.
> +
> ::
>
> bool
> diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
> index 0d5b06b3a4a6..98ceba6fa848 100644
> --- a/include/linux/dma-map-ops.h
> +++ b/include/linux/dma-map-ops.h
> @@ -69,6 +69,7 @@ struct dma_map_ops {
> int (*dma_supported)(struct device *dev, u64 mask);
> u64 (*get_required_mask)(struct device *dev);
> size_t (*max_mapping_size)(struct device *dev);
> + size_t (*opt_mapping_size)(void);
> unsigned long (*get_merge_boundary)(struct device *dev);
> };
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index dca2b1355bb1..fe3849434b2a 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -144,6 +144,7 @@ int dma_set_mask(struct device *dev, u64 mask);
> int dma_set_coherent_mask(struct device *dev, u64 mask);
> u64 dma_get_required_mask(struct device *dev);
> size_t dma_max_mapping_size(struct device *dev);
> +size_t dma_opt_mapping_size(struct device *dev);
> bool dma_need_sync(struct device *dev, dma_addr_t dma_addr);
> unsigned long dma_get_merge_boundary(struct device *dev);
> struct sg_table *dma_alloc_noncontiguous(struct device *dev, size_t size,
> @@ -266,6 +267,10 @@ static inline size_t dma_max_mapping_size(struct device
> *dev)
> {
> return 0;
> }
> +static inline size_t dma_opt_mapping_size(struct device *dev)
> +{
> + return 0;
> +}
> static inline bool dma_need_sync(struct device *dev, dma_addr_t dma_addr)
> {
> return false;
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index db7244291b74..1bfe11b1edb6 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -773,6 +773,18 @@ size_t dma_max_mapping_size(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(dma_max_mapping_size);
>
> +size_t dma_opt_mapping_size(struct device *dev)
> +{
> + const struct dma_map_ops *ops = get_dma_ops(dev);
> + size_t size = SIZE_MAX;
> +
> + if (ops && ops->opt_mapping_size)
> + size = ops->opt_mapping_size();
> +
> + return min(dma_max_mapping_size(dev), size);
> +}
> +EXPORT_SYMBOL_GPL(dma_opt_mapping_size);
> +
> bool dma_need_sync(struct device *dev, dma_addr_t dma_addr)
> {
> const struct dma_map_ops *ops = get_dma_ops(dev);
Looks OK to me.
Reviewed-by: Damien Le Moal
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/4] dma-iommu: Add iommu_dma_opt_mapping_size()
On 5/20/22 17:23, John Garry wrote:
> Add the IOMMU callback for DMA mapping API dma_opt_mapping_size(), which
> allows the drivers to know the optimal mapping limit and thus limit the
> requested IOVA lengths.
>
> This value is based on the IOVA rcache range limit, as IOVAs allocated
> above this limit must always be newly allocated, which may be quite slow.
>
> Signed-off-by: John Garry
> ---
> drivers/iommu/dma-iommu.c | 6 ++
> drivers/iommu/iova.c | 5 +
> include/linux/iova.h | 2 ++
> 3 files changed, 13 insertions(+)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 09f6e1c0f9c0..f619e41b9172 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1442,6 +1442,11 @@ static unsigned long
> iommu_dma_get_merge_boundary(struct device *dev)
> return (1UL << __ffs(domain->pgsize_bitmap)) - 1;
> }
>
> +static size_t iommu_dma_opt_mapping_size(void)
> +{
> + return iova_rcache_range();
> +}
> +
> static const struct dma_map_ops iommu_dma_ops = {
> .alloc = iommu_dma_alloc,
> .free = iommu_dma_free,
> @@ -1462,6 +1467,7 @@ static const struct dma_map_ops iommu_dma_ops = {
> .map_resource = iommu_dma_map_resource,
> .unmap_resource = iommu_dma_unmap_resource,
> .get_merge_boundary = iommu_dma_get_merge_boundary,
> + .opt_mapping_size = iommu_dma_opt_mapping_size,
> };
>
> /*
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index db77aa675145..9f00b58d546e 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -26,6 +26,11 @@ static unsigned long iova_rcache_get(struct iova_domain
> *iovad,
> static void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain
> *iovad);
> static void free_iova_rcaches(struct iova_domain *iovad);
>
> +unsigned long iova_rcache_range(void)
Why not a size_t return type ?
> +{
> + return PAGE_SIZE << (IOVA_RANGE_CACHE_MAX_SIZE - 1);
> +}
> +
> static int iova_cpuhp_dead(unsigned int cpu, struct hlist_node *node)
> {
> struct iova_domain *iovad;
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index 320a70e40233..c6ba6d95d79c 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h
> @@ -79,6 +79,8 @@ static inline unsigned long iova_pfn(struct iova_domain
> *iovad, dma_addr_t iova)
> int iova_cache_get(void);
> void iova_cache_put(void);
>
> +unsigned long iova_rcache_range(void);
> +
> void free_iova(struct iova_domain *iovad, unsigned long pfn);
> void __free_iova(struct iova_domain *iovad, struct iova *iova);
> struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] scsi: core: Cap shost max_sectors according to DMA optimum mapping limits
On 5/20/22 17:23, 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.
>
> In addition, the shost->max_sectors is repeatedly set for each sdev in
> __scsi_init_queue(). This is unnecessary, so set once when adding the
> host.
>
> Signed-off-by: John Garry
> ---
> drivers/scsi/hosts.c| 5 +
> drivers/scsi/scsi_lib.c | 4
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index f69b77cbf538..a3ae6345473b 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -225,6 +225,11 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost,
> struct device *dev,
> shost->cmd_per_lun = min_t(int, shost->cmd_per_lun,
> shost->can_queue);
>
> + if (dma_dev->dma_mask) {
> + shost->max_sectors = min_t(unsigned int, shost->max_sectors,
> + dma_opt_mapping_size(dma_dev) >> SECTOR_SHIFT);
> + }
Nit: you could drop the curly brackets here.
> +
> error = scsi_init_sense_cache(shost);
> if (error)
> goto fail;
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 8d18cc7e510e..2d43bb8799bd 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1884,10 +1884,6 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct
> request_queue *q)
> blk_queue_max_integrity_segments(q, shost->sg_prot_tablesize);
> }
>
> - if (dev->dma_mask) {
> - shost->max_sectors = min_t(unsigned int, shost->max_sectors,
> - dma_max_mapping_size(dev) >> SECTOR_SHIFT);
> - }
> blk_queue_max_hw_sectors(q, shost->max_sectors);
> blk_queue_segment_boundary(q, shost->dma_boundary);
> dma_set_seg_boundary(dev, shost->dma_boundary);
Reviewed-by: Damien Le Moal
--
Damien Le Moal
Western Digital Research
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
