[PATCH] media: staging: imgu: do not hold spinlock during freeing mmu page table

[PATCH] media: staging: imgu: do not hold spinlock during freeing mmu page table

[Bingbu Cao]

The spinlock should not be hold during ImgU page alloc and free, the
irq should be enabled during memory cache flush - cpa_flush(). The
spinlock should be released before freeing pages table.

Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 drivers/staging/media/ipu3/ipu3-mmu.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-mmu.c b/drivers/staging/media/ipu3/ipu3-mmu.c
index 5f3ff964f3e7..cb9bf5fb29a5 100644
--- a/drivers/staging/media/ipu3/ipu3-mmu.c
+++ b/drivers/staging/media/ipu3/ipu3-mmu.c
@@ -174,8 +174,10 @@ static u32 *imgu_mmu_get_l2pt(struct imgu_mmu *mmu, u32 l1pt_idx)
 	spin_lock_irqsave(&mmu->lock, flags);
 
 	l2pt = mmu->l2pts[l1pt_idx];
-	if (l2pt)
-		goto done;
+	if (l2pt) {
+		spin_unlock_irqrestore(&mmu->lock, flags);
+		return l2pt;
+	}
 
 	spin_unlock_irqrestore(&mmu->lock, flags);
 
@@ -190,8 +192,9 @@ static u32 *imgu_mmu_get_l2pt(struct imgu_mmu *mmu, u32 l1pt_idx)
 
 	l2pt = mmu->l2pts[l1pt_idx];
 	if (l2pt) {
+		spin_unlock_irqrestore(&mmu->lock, flags);
 		imgu_mmu_free_page_table(new_l2pt);
-		goto done;
+		return l2pt;
 	}
 
 	l2pt = new_l2pt;
@@ -200,7 +203,6 @@ static u32 *imgu_mmu_get_l2pt(struct imgu_mmu *mmu, u32 l1pt_idx)
 	pteval = IPU3_ADDR2PTE(virt_to_phys(new_l2pt));
 	mmu->l1pt[l1pt_idx] = pteval;
 
-done:
 	spin_unlock_irqrestore(&mmu->lock, flags);
 	return l2pt;
 }
-- 
2.7.4

Re: [PATCH] media: staging: imgu: do not hold spinlock during freeing mmu page table

[Tomasz Figa]

Hi Bingbu,

On Mon, Mar 23, 2020 at 3:15 PM Bingbu Cao <bingbu.cao@intel.com> wrote:
>
> The spinlock should not be hold during ImgU page alloc and free, the
> irq should be enabled during memory cache flush - cpa_flush(). The
> spinlock should be released before freeing pages table.

Thanks for the patch! Would be good to explain why it is so.

Otherwise feel free to add

Reviewed-by: Tomasz Figa <tfiga@chromium.org>

Best regards,
Tomasz

>
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> ---
>  drivers/staging/media/ipu3/ipu3-mmu.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/ipu3/ipu3-mmu.c b/drivers/staging/media/ipu3/ipu3-mmu.c
> index 5f3ff964f3e7..cb9bf5fb29a5 100644
> --- a/drivers/staging/media/ipu3/ipu3-mmu.c
> +++ b/drivers/staging/media/ipu3/ipu3-mmu.c
> @@ -174,8 +174,10 @@ static u32 *imgu_mmu_get_l2pt(struct imgu_mmu *mmu, u32 l1pt_idx)
>         spin_lock_irqsave(&mmu->lock, flags);
>
>         l2pt = mmu->l2pts[l1pt_idx];
> -       if (l2pt)
> -               goto done;
> +       if (l2pt) {
> +               spin_unlock_irqrestore(&mmu->lock, flags);
> +               return l2pt;
> +       }
>
>         spin_unlock_irqrestore(&mmu->lock, flags);
>
> @@ -190,8 +192,9 @@ static u32 *imgu_mmu_get_l2pt(struct imgu_mmu *mmu, u32 l1pt_idx)
>
>         l2pt = mmu->l2pts[l1pt_idx];
>         if (l2pt) {
> +               spin_unlock_irqrestore(&mmu->lock, flags);
>                 imgu_mmu_free_page_table(new_l2pt);
> -               goto done;
> +               return l2pt;
>         }
>
>         l2pt = new_l2pt;
> @@ -200,7 +203,6 @@ static u32 *imgu_mmu_get_l2pt(struct imgu_mmu *mmu, u32 l1pt_idx)
>         pteval = IPU3_ADDR2PTE(virt_to_phys(new_l2pt));
>         mmu->l1pt[l1pt_idx] = pteval;
>
> -done:
>         spin_unlock_irqrestore(&mmu->lock, flags);
>         return l2pt;
>  }
> --
> 2.7.4
>