lkml.org 
[lkml]   [2020]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 1/2] iommu/iova: Flush CPU rcache for when a depot fills
From
Date
On 25/09/2020 15:34, John Garry wrote:
> Indeed, I think that the mainline code has a bug:
>
> If the initial allocation for the loaded/prev magazines fail (give NULL)
> in init_iova_rcaches(), then in __iova_rcache_insert():
>
> if (!iova_magazine_full(cpu_rcache->loaded)) {
>     can_insert = true;
>
> If cpu_rcache->loaded == NULL, then can_insert is assigned true -> bang,
> as I experimented, below. This needs to be fixed...
>

This looks better:

Subject: [PATCH] iommu/iova: Avoid double-negatives with magazine helpers

Expression !iova_magazine_full(mag) evaluates true when mag == NULL.

This falls over in __iova_rcache_insert() when loaded == NULL:

if (!iova_magazine_full(cpu_rcache->loaded)) {
can_insert = true;

...

if (can_insert)
iova_magazine_push(cpu_rcache->loaded, iova_pfn);

Here, can_insert is evaluated true, which is wrong. Members
loaded/prev can possibly be NULL if the initial allocations fail in
__iova_rcache_insert().

Let's stop using double-negatives, like !iova_magazine_full(), and use
iova_magazine_has_space() instead in this case. And similar for
!iova_magazine_empty().

Signed-off-by: John Garry <john.garry@huawei.com>

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 5b4ffab7140b..42ca9d0f39b7 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -827,14 +827,18 @@ iova_magazine_free_pfns(struct iova_magazine *mag,
struct iova_domain *iovad)
mag->size = 0;
}

-static bool iova_magazine_full(struct iova_magazine *mag)
+static bool iova_magazine_has_space(struct iova_magazine *mag)
{
- return (mag && mag->size == IOVA_MAG_SIZE);
+ if (!mag)
+ return false;
+ return mag->size < IOVA_MAG_SIZE;
}

-static bool iova_magazine_empty(struct iova_magazine *mag)
+static bool iova_magazine_has_pfns(struct iova_magazine *mag)
{
- return (!mag || mag->size == 0);
+ if (!mag)
+ return false;
+ return mag->size;
}

static unsigned long iova_magazine_pop(struct iova_magazine *mag,
@@ -843,7 +847,7 @@ static unsigned long iova_magazine_pop(struct
iova_magazine *mag,
int i;
unsigned long pfn;

- BUG_ON(iova_magazine_empty(mag));
+ BUG_ON(!iova_magazine_has_pfns(mag));

/* Only fall back to the rbtree if we have no suitable pfns at all */
for (i = mag->size - 1; mag->pfns[i] > limit_pfn; i--)
@@ -859,7 +863,7 @@ static unsigned long iova_magazine_pop(struct
iova_magazine *mag,

static void iova_magazine_push(struct iova_magazine *mag, unsigned
long pfn)
{
- BUG_ON(iova_magazine_full(mag));
+ BUG_ON(!iova_magazine_has_space(mag));

mag->pfns[mag->size++] = pfn;
}
@@ -905,9 +909,9 @@ static bool __iova_rcache_insert(struct iova_domain
*iovad,
cpu_rcache = raw_cpu_ptr(rcache->cpu_rcaches);
spin_lock_irqsave(&cpu_rcache->lock, flags);

- if (!iova_magazine_full(cpu_rcache->loaded)) {
+ if (iova_magazine_has_space(cpu_rcache->loaded)) {
can_insert = true;
- } else if (!iova_magazine_full(cpu_rcache->prev)) {
+ } else if (iova_magazine_has_space(cpu_rcache->prev)) {
swap(cpu_rcache->prev, cpu_rcache->loaded);
can_insert = true;
} else {
@@ -915,7 +919,8 @@ static bool __iova_rcache_insert(struct iova_domain
*iovad,

if (new_mag) {
spin_lock(&rcache->lock);
- if (rcache->depot_size < MAX_GLOBAL_MAGS) {
+ if (rcache->depot_size < MAX_GLOBAL_MAGS &&
+ cpu_rcache->loaded) {
rcache->depot[rcache->depot_size++] =
cpu_rcache->loaded;
} else {
@@ -968,9 +973,9 @@ static unsigned long __iova_rcache_get(struct
iova_rcache *rcache,
cpu_rcache = raw_cpu_ptr(rcache->cpu_rcaches);
spin_lock_irqsave(&cpu_rcache->lock, flags);

- if (!iova_magazine_empty(cpu_rcache->loaded)) {
+ if (iova_magazine_has_pfns(cpu_rcache->loaded)) {
has_pfn = true;
- } else if (!iova_magazine_empty(cpu_rcache->prev)) {
+ } else if (iova_magazine_has_pfns(cpu_rcache->prev)) {
swap(cpu_rcache->prev, cpu_rcache->loaded);
has_pfn = true;
} else {
--
2.26.2
\
 
 \ /
  Last update: 2020-09-25 18:16    [W:0.297 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site