| Message ID | 20240523031935.90856-1-amhetre@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] iommu: Optimize IOMMU UnMap | expand |
On 23/05/2024 4:19 am, Ashish Mhetre wrote: > The current __arm_lpae_unmap() function calls dma_sync() on individual > PTEs after clearing them. By updating the __arm_lpae_unmap() to call > dma_sync() once for all cleared PTEs, the overall performance can be > improved 25% for large buffer sizes. > Below is detailed analysis of average unmap latency(in us) with and > without this optimization obtained by running dma_map_benchmark for > different buffer sizes. > > Size Time W/O Time With % Improvement > Optimization Optimization > (us) (us) > > 4KB 3.0 3.1 -3.33 > 1MB 250.3 187.9 24.93 This seems highly suspect - the smallest possible block size is 2MB so a 1MB unmap should not be affected by this path at all. > 2MB 493.7 368.7 25.32 > 4MB 974.7 723.4 25.78 I'm guessing this is on Tegra with the workaround to force everything to PAGE_SIZE? In the normal case a 2MB unmap should be nominally *faster* than 4KB, since it would also be a single PTE, but with one fewer level of table to walk to reach it. The 25% figure is rather misleading if it's only a mitigation of an existing erratum workaround, and the actual impact on the majority of non-broken systems is unmeasured. (As an aside, I think that workaround itself is a bit broken, since at least on Tegra234 with Cortex-A78, PAGE_SIZE could be 16KB which MMU-500 doesn't support.) > Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> > --- > drivers/iommu/io-pgtable-arm.c | 34 +++++++++++++++++++++++++--------- > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > index 3d23b924cec1..94094b711cba 100644 > --- a/drivers/iommu/io-pgtable-arm.c > +++ b/drivers/iommu/io-pgtable-arm.c > @@ -256,13 +256,15 @@ static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep, int num_entries, > sizeof(*ptep) * num_entries, DMA_TO_DEVICE); > } > > -static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct io_pgtable_cfg *cfg) > +static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct io_pgtable_cfg *cfg, int num_entries) > { > + int i; > > - *ptep = 0; > + for (i = 0; i < num_entries; i++) > + ptep[i] = 0; > > if (!cfg->coherent_walk) > - __arm_lpae_sync_pte(ptep, 1, cfg); > + __arm_lpae_sync_pte(ptep, num_entries, cfg); > } > > static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > @@ -633,13 +635,25 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) { > max_entries = ARM_LPAE_PTES_PER_TABLE(data) - unmap_idx_start; > num_entries = min_t(int, pgcount, max_entries); > - > - while (i < num_entries) { > - pte = READ_ONCE(*ptep); > + arm_lpae_iopte *pte_flush; > + int j = 0; > + > + pte_flush = kvcalloc(num_entries, sizeof(*pte_flush), GFP_ATOMIC); kvmalloc() with GFP_ATOMIC isn't valid. However, I'm not sure if there isn't a more fundamental problem here - Rob, Boris; was it just the map path, or would any allocation on unmap risk the GPU reclaim deadlock thing as well? Thanks, Robin. > + if (pte_flush) { > + for (j = 0; j < num_entries; j++) { > + pte_flush[j] = READ_ONCE(ptep[j]); > + if (WARN_ON(!pte_flush[j])) > + break; > + } > + __arm_lpae_clear_pte(ptep, &iop->cfg, j); > + } > + while (i < (pte_flush ? j : num_entries)) { > + pte = pte_flush ? pte_flush[i] : READ_ONCE(*ptep); > if (WARN_ON(!pte)) > break; > > - __arm_lpae_clear_pte(ptep, &iop->cfg); > + if (!pte_flush) > + __arm_lpae_clear_pte(ptep, &iop->cfg, 1); > > if (!iopte_leaf(pte, lvl, iop->fmt)) { > /* Also flush any partial walks */ > @@ -649,10 +663,12 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > } else if (!iommu_iotlb_gather_queued(gather)) { > io_pgtable_tlb_add_page(iop, gather, iova + i * size, size); > } > - > - ptep++; > + if (!pte_flush) > + ptep++; > i++; > } > + if (pte_flush) > + kvfree(pte_flush); > > return i * size; > } else if (iopte_leaf(pte, lvl, iop->fmt)) {
On Thu, 23 May 2024 14:41:12 +0100 Robin Murphy <robin.murphy@arm.com> wrote: > On 23/05/2024 4:19 am, Ashish Mhetre wrote: > > The current __arm_lpae_unmap() function calls dma_sync() on individual > > PTEs after clearing them. By updating the __arm_lpae_unmap() to call > > dma_sync() once for all cleared PTEs, the overall performance can be > > improved 25% for large buffer sizes. > > Below is detailed analysis of average unmap latency(in us) with and > > without this optimization obtained by running dma_map_benchmark for > > different buffer sizes. > > > > Size Time W/O Time With % Improvement > > Optimization Optimization > > (us) (us) > > > > 4KB 3.0 3.1 -3.33 > > 1MB 250.3 187.9 24.93 > > This seems highly suspect - the smallest possible block size is 2MB so a > 1MB unmap should not be affected by this path at all. > > > 2MB 493.7 368.7 25.32 > > 4MB 974.7 723.4 25.78 > > I'm guessing this is on Tegra with the workaround to force everything to > PAGE_SIZE? In the normal case a 2MB unmap should be nominally *faster* > than 4KB, since it would also be a single PTE, but with one fewer level > of table to walk to reach it. The 25% figure is rather misleading if > it's only a mitigation of an existing erratum workaround, and the actual > impact on the majority of non-broken systems is unmeasured. > > (As an aside, I think that workaround itself is a bit broken, since at > least on Tegra234 with Cortex-A78, PAGE_SIZE could be 16KB which MMU-500 > doesn't support.) > > > Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> > > --- > > drivers/iommu/io-pgtable-arm.c | 34 +++++++++++++++++++++++++--------- > > 1 file changed, 25 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > > index 3d23b924cec1..94094b711cba 100644 > > --- a/drivers/iommu/io-pgtable-arm.c > > +++ b/drivers/iommu/io-pgtable-arm.c > > @@ -256,13 +256,15 @@ static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep, int num_entries, > > sizeof(*ptep) * num_entries, DMA_TO_DEVICE); > > } > > > > -static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct io_pgtable_cfg *cfg) > > +static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct io_pgtable_cfg *cfg, int num_entries) > > { > > + int i; > > > > - *ptep = 0; > > + for (i = 0; i < num_entries; i++) > > + ptep[i] = 0; > > > > if (!cfg->coherent_walk) > > - __arm_lpae_sync_pte(ptep, 1, cfg); > > + __arm_lpae_sync_pte(ptep, num_entries, cfg); > > } > > > > static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > > @@ -633,13 +635,25 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > > if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) { > > max_entries = ARM_LPAE_PTES_PER_TABLE(data) - unmap_idx_start; > > num_entries = min_t(int, pgcount, max_entries); > > - > > - while (i < num_entries) { > > - pte = READ_ONCE(*ptep); > > + arm_lpae_iopte *pte_flush; > > + int j = 0; > > + > > + pte_flush = kvcalloc(num_entries, sizeof(*pte_flush), GFP_ATOMIC); > > kvmalloc() with GFP_ATOMIC isn't valid. However, I'm not sure if there > isn't a more fundamental problem here - Rob, Boris; was it just the map > path, or would any allocation on unmap risk the GPU reclaim deadlock > thing as well? Unmap as well, because of the 'split huge page into small pages' logic when the unmap region is not aligned on 2MB.
On 5/23/2024 7:11 PM, Robin Murphy wrote: > External email: Use caution opening links or attachments > > > On 23/05/2024 4:19 am, Ashish Mhetre wrote: >> The current __arm_lpae_unmap() function calls dma_sync() on individual >> PTEs after clearing them. By updating the __arm_lpae_unmap() to call >> dma_sync() once for all cleared PTEs, the overall performance can be >> improved 25% for large buffer sizes. >> Below is detailed analysis of average unmap latency(in us) with and >> without this optimization obtained by running dma_map_benchmark for >> different buffer sizes. >> >> Size Time W/O Time With % Improvement >> Optimization Optimization >> (us) (us) >> >> 4KB 3.0 3.1 -3.33 >> 1MB 250.3 187.9 24.93 > > This seems highly suspect - the smallest possible block size is 2MB so a > 1MB unmap should not be affected by this path at all. > It will be unmapped at 4KB block size, right? The 'size' passed to __arm_lpae_unmap will be 4KB and 'pgcount' will be 256 for 1MB buffer from iommu_pgsize() unless the IOVA and phys address met conditions for next bigger size i.e., 2MB. >> 2MB 493.7 368.7 25.32 >> 4MB 974.7 723.4 25.78 > > I'm guessing this is on Tegra with the workaround to force everything to > PAGE_SIZE? In the normal case a 2MB unmap should be nominally *faster* > than 4KB, since it would also be a single PTE, but with one fewer level > of table to walk to reach it. The 25% figure is rather misleading if > it's only a mitigation of an existing erratum workaround, and the actual > impact on the majority of non-broken systems is unmeasured. > Yes, I forgot about the workaround we have and agree that without the workaround, 2MB unmap will be faster without this optimization. But for any size between 4KB and 2MB, this optimization would help in improving the unmap latencies. To verify that, I reverted the workaround and again got unmap latencies using dma_map_benchmark which are as mentioned below. We can see an improvement around 20% to 25%: Size Time WO Opt(us) Time With Opt(us) % improvement 4KB 3 3.1 -3.33 64KB 18.6 15 19.36 128KB 35.2 27.7 21.31 256KB 67.6 52.6 22.19 512KB 128.4 97.7 23.91 1MB 249.9 188.1 24.72 2MB 67.4 67.5 -0.15 4MB 121.3 121.2 0.08 > (As an aside, I think that workaround itself is a bit broken, since at > least on Tegra234 with Cortex-A78, PAGE_SIZE could be 16KB which MMU-500 > doesn't support.) > Yes, that's true. For 16KB PAGE_SIZE, we need to fall back to 4KB pgsize_bitmap. >> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> >> --- >> drivers/iommu/io-pgtable-arm.c | 34 +++++++++++++++++++++++++--------- >> 1 file changed, 25 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/iommu/io-pgtable-arm.c >> b/drivers/iommu/io-pgtable-arm.c >> index 3d23b924cec1..94094b711cba 100644 >> --- a/drivers/iommu/io-pgtable-arm.c >> +++ b/drivers/iommu/io-pgtable-arm.c >> @@ -256,13 +256,15 @@ static void __arm_lpae_sync_pte(arm_lpae_iopte >> *ptep, int num_entries, >> sizeof(*ptep) * num_entries, >> DMA_TO_DEVICE); >> } >> >> -static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct >> io_pgtable_cfg *cfg) >> +static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct >> io_pgtable_cfg *cfg, int num_entries) >> { >> + int i; >> >> - *ptep = 0; >> + for (i = 0; i < num_entries; i++) >> + ptep[i] = 0; >> >> if (!cfg->coherent_walk) >> - __arm_lpae_sync_pte(ptep, 1, cfg); >> + __arm_lpae_sync_pte(ptep, num_entries, cfg); >> } >> >> static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, >> @@ -633,13 +635,25 @@ static size_t __arm_lpae_unmap(struct >> arm_lpae_io_pgtable *data, >> if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) { >> max_entries = ARM_LPAE_PTES_PER_TABLE(data) - >> unmap_idx_start; >> num_entries = min_t(int, pgcount, max_entries); >> - >> - while (i < num_entries) { >> - pte = READ_ONCE(*ptep); >> + arm_lpae_iopte *pte_flush; >> + int j = 0; >> + >> + pte_flush = kvcalloc(num_entries, sizeof(*pte_flush), >> GFP_ATOMIC); > > kvmalloc() with GFP_ATOMIC isn't valid. However, I'm not sure if there > isn't a more fundamental problem here - Rob, Boris; was it just the map > path, or would any allocation on unmap risk the GPU reclaim deadlock > thing as well? > I am using kvmalloc() here to create an array which is used to store PTEs that are going to be flushed after clearing. If we don't store them then those will be lost once cleared and we won't be able to flush them. I tried using GFP_KERNEL instead of GFP_ATOMIC but then I am getting warning from might_sleep(). Is there any other alternative way we can use here to store the PTEs? > Thanks, > Robin. > >> + if (pte_flush) { >> + for (j = 0; j < num_entries; j++) { >> + pte_flush[j] = READ_ONCE(ptep[j]); >> + if (WARN_ON(!pte_flush[j])) >> + break; >> + } >> + __arm_lpae_clear_pte(ptep, &iop->cfg, j); >> + } >> + while (i < (pte_flush ? j : num_entries)) { >> + pte = pte_flush ? pte_flush[i] : READ_ONCE(*ptep); >> if (WARN_ON(!pte)) >> break; >> >> - __arm_lpae_clear_pte(ptep, &iop->cfg); >> + if (!pte_flush) >> + __arm_lpae_clear_pte(ptep, &iop->cfg, 1); >> >> if (!iopte_leaf(pte, lvl, iop->fmt)) { >> /* Also flush any partial walks */ >> @@ -649,10 +663,12 @@ static size_t __arm_lpae_unmap(struct >> arm_lpae_io_pgtable *data, >> } else if (!iommu_iotlb_gather_queued(gather)) { >> io_pgtable_tlb_add_page(iop, gather, >> iova + i * size, size); >> } >> - >> - ptep++; >> + if (!pte_flush) >> + ptep++; >> i++; >> } >> + if (pte_flush) >> + kvfree(pte_flush); >> >> return i * size; >> } else if (iopte_leaf(pte, lvl, iop->fmt)) {
On 5/24/2024 6:09 PM, Ashish Mhetre wrote: > > On 5/23/2024 7:11 PM, Robin Murphy wrote: >> External email: Use caution opening links or attachments >> >> >> On 23/05/2024 4:19 am, Ashish Mhetre wrote: >>> The current __arm_lpae_unmap() function calls dma_sync() on individual >>> PTEs after clearing them. By updating the __arm_lpae_unmap() to call >>> dma_sync() once for all cleared PTEs, the overall performance can be >>> improved 25% for large buffer sizes. >>> Below is detailed analysis of average unmap latency(in us) with and >>> without this optimization obtained by running dma_map_benchmark for >>> different buffer sizes. >>> >>> Size Time W/O Time With % Improvement >>> Optimization Optimization >>> (us) (us) >>> >>> 4KB 3.0 3.1 -3.33 >>> 1MB 250.3 187.9 24.93 >> >> This seems highly suspect - the smallest possible block size is 2MB so a >> 1MB unmap should not be affected by this path at all. >> > It will be unmapped at 4KB block size, right? The 'size' passed to > __arm_lpae_unmap will be 4KB and 'pgcount' will be 256 for 1MB > buffer from iommu_pgsize() unless the IOVA and phys address met > conditions for next bigger size i.e., 2MB. >>> 2MB 493.7 368.7 25.32 >>> 4MB 974.7 723.4 25.78 >> >> I'm guessing this is on Tegra with the workaround to force everything to >> PAGE_SIZE? In the normal case a 2MB unmap should be nominally *faster* >> than 4KB, since it would also be a single PTE, but with one fewer level >> of table to walk to reach it. The 25% figure is rather misleading if >> it's only a mitigation of an existing erratum workaround, and the actual >> impact on the majority of non-broken systems is unmeasured. >> > Yes, I forgot about the workaround we have and agree that without the > workaround, 2MB unmap will be faster without this optimization. But > for any size between 4KB and 2MB, this optimization would help in > improving the unmap latencies. To verify that, I reverted the workaround > and again got unmap latencies using dma_map_benchmark which are as > mentioned below. We can see an improvement around 20% to 25%: > > Size Time WO Opt(us) Time With Opt(us) % improvement > 4KB 3 3.1 -3.33 > 64KB 18.6 15 19.36 > 128KB 35.2 27.7 21.31 > 256KB 67.6 52.6 22.19 > 512KB 128.4 97.7 23.91 > 1MB 249.9 188.1 24.72 > 2MB 67.4 67.5 -0.15 > 4MB 121.3 121.2 0.08 > >> (As an aside, I think that workaround itself is a bit broken, since at >> least on Tegra234 with Cortex-A78, PAGE_SIZE could be 16KB which MMU-500 >> doesn't support.) >> > Yes, that's true. For 16KB PAGE_SIZE, we need to fall back to 4KB > pgsize_bitmap. >>> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> >>> --- >>> drivers/iommu/io-pgtable-arm.c | 34 >>> +++++++++++++++++++++++++--------- >>> 1 file changed, 25 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/iommu/io-pgtable-arm.c >>> b/drivers/iommu/io-pgtable-arm.c >>> index 3d23b924cec1..94094b711cba 100644 >>> --- a/drivers/iommu/io-pgtable-arm.c >>> +++ b/drivers/iommu/io-pgtable-arm.c >>> @@ -256,13 +256,15 @@ static void __arm_lpae_sync_pte(arm_lpae_iopte >>> *ptep, int num_entries, >>> sizeof(*ptep) * num_entries, >>> DMA_TO_DEVICE); >>> } >>> >>> -static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct >>> io_pgtable_cfg *cfg) >>> +static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct >>> io_pgtable_cfg *cfg, int num_entries) >>> { >>> + int i; >>> >>> - *ptep = 0; >>> + for (i = 0; i < num_entries; i++) >>> + ptep[i] = 0; >>> >>> if (!cfg->coherent_walk) >>> - __arm_lpae_sync_pte(ptep, 1, cfg); >>> + __arm_lpae_sync_pte(ptep, num_entries, cfg); >>> } >>> >>> static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, >>> @@ -633,13 +635,25 @@ static size_t __arm_lpae_unmap(struct >>> arm_lpae_io_pgtable *data, >>> if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) { >>> max_entries = ARM_LPAE_PTES_PER_TABLE(data) - >>> unmap_idx_start; >>> num_entries = min_t(int, pgcount, max_entries); >>> - >>> - while (i < num_entries) { >>> - pte = READ_ONCE(*ptep); >>> + arm_lpae_iopte *pte_flush; >>> + int j = 0; >>> + >>> + pte_flush = kvcalloc(num_entries, sizeof(*pte_flush), >>> GFP_ATOMIC); >> >> kvmalloc() with GFP_ATOMIC isn't valid. However, I'm not sure if there >> isn't a more fundamental problem here - Rob, Boris; was it just the map >> path, or would any allocation on unmap risk the GPU reclaim deadlock >> thing as well? >> > I am using kvmalloc() here to create an array which is used to store PTEs > that are going to be flushed after clearing. If we don't store them then > those will be lost once cleared and we won't be able to flush them. > I tried using GFP_KERNEL instead of GFP_ATOMIC but then I am getting > warning from might_sleep(). > Is there any other alternative way we can use here to store the PTEs? >> Thanks, >> Robin. >> >>> + if (pte_flush) { >>> + for (j = 0; j < num_entries; j++) { >>> + pte_flush[j] = READ_ONCE(ptep[j]); >>> + if (WARN_ON(!pte_flush[j])) >>> + break; >>> + } >>> + __arm_lpae_clear_pte(ptep, &iop->cfg, j); >>> + } >>> + while (i < (pte_flush ? j : num_entries)) { >>> + pte = pte_flush ? pte_flush[i] : >>> READ_ONCE(*ptep); >>> if (WARN_ON(!pte)) >>> break; >>> >>> - __arm_lpae_clear_pte(ptep, &iop->cfg); >>> + if (!pte_flush) >>> + __arm_lpae_clear_pte(ptep, &iop->cfg, 1); >>> >>> if (!iopte_leaf(pte, lvl, iop->fmt)) { >>> /* Also flush any partial walks */ >>> @@ -649,10 +663,12 @@ static size_t __arm_lpae_unmap(struct >>> arm_lpae_io_pgtable *data, >>> } else if (!iommu_iotlb_gather_queued(gather)) { >>> io_pgtable_tlb_add_page(iop, gather, >>> iova + i * size, size); >>> } >>> - >>> - ptep++; >>> + if (!pte_flush) >>> + ptep++; >>> i++; >>> } >>> + if (pte_flush) >>> + kvfree(pte_flush); >>> >>> return i * size; >>> } else if (iopte_leaf(pte, lvl, iop->fmt)) { Hi all, Can you please provide feedback on this patch? Is this optimization worth pursuing? Thanks, Ashish Mhetre
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c index 3d23b924cec1..94094b711cba 100644 --- a/drivers/iommu/io-pgtable-arm.c +++ b/drivers/iommu/io-pgtable-arm.c @@ -256,13 +256,15 @@ static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep, int num_entries, sizeof(*ptep) * num_entries, DMA_TO_DEVICE); } -static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct io_pgtable_cfg *cfg) +static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct io_pgtable_cfg *cfg, int num_entries) { + int i; - *ptep = 0; + for (i = 0; i < num_entries; i++) + ptep[i] = 0; if (!cfg->coherent_walk) - __arm_lpae_sync_pte(ptep, 1, cfg); + __arm_lpae_sync_pte(ptep, num_entries, cfg); } static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, @@ -633,13 +635,25 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) { max_entries = ARM_LPAE_PTES_PER_TABLE(data) - unmap_idx_start; num_entries = min_t(int, pgcount, max_entries); - - while (i < num_entries) { - pte = READ_ONCE(*ptep); + arm_lpae_iopte *pte_flush; + int j = 0; + + pte_flush = kvcalloc(num_entries, sizeof(*pte_flush), GFP_ATOMIC); + if (pte_flush) { + for (j = 0; j < num_entries; j++) { + pte_flush[j] = READ_ONCE(ptep[j]); + if (WARN_ON(!pte_flush[j])) + break; + } + __arm_lpae_clear_pte(ptep, &iop->cfg, j); + } + while (i < (pte_flush ? j : num_entries)) { + pte = pte_flush ? pte_flush[i] : READ_ONCE(*ptep); if (WARN_ON(!pte)) break; - __arm_lpae_clear_pte(ptep, &iop->cfg); + if (!pte_flush) + __arm_lpae_clear_pte(ptep, &iop->cfg, 1); if (!iopte_leaf(pte, lvl, iop->fmt)) { /* Also flush any partial walks */ @@ -649,10 +663,12 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, } else if (!iommu_iotlb_gather_queued(gather)) { io_pgtable_tlb_add_page(iop, gather, iova + i * size, size); } - - ptep++; + if (!pte_flush) + ptep++; i++; } + if (pte_flush) + kvfree(pte_flush); return i * size; } else if (iopte_leaf(pte, lvl, iop->fmt)) {
The current __arm_lpae_unmap() function calls dma_sync() on individual PTEs after clearing them. By updating the __arm_lpae_unmap() to call dma_sync() once for all cleared PTEs, the overall performance can be improved 25% for large buffer sizes. Below is detailed analysis of average unmap latency(in us) with and without this optimization obtained by running dma_map_benchmark for different buffer sizes. Size Time W/O Time With % Improvement Optimization Optimization (us) (us) 4KB 3.0 3.1 -3.33 1MB 250.3 187.9 24.93 2MB 493.7 368.7 25.32 4MB 974.7 723.4 25.78 Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> --- drivers/iommu/io-pgtable-arm.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)