* [Patch v3 1/3] iommu: avoid unnecessary magazine allocations
2019-12-06 21:38 [Patch v3 0/3] iommu: reduce spinlock contention on fast path Cong Wang
@ 2019-12-06 21:38 ` Cong Wang
2019-12-06 21:38 ` [Patch v3 2/3] iommu: optimize iova_magazine_free_pfns() Cong Wang
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Cong Wang @ 2019-12-06 21:38 UTC (permalink / raw)
To: iommu; +Cc: Cong Wang, linux-kernel
The IOVA cache algorithm implemented in IOMMU code does not
exactly match the original algorithm described in the paper
"Magazines and Vmem: Extending the Slab Allocator to Many
CPUs and Arbitrary Resources".
Particularly, it doesn't need to free the loaded empty magazine
when trying to put it back to global depot. To make it work, we
have to pre-allocate magazines in the depot and only recycle them
when all of them are full.
Before this patch, rcache->depot[] contains either full or
freed entries, after this patch, it contains either full or
empty (but allocated) entries.
Together with a few other changes to make it exactly match
the pseudo code in the paper.
Cc: Joerg Roedel <joro@8bytes.org>
Cc: John Garry <john.garry@huawei.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
drivers/iommu/iova.c | 45 +++++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 41c605b0058f..cb473ddce4cf 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -862,12 +862,16 @@ static void init_iova_rcaches(struct iova_domain *iovad)
struct iova_cpu_rcache *cpu_rcache;
struct iova_rcache *rcache;
unsigned int cpu;
- int i;
+ int i, j;
for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
rcache = &iovad->rcaches[i];
spin_lock_init(&rcache->lock);
rcache->depot_size = 0;
+ for (j = 0; j < MAX_GLOBAL_MAGS; ++j) {
+ rcache->depot[j] = iova_magazine_alloc(GFP_KERNEL);
+ WARN_ON(!rcache->depot[j]);
+ }
rcache->cpu_rcaches = __alloc_percpu(sizeof(*cpu_rcache), cache_line_size());
if (WARN_ON(!rcache->cpu_rcaches))
continue;
@@ -900,24 +904,30 @@ static bool __iova_rcache_insert(struct iova_domain *iovad,
if (!iova_magazine_full(cpu_rcache->loaded)) {
can_insert = true;
- } else if (!iova_magazine_full(cpu_rcache->prev)) {
+ } else if (iova_magazine_empty(cpu_rcache->prev)) {
swap(cpu_rcache->prev, cpu_rcache->loaded);
can_insert = true;
} else {
- struct iova_magazine *new_mag = iova_magazine_alloc(GFP_ATOMIC);
+ spin_lock(&rcache->lock);
+ if (rcache->depot_size < MAX_GLOBAL_MAGS) {
+ swap(rcache->depot[rcache->depot_size], cpu_rcache->prev);
+ swap(cpu_rcache->prev, cpu_rcache->loaded);
+ rcache->depot_size++;
+ can_insert = true;
+ } else {
+ mag_to_free = cpu_rcache->loaded;
+ }
+ spin_unlock(&rcache->lock);
+
+ if (mag_to_free) {
+ struct iova_magazine *new_mag = iova_magazine_alloc(GFP_ATOMIC);
- if (new_mag) {
- spin_lock(&rcache->lock);
- if (rcache->depot_size < MAX_GLOBAL_MAGS) {
- rcache->depot[rcache->depot_size++] =
- cpu_rcache->loaded;
+ if (new_mag) {
+ cpu_rcache->loaded = new_mag;
+ can_insert = true;
} else {
- mag_to_free = cpu_rcache->loaded;
+ mag_to_free = NULL;
}
- spin_unlock(&rcache->lock);
-
- cpu_rcache->loaded = new_mag;
- can_insert = true;
}
}
@@ -963,14 +973,15 @@ static unsigned long __iova_rcache_get(struct iova_rcache *rcache,
if (!iova_magazine_empty(cpu_rcache->loaded)) {
has_pfn = true;
- } else if (!iova_magazine_empty(cpu_rcache->prev)) {
+ } else if (iova_magazine_full(cpu_rcache->prev)) {
swap(cpu_rcache->prev, cpu_rcache->loaded);
has_pfn = true;
} else {
spin_lock(&rcache->lock);
if (rcache->depot_size > 0) {
- iova_magazine_free(cpu_rcache->loaded);
- cpu_rcache->loaded = rcache->depot[--rcache->depot_size];
+ swap(rcache->depot[rcache->depot_size - 1], cpu_rcache->prev);
+ swap(cpu_rcache->prev, cpu_rcache->loaded);
+ rcache->depot_size--;
has_pfn = true;
}
spin_unlock(&rcache->lock);
@@ -1019,7 +1030,7 @@ static void free_iova_rcaches(struct iova_domain *iovad)
iova_magazine_free(cpu_rcache->prev);
}
free_percpu(rcache->cpu_rcaches);
- for (j = 0; j < rcache->depot_size; ++j)
+ for (j = 0; j < MAX_GLOBAL_MAGS; ++j)
iova_magazine_free(rcache->depot[j]);
}
}
--
2.21.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Patch v3 2/3] iommu: optimize iova_magazine_free_pfns()
2019-12-06 21:38 [Patch v3 0/3] iommu: reduce spinlock contention on fast path Cong Wang
2019-12-06 21:38 ` [Patch v3 1/3] iommu: avoid unnecessary magazine allocations Cong Wang
@ 2019-12-06 21:38 ` Cong Wang
2019-12-06 21:38 ` [Patch v3 3/3] iommu: avoid taking iova_rbtree_lock twice Cong Wang
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Cong Wang @ 2019-12-06 21:38 UTC (permalink / raw)
To: iommu; +Cc: Cong Wang, linux-kernel
If the magazine is empty, iova_magazine_free_pfns() should
be a nop, however it misses the case of mag->size==0. So we
should just call iova_magazine_empty().
This should reduce the contention on iovad->iova_rbtree_lock
a little bit, not much at all.
Cc: Joerg Roedel <joro@8bytes.org>
Cc: John Garry <john.garry@huawei.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
drivers/iommu/iova.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index cb473ddce4cf..184d4c0e20b5 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -797,13 +797,23 @@ static void iova_magazine_free(struct iova_magazine *mag)
kfree(mag);
}
+static bool iova_magazine_full(struct iova_magazine *mag)
+{
+ return (mag && mag->size == IOVA_MAG_SIZE);
+}
+
+static bool iova_magazine_empty(struct iova_magazine *mag)
+{
+ return (!mag || mag->size == 0);
+}
+
static void
iova_magazine_free_pfns(struct iova_magazine *mag, struct iova_domain *iovad)
{
unsigned long flags;
int i;
- if (!mag)
+ if (iova_magazine_empty(mag))
return;
spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);
@@ -820,16 +830,6 @@ iova_magazine_free_pfns(struct iova_magazine *mag, struct iova_domain *iovad)
mag->size = 0;
}
-static bool iova_magazine_full(struct iova_magazine *mag)
-{
- return (mag && mag->size == IOVA_MAG_SIZE);
-}
-
-static bool iova_magazine_empty(struct iova_magazine *mag)
-{
- return (!mag || mag->size == 0);
-}
-
static unsigned long iova_magazine_pop(struct iova_magazine *mag,
unsigned long limit_pfn)
{
--
2.21.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Patch v3 3/3] iommu: avoid taking iova_rbtree_lock twice
2019-12-06 21:38 [Patch v3 0/3] iommu: reduce spinlock contention on fast path Cong Wang
2019-12-06 21:38 ` [Patch v3 1/3] iommu: avoid unnecessary magazine allocations Cong Wang
2019-12-06 21:38 ` [Patch v3 2/3] iommu: optimize iova_magazine_free_pfns() Cong Wang
@ 2019-12-06 21:38 ` Cong Wang
2019-12-06 21:57 ` [Patch v3 0/3] iommu: reduce spinlock contention on fast path Qian Cai
2019-12-10 14:53 ` John Garry
4 siblings, 0 replies; 8+ messages in thread
From: Cong Wang @ 2019-12-06 21:38 UTC (permalink / raw)
To: iommu; +Cc: Cong Wang, linux-kernel
Both find_iova() and __free_iova() take iova_rbtree_lock,
there is no reason to take and release it twice inside
free_iova().
Fold them into one critical section by calling the unlock
versions instead.
Cc: Joerg Roedel <joro@8bytes.org>
Cc: John Garry <john.garry@huawei.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
drivers/iommu/iova.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 184d4c0e20b5..f46f8f794678 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -390,10 +390,14 @@ EXPORT_SYMBOL_GPL(__free_iova);
void
free_iova(struct iova_domain *iovad, unsigned long pfn)
{
- struct iova *iova = find_iova(iovad, pfn);
+ unsigned long flags;
+ struct iova *iova;
+ spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);
+ iova = private_find_iova(iovad, pfn);
if (iova)
- __free_iova(iovad, iova);
+ private_free_iova(iovad, iova);
+ spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
}
EXPORT_SYMBOL_GPL(free_iova);
--
2.21.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Patch v3 0/3] iommu: reduce spinlock contention on fast path
2019-12-06 21:38 [Patch v3 0/3] iommu: reduce spinlock contention on fast path Cong Wang
` (2 preceding siblings ...)
2019-12-06 21:38 ` [Patch v3 3/3] iommu: avoid taking iova_rbtree_lock twice Cong Wang
@ 2019-12-06 21:57 ` Qian Cai
2019-12-10 14:53 ` John Garry
4 siblings, 0 replies; 8+ messages in thread
From: Qian Cai @ 2019-12-06 21:57 UTC (permalink / raw)
To: Cong Wang; +Cc: iommu, linux-kernel
> On Dec 6, 2019, at 4:38 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> This patchset contains three small optimizations for the global spinlock
> contention in IOVA cache. Our memcache perf test shows this reduced its
> p999 latency down by 45% on AMD when IOMMU is enabled.
Can you at least have a changelog compared to previous versions?
>
> Cong Wang (3):
> iommu: avoid unnecessary magazine allocations
> iommu: optimize iova_magazine_free_pfns()
> iommu: avoid taking iova_rbtree_lock twice
> ---
> drivers/iommu/iova.c | 75 ++++++++++++++++++++++++++------------------
> 1 file changed, 45 insertions(+), 30 deletions(-)
>
> --
> 2.21.0
>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch v3 0/3] iommu: reduce spinlock contention on fast path
2019-12-06 21:38 [Patch v3 0/3] iommu: reduce spinlock contention on fast path Cong Wang
` (3 preceding siblings ...)
2019-12-06 21:57 ` [Patch v3 0/3] iommu: reduce spinlock contention on fast path Qian Cai
@ 2019-12-10 14:53 ` John Garry
4 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2019-12-10 14:53 UTC (permalink / raw)
To: Cong Wang, iommu; +Cc: linux-kernel
On 06/12/2019 21:38, Cong Wang wrote:
> This patchset contains three small optimizations for the global spinlock
> contention in IOVA cache. Our memcache perf test shows this reduced its
> p999 latency down by 45% on AMD when IOMMU is enabled.
>
> Cong Wang (3):
> iommu: avoid unnecessary magazine allocations
> iommu: optimize iova_magazine_free_pfns()
> iommu: avoid taking iova_rbtree_lock twice
> ---
> drivers/iommu/iova.c | 75 ++++++++++++++++++++++++++------------------
> 1 file changed, 45 insertions(+), 30 deletions(-)
>
I retested, and got a ~1.1% gain in throughput for my storage test -
results here if interested https://pastebin.com/dSQxYpN8
Thanks,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 8+ messages in thread