All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock
@ 2021-05-10 11:53 chenxiang
  2021-05-11  9:41 ` Robin Murphy
  2021-06-04 15:34 ` Joerg Roedel
  0 siblings, 2 replies; 3+ messages in thread
From: chenxiang @ 2021-05-10 11:53 UTC (permalink / raw)
  To: robin.murphy, will, joro; +Cc: iommu, linuxarm, linuxarm

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>
Reviewed-by: John Garry <john.garry@huawei.com>
---
 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(&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);
 
@@ -472,10 +472,13 @@ 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);
+	if (!iova) {
+		spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
+		return;
+	}
+	remove_iova(iovad, iova);
 	spin_unlock_irqrestore(&iovad->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(&iovad->iova_rbtree_lock, flags);
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock
  2021-05-10 11:53 [RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock chenxiang
@ 2021-05-11  9:41 ` Robin Murphy
  2021-06-04 15:34 ` Joerg Roedel
  1 sibling, 0 replies; 3+ messages in thread
From: Robin Murphy @ 2021-05-11  9:41 UTC (permalink / raw)
  To: chenxiang, will, joro; +Cc: iommu, linuxarm, linuxarm

On 2021-05-10 12:53, 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.

Acked-by: Robin Murphy <robin.murphy@arm.com>

> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> Reviewed-by: John Garry <john.garry@huawei.com>
> ---
>   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(&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);
>   
> @@ -472,10 +472,13 @@ 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);
> +	if (!iova) {
> +		spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
> +		return;
> +	}
> +	remove_iova(iovad, iova);
>   	spin_unlock_irqrestore(&iovad->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(&iovad->iova_rbtree_lock, flags);
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock
  2021-05-10 11:53 [RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock chenxiang
  2021-05-11  9:41 ` Robin Murphy
@ 2021-06-04 15:34 ` Joerg Roedel
  1 sibling, 0 replies; 3+ messages in thread
From: Joerg Roedel @ 2021-06-04 15:34 UTC (permalink / raw)
  To: chenxiang; +Cc: robin.murphy, linuxarm, linuxarm, iommu, will

On Mon, May 10, 2021 at 07:53:02PM +0800, 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>
> Reviewed-by: John Garry <john.garry@huawei.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-04 15:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10 11:53 [RESEND PATCH v3] iommu/iova: put free_iova_mem() outside of spinlock iova_rbtree_lock chenxiang
2021-05-11  9:41 ` Robin Murphy
2021-06-04 15:34 ` Joerg Roedel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.