On 15/04/2021 04:52, chenxiang wrote:

From: Xiang Chen chenxiang66@hisilicon.com

It is not necessary to put free_iova_mem() inside of spinlock/unlock iova_rbtree_lock which only leads to more completion for the spinlock. It has a small promote on the performance after the change. And also rename private_free_iova() as remove_iova() because the function will not free iova after that change.

Signed-off-by: Xiang Chen chenxiang66@hisilicon.com

drivers/iommu/iova.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c index b7ecd5b..c10af23 100644 --- a/drivers/iommu/iova.c +++ b/drivers/iommu/iova.c @@ -412,12 +412,11 @@ private_find_iova(struct iova_domain *iovad, unsigned long pfn) return NULL; }

-static void private_free_iova(struct iova_domain *iovad, struct iova *iova) +static void remove_iova(struct iova_domain *iovad, struct iova *iova) { assert_spin_locked(&iovad->iova_rbtree_lock); __cached_rbnode_delete_update(iovad, iova); rb_erase(&iova->node, &iovad->rbroot);

• free_iova_mem(iova); }

  /**

@@ -452,8 +451,9 @@ __free_iova(struct iova_domain *iovad, struct iova *iova) unsigned long flags;

spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);

• private_free_iova(iovad, iova);

• remove_iova(iovad, iova); spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
• free_iova_mem(iova); } EXPORT_SYMBOL_GPL(__free_iova);

@@ -473,9 +473,9 @@ free_iova(struct iova_domain *iovad, unsigned long pfn) spin_lock_irqsave(&iovad->iova_rbtree_lock, flags); iova = private_find_iova(iovad, pfn); if (iova)

• private_free_iova(iovad, iova);
  

• remove_iova(iovad, iova);
  

  spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);

• free_iova_mem(iova);

you need to make this: if (iova) free_iova_mem(iova);

as free_iova_mem(iova) dereferences iova:

if (iova->pfn_lo != IOVA_ANCHOR) kmem_cache_free(iova_cache, iova)

So if iova were NULL, we crash.

Or you can make free_iova_mem() NULL safe.

} EXPORT_SYMBOL_GPL(free_iova);

@@ -825,7 +825,8 @@ iova_magazine_free_pfns(struct iova_magazine *mag, struct iova_domain *iovad) if (WARN_ON(!iova)) continue;

• private_free_iova(iovad, iova);
  

• remove_iova(iovad, iova);
  

• free_iova_mem(iova);
  

  }

  spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);