Re: [Patch v2 2/3] iommu: optimize iova_magazine_free_pfns()

John Garry
2019-11-29T13:24:50Z

On 29/11/2019 00:48, Cong Wang wrote:
> If the maganize is empty, iova_magazine_free_pfns() should

magazine

> 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.
>
> Cc: Joerg Roedel <joro@8bytes.org>
> 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))

The only hot path we this call is
__iova_rcache_insert()->iova_magazine_free_pfns(mag_to_free) and
mag_to_free is full in this case, so I am sure how the additional check
helps, right?

Thanks,
John

> 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)
> {