Re: [RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock

2021-06-04 Thread Joerg Roedel 
On Mon, May 10, 2021 at 07:53:02PM +0800, chenxiang wrote:
> From: Xiang Chen 
> 
> 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 
> Reviewed-by: John Garry 
> ---
>  drivers/iommu/iova.c | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)

Applied, thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock

2021-05-11 Thread Robin Murphy 

On 2021-05-10 12:53, chenxiang wrote:

From: Xiang Chen 

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.


Acked-by: Robin Murphy 


Signed-off-by: Xiang Chen 
Reviewed-by: John Garry 
---
  drivers/iommu/iova.c | 18 +++---
  1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index b7ecd5b..b6cf5f1 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(>iova_rbtree_lock);
__cached_rbnode_delete_update(iovad, iova);
rb_erase(>node, >rbroot);
-   free_iova_mem(iova);
  }
  
  /**

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

-   private_free_iova(iovad, iova);
+   remove_iova(iovad, iova);
spin_unlock_irqrestore(>iova_rbtree_lock, flags);
+   free_iova_mem(iova);
  }
  EXPORT_SYMBOL_GPL(__free_iova);
  
@@ -472,10 +472,13 @@ free_iova(struct iova_domain *iovad, unsigned long pfn)
  
  	spin_lock_irqsave(>iova_rbtree_lock, flags);

iova = private_find_iova(iovad, pfn);
-   if (iova)
-   private_free_iova(iovad, iova);
+   if (!iova) {
+   spin_unlock_irqrestore(>iova_rbtree_lock, flags);
+   return;
+   }
+   remove_iova(iovad, iova);
spin_unlock_irqrestore(>iova_rbtree_lock, flags);
-
+   free_iova_mem(iova);
  }
  EXPORT_SYMBOL_GPL(free_iova);
  
@@ -825,7 +828,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(>iova_rbtree_lock, flags);



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock

2021-05-10 Thread chenxiang 
From: Xiang Chen 

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 
Reviewed-by: John Garry 
---
 drivers/iommu/iova.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index b7ecd5b..b6cf5f1 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(>iova_rbtree_lock);
__cached_rbnode_delete_update(iovad, iova);
rb_erase(>node, >rbroot);
-   free_iova_mem(iova);
 }
 
 /**
@@ -452,8 +451,9 @@ __free_iova(struct iova_domain *iovad, struct iova *iova)
unsigned long flags;
 
spin_lock_irqsave(>iova_rbtree_lock, flags);
-   private_free_iova(iovad, iova);
+   remove_iova(iovad, iova);
spin_unlock_irqrestore(>iova_rbtree_lock, flags);
+   free_iova_mem(iova);
 }
 EXPORT_SYMBOL_GPL(__free_iova);
 
@@ -472,10 +472,13 @@ free_iova(struct iova_domain *iovad, unsigned long pfn)
 
spin_lock_irqsave(>iova_rbtree_lock, flags);
iova = private_find_iova(iovad, pfn);
-   if (iova)
-   private_free_iova(iovad, iova);
+   if (!iova) {
+   spin_unlock_irqrestore(>iova_rbtree_lock, flags);
+   return;
+   }
+   remove_iova(iovad, iova);
spin_unlock_irqrestore(>iova_rbtree_lock, flags);
-
+   free_iova_mem(iova);
 }
 EXPORT_SYMBOL_GPL(free_iova);
 
@@ -825,7 +828,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(>iova_rbtree_lock, flags);
-- 
2.8.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu