[PATCH V2 0/1] Optimise IOVA allocations for PCI devices

[PATCH V2 0/1] Optimise IOVA allocations for PCI devices

[Tomasz Nowicki]

Here is my test setup where I have stareted performance measurements.

 ------------  PCIe  -------------   TX   -------------  PCIe  -----
| ThunderX2  |------| Intel XL710 | ---> | Intel XL710 |------| X86 |
| (128 cpus) |      |   40GbE     |      |    40GbE    |       -----
 ------------        -------------        -------------

As the reference lets take v4.13 host, SMMUv3 off and 1-thread iperf
taskset to one CPU. The performance results I got:

SMMU off -> 100%
SMMU on -> 0,02%

I followed down the DMA mapping path and found out IOVA 32-bit space
full so that kernel was flushing rcaches for all CPUs in (1).
For 128 CPUs, this kills the performance. Furthermore, for my case, rcaches
contained PFNs > 32-bit mostly so the second round of IOVA allocation failed
as well. As the consequence IOVA had to be allocated outside of 32-bit (2)
from scratch since all rcaches have been flushed in (1).

    if (dma_limit > DMA_BIT_MASK(32) && dev_is_pci(dev))
(1)-->  iova = alloc_iova_fast(iovad, iova_len, DMA_BIT_MASK(32) >> shift);

    if (!iova)
(2)-->  iova = alloc_iova_fast(iovad, iova_len, dma_limit >> shift);

My fix simply introduces parameter for alloc_iova_fast() to decide whether
rcache flush has to be done or not. All users follow mentioned scenario
so they should let flush as the last chance to avoid time costly iteration
over all CPUs.

This bring my iperf performance back to 100% with SMMU on.

My bad feelings regarding this solution is that machines with relatively
small numbers of CPUs may get DAC addresses more frequently for PCI
devices. Please let me know your thoughts.

Changelog:

v1 --> v2
- add missing documentation
- fix typo

Tomasz Nowicki (1):
  iommu/iova: Make rcache flush optional on IOVA allocation failure

 drivers/iommu/amd_iommu.c   |  5 +++--
 drivers/iommu/dma-iommu.c   |  6 ++++--
 drivers/iommu/intel-iommu.c |  5 +++--
 drivers/iommu/iova.c        | 11 ++++++-----
 include/linux/iova.h        |  5 +++--
 5 files changed, 19 insertions(+), 13 deletions(-)

-- 
2.7.4

[PATCH V2 1/1] iommu/iova: Make rcache flush optional on IOVA allocation failure

[Tomasz Nowicki]

Since IOVA allocation failure is not unusual case we need to flush
CPUs' rcache in hope we will succeed in next round.

However, it is useful to decide whether we need rcache flush step because
of two reasons:
- Not scalability. On large system with ~100 CPUs iterating and flushing
  rcache for each CPU becomes serious bottleneck so we may want to defer it.
- free_cpu_cached_iovas() does not care about max PFN we are interested in.
  Thus we may flush our rcaches and still get no new IOVA like in the
  commonly used scenario:

    if (dma_limit > DMA_BIT_MASK(32) && dev_is_pci(dev))
        iova = alloc_iova_fast(iovad, iova_len, DMA_BIT_MASK(32) >> shift);

    if (!iova)
        iova = alloc_iova_fast(iovad, iova_len, dma_limit >> shift);

   1. First alloc_iova_fast() call is limited to DMA_BIT_MASK(32) to get
      PCI devices a SAC address
   2. alloc_iova() fails due to full 32-bit space
   3. rcaches contain PFNs out of 32-bit space so free_cpu_cached_iovas()
      throws entries away for nothing and alloc_iova() fails again
   4. Next alloc_iova_fast() call cannot take advantage of rcache since we
      have just defeated caches. In this case we pick the slowest option
      to proceed.

This patch reworks flushed_rcache local flag to be additional function
argument instead and control rcache flush step. Also, it updates all users
to do the flush as the last chance.

Signed-off-by: Tomasz Nowicki <Tomasz.Nowicki@caviumnetworks.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Nate Watterson <nwatters@codeaurora.org>
---
 drivers/iommu/amd_iommu.c   |  5 +++--
 drivers/iommu/dma-iommu.c   |  6 ++++--
 drivers/iommu/intel-iommu.c |  5 +++--
 drivers/iommu/iova.c        | 11 ++++++-----
 include/linux/iova.h        |  5 +++--
 5 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8d2ec60..ce68986 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1604,10 +1604,11 @@ static unsigned long dma_ops_alloc_iova(struct device *dev,
 
 	if (dma_mask > DMA_BIT_MASK(32))
 		pfn = alloc_iova_fast(&dma_dom->iovad, pages,
-				      IOVA_PFN(DMA_BIT_MASK(32)));
+				      IOVA_PFN(DMA_BIT_MASK(32)), false);
 
 	if (!pfn)
-		pfn = alloc_iova_fast(&dma_dom->iovad, pages, IOVA_PFN(dma_mask));
+		pfn = alloc_iova_fast(&dma_dom->iovad, pages,
+				      IOVA_PFN(dma_mask), true);
 
 	return (pfn << PAGE_SHIFT);
 }
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 191be9c..25914d3 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -370,10 +370,12 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain,
 
 	/* Try to get PCI devices a SAC address */
 	if (dma_limit > DMA_BIT_MASK(32) && dev_is_pci(dev))
-		iova = alloc_iova_fast(iovad, iova_len, DMA_BIT_MASK(32) >> shift);
+		iova = alloc_iova_fast(iovad, iova_len,
+				       DMA_BIT_MASK(32) >> shift, false);
 
 	if (!iova)
-		iova = alloc_iova_fast(iovad, iova_len, dma_limit >> shift);
+		iova = alloc_iova_fast(iovad, iova_len, dma_limit >> shift,
+				       true);
 
 	return (dma_addr_t)iova << shift;
 }
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 05c0c3a..75c8320 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3460,11 +3460,12 @@ static unsigned long intel_alloc_iova(struct device *dev,
 		 * from higher range
 		 */
 		iova_pfn = alloc_iova_fast(&domain->iovad, nrpages,
-					   IOVA_PFN(DMA_BIT_MASK(32)));
+					   IOVA_PFN(DMA_BIT_MASK(32)), false);
 		if (iova_pfn)
 			return iova_pfn;
 	}
-	iova_pfn = alloc_iova_fast(&domain->iovad, nrpages, IOVA_PFN(dma_mask));
+	iova_pfn = alloc_iova_fast(&domain->iovad, nrpages,
+				   IOVA_PFN(dma_mask), true);
 	if (unlikely(!iova_pfn)) {
 		pr_err("Allocating %ld-page iova for %s failed",
 		       nrpages, dev_name(dev));
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index f88acad..e8c140c 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -353,14 +353,15 @@ EXPORT_SYMBOL_GPL(free_iova);
  * @iovad: - iova domain in question
  * @size: - size of page frames to allocate
  * @limit_pfn: - max limit address
+ * @flush_rcache: - set to flush rcache on regular allocation failure
  * This function tries to satisfy an iova allocation from the rcache,
- * and falls back to regular allocation on failure.
+ * and falls back to regular allocation on failure. If regular allocation
+ * fails too and the flush_rcache flag is set then the rcache will be flushed.
 */
 unsigned long
 alloc_iova_fast(struct iova_domain *iovad, unsigned long size,
-		unsigned long limit_pfn)
+		unsigned long limit_pfn, bool flush_rcache)
 {
-	bool flushed_rcache = false;
 	unsigned long iova_pfn;
 	struct iova *new_iova;
 
@@ -373,11 +374,11 @@ alloc_iova_fast(struct iova_domain *iovad, unsigned long size,
 	if (!new_iova) {
 		unsigned int cpu;
 
-		if (flushed_rcache)
+		if (!flush_rcache)
 			return 0;
 
 		/* Try replenishing IOVAs by flushing rcache. */
-		flushed_rcache = true;
+		flush_rcache = false;
 		for_each_online_cpu(cpu)
 			free_cpu_cached_iovas(cpu, iovad);
 		goto retry;
diff --git a/include/linux/iova.h b/include/linux/iova.h
index 58c2a36..8fdcb66 100644
--- a/include/linux/iova.h
+++ b/include/linux/iova.h
@@ -97,7 +97,7 @@ struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,
 void free_iova_fast(struct iova_domain *iovad, unsigned long pfn,
 		    unsigned long size);
 unsigned long alloc_iova_fast(struct iova_domain *iovad, unsigned long size,
-			      unsigned long limit_pfn);
+			      unsigned long limit_pfn, bool flush_rcache);
 struct iova *reserve_iova(struct iova_domain *iovad, unsigned long pfn_lo,
 	unsigned long pfn_hi);
 void copy_reserved_iova(struct iova_domain *from, struct iova_domain *to);
@@ -151,7 +151,8 @@ static inline void free_iova_fast(struct iova_domain *iovad,
 
 static inline unsigned long alloc_iova_fast(struct iova_domain *iovad,
 					    unsigned long size,
-					    unsigned long limit_pfn)
+					    unsigned long limit_pfn,
+					    bool flush_rcache)
 {
 	return 0;
 }
-- 
2.7.4

[PATCH V2 0/1] Optimise IOVA allocations for PCI devices

[Tomasz Nowicki]

Hi Joerg,

Can you please have a look and see if you are fine with this patch?

Thanks in advance,
Tomasz

On 20.09.2017 10:52, Tomasz Nowicki wrote:
> Here is my test setup where I have stareted performance measurements.
> 
>   ------------  PCIe  -------------   TX   -------------  PCIe  -----
> | ThunderX2  |------| Intel XL710 | ---> | Intel XL710 |------| X86 |
> | (128 cpus) |      |   40GbE     |      |    40GbE    |       -----
>   ------------        -------------        -------------
> 
> As the reference lets take v4.13 host, SMMUv3 off and 1-thread iperf
> taskset to one CPU. The performance results I got:
> 
> SMMU off -> 100%
> SMMU on -> 0,02%
> 
> I followed down the DMA mapping path and found out IOVA 32-bit space
> full so that kernel was flushing rcaches for all CPUs in (1).
> For 128 CPUs, this kills the performance. Furthermore, for my case, rcaches
> contained PFNs > 32-bit mostly so the second round of IOVA allocation failed
> as well. As the consequence IOVA had to be allocated outside of 32-bit (2)
> from scratch since all rcaches have been flushed in (1).
> 
>      if (dma_limit > DMA_BIT_MASK(32) && dev_is_pci(dev))
> (1)-->  iova = alloc_iova_fast(iovad, iova_len, DMA_BIT_MASK(32) >> shift);
> 
>      if (!iova)
> (2)-->  iova = alloc_iova_fast(iovad, iova_len, dma_limit >> shift);
> 
> My fix simply introduces parameter for alloc_iova_fast() to decide whether
> rcache flush has to be done or not. All users follow mentioned scenario
> so they should let flush as the last chance to avoid time costly iteration
> over all CPUs.
> 
> This bring my iperf performance back to 100% with SMMU on.
> 
> My bad feelings regarding this solution is that machines with relatively
> small numbers of CPUs may get DAC addresses more frequently for PCI
> devices. Please let me know your thoughts.
> 
> Changelog:
> 
> v1 --> v2
> - add missing documentation
> - fix typo
> 
> Tomasz Nowicki (1):
>    iommu/iova: Make rcache flush optional on IOVA allocation failure
> 
>   drivers/iommu/amd_iommu.c   |  5 +++--
>   drivers/iommu/dma-iommu.c   |  6 ++++--
>   drivers/iommu/intel-iommu.c |  5 +++--
>   drivers/iommu/iova.c        | 11 ++++++-----
>   include/linux/iova.h        |  5 +++--
>   5 files changed, 19 insertions(+), 13 deletions(-)
> 

[PATCH V2 0/1] Optimise IOVA allocations for PCI devices

[Tomasz Nowicki]

Hi Joerg,

Can you please have a look and see if you are fine with this patch?

Thanks in advance,
Tomasz

On 20.09.2017 10:52, Tomasz Nowicki wrote:
> Here is my test setup where I have stareted performance measurements.
> 
>   ------------  PCIe  -------------   TX   -------------  PCIe  -----
> | ThunderX2  |------| Intel XL710 | ---> | Intel XL710 |------| X86 |
> | (128 cpus) |      |   40GbE     |      |    40GbE    |       -----
>   ------------        -------------        -------------
> 
> As the reference lets take v4.13 host, SMMUv3 off and 1-thread iperf
> taskset to one CPU. The performance results I got:
> 
> SMMU off -> 100%
> SMMU on -> 0,02%
> 
> I followed down the DMA mapping path and found out IOVA 32-bit space
> full so that kernel was flushing rcaches for all CPUs in (1).
> For 128 CPUs, this kills the performance. Furthermore, for my case, rcaches
> contained PFNs > 32-bit mostly so the second round of IOVA allocation failed
> as well. As the consequence IOVA had to be allocated outside of 32-bit (2)
> from scratch since all rcaches have been flushed in (1).
> 
>      if (dma_limit > DMA_BIT_MASK(32) && dev_is_pci(dev))
> (1)-->  iova = alloc_iova_fast(iovad, iova_len, DMA_BIT_MASK(32) >> shift);
> 
>      if (!iova)
> (2)-->  iova = alloc_iova_fast(iovad, iova_len, dma_limit >> shift);
> 
> My fix simply introduces parameter for alloc_iova_fast() to decide whether
> rcache flush has to be done or not. All users follow mentioned scenario
> so they should let flush as the last chance to avoid time costly iteration
> over all CPUs.
> 
> This bring my iperf performance back to 100% with SMMU on.
> 
> My bad feelings regarding this solution is that machines with relatively
> small numbers of CPUs may get DAC addresses more frequently for PCI
> devices. Please let me know your thoughts.
> 
> Changelog:
> 
> v1 --> v2
> - add missing documentation
> - fix typo
> 
> Tomasz Nowicki (1):
>    iommu/iova: Make rcache flush optional on IOVA allocation failure
> 
>   drivers/iommu/amd_iommu.c   |  5 +++--
>   drivers/iommu/dma-iommu.c   |  6 ++++--
>   drivers/iommu/intel-iommu.c |  5 +++--
>   drivers/iommu/iova.c        | 11 ++++++-----
>   include/linux/iova.h        |  5 +++--
>   5 files changed, 19 insertions(+), 13 deletions(-)
> 

[PATCH V2 0/1] Optimise IOVA allocations for PCI devices

[Joerg Roedel]

Hi Tomasz,

On Thu, Oct 12, 2017 at 11:40:27AM +0200, Tomasz Nowicki wrote:
> Can you please have a look and see if you are fine with this patch?

How do these changes relate to Robin's IOVA optimizations already in the
iommu-tree?

Regards,

	Joerg

[PATCH V2 0/1] Optimise IOVA allocations for PCI devices

[Tomasz Nowicki]

Hi Joerg,

On 12.10.2017 12:04, Joerg Roedel wrote:
> Hi Tomasz,
> 
> On Thu, Oct 12, 2017 at 11:40:27AM +0200, Tomasz Nowicki wrote:
>> Can you please have a look and see if you are fine with this patch?
> 
> How do these changes relate to Robin's IOVA optimizations already in the
> iommu-tree?
> 

This is another optimization patch which does not conflict with Robin's 
patches.

Robin, please correct me if I am wrong.

Thanks,
Tomasz

[PATCH V2 0/1] Optimise IOVA allocations for PCI devices

[Robin Murphy]

On 12/10/17 11:08, Tomasz Nowicki wrote:
> Hi Joerg,
> 
> On 12.10.2017 12:04, Joerg Roedel wrote:
>> Hi Tomasz,
>>
>> On Thu, Oct 12, 2017 at 11:40:27AM +0200, Tomasz Nowicki wrote:
>>> Can you please have a look and see if you are fine with this patch?
>>
>> How do these changes relate to Robin's IOVA optimizations already in the
>> iommu-tree?
>>
> 
> This is another optimization patch which does not conflict with Robin's
> patches.
> 
> Robin, please correct me if I am wrong.

Yup, I've had Tomasz' patch included in my iommu/iova branch for a
while, and the "Misc. IOVA tweaks" patches were actually written on top
of it.

This should be the final piece of the puzzle for several arm64 server
platforms, which exacerbate the problem by using 64K pages and having
host bridge windows consume most of the 32-bit mem space.

Robin.

[PATCH V2 0/1] Optimise IOVA allocations for PCI devices

[Joerg Roedel]

On Thu, Oct 12, 2017 at 12:14:08PM +0100, Robin Murphy wrote:
> Yup, I've had Tomasz' patch included in my iommu/iova branch for a
> while, and the "Misc. IOVA tweaks" patches were actually written on top
> of it.
> 
> This should be the final piece of the puzzle for several arm64 server
> platforms, which exacerbate the problem by using 64K pages and having
> host bridge windows consume most of the 32-bit mem space.

Okay, thanks. Applied the patch.