diff mbox series

drm/i915: Reserve enough fence slot for i915_vma_unbind_async

Message ID 20221223092011.11657-1-nirmoy.das@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Reserve enough fence slot for i915_vma_unbind_async | expand

Commit Message

Nirmoy Das Dec. 23, 2022, 9:20 a.m. UTC
A nested dma_resv_reserve_fences(1) will not reserve slot from the
2nd call onwards and folowing dma_resv_add_fence() might hit the
"BUG_ON(fobj->num_fences >= fobj->max_fences)" check.

I915 hit above nested dma_resv case in ttm_bo_handle_move_mem() with
async unbind:

dma_resv_reserve_fences() from --> ttm_bo_handle_move_mem()
        dma_resv_reserve_fences() from --> i915_vma_unbind_async()
        dma_resv_add_fence() from --> i915_vma_unbind_async()
dma_resv_add_fence() from -->ttm_bo_move_accel_cleanup()

Resolve this by adding an extra fence in i915_vma_unbind_async().

Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/i915_vma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Matthew Auld Jan. 5, 2023, 9:56 a.m. UTC | #1
On Fri, 23 Dec 2022 at 09:20, Nirmoy Das <nirmoy.das@intel.com> wrote:
>
> A nested dma_resv_reserve_fences(1) will not reserve slot from the
> 2nd call onwards and folowing dma_resv_add_fence() might hit the
> "BUG_ON(fobj->num_fences >= fobj->max_fences)" check.
>
> I915 hit above nested dma_resv case in ttm_bo_handle_move_mem() with
> async unbind:
>
> dma_resv_reserve_fences() from --> ttm_bo_handle_move_mem()
>         dma_resv_reserve_fences() from --> i915_vma_unbind_async()
>         dma_resv_add_fence() from --> i915_vma_unbind_async()
> dma_resv_add_fence() from -->ttm_bo_move_accel_cleanup()
>
> Resolve this by adding an extra fence in i915_vma_unbind_async().
>
> Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Does this need a fixes tag?

Fixes: 2f6b90da9192 ("drm/i915: Use vma resources for async unbinding")
Cc: <stable@vger.kernel.org> # v5.18+

I can add when pushing.

> ---
>  drivers/gpu/drm/i915/i915_vma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 7d044888ac33..5ac4c1c2403c 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -2149,7 +2149,7 @@ int i915_vma_unbind_async(struct i915_vma *vma, bool trylock_vm)
>         if (!obj->mm.rsgt)
>                 return -EBUSY;
>
> -       err = dma_resv_reserve_fences(obj->base.resv, 1);
> +       err = dma_resv_reserve_fences(obj->base.resv, 2);
>         if (err)
>                 return -EBUSY;
>
> --
> 2.38.0
>
Nirmoy Das Jan. 5, 2023, 10:03 a.m. UTC | #2
On 1/5/2023 10:56 AM, Matthew Auld wrote:
> On Fri, 23 Dec 2022 at 09:20, Nirmoy Das <nirmoy.das@intel.com> wrote:
>> A nested dma_resv_reserve_fences(1) will not reserve slot from the
>> 2nd call onwards and folowing dma_resv_add_fence() might hit the
>> "BUG_ON(fobj->num_fences >= fobj->max_fences)" check.
>>
>> I915 hit above nested dma_resv case in ttm_bo_handle_move_mem() with
>> async unbind:
>>
>> dma_resv_reserve_fences() from --> ttm_bo_handle_move_mem()
>>          dma_resv_reserve_fences() from --> i915_vma_unbind_async()
>>          dma_resv_add_fence() from --> i915_vma_unbind_async()
>> dma_resv_add_fence() from -->ttm_bo_move_accel_cleanup()
>>
>> Resolve this by adding an extra fence in i915_vma_unbind_async().
>>
>> Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
> Does this need a fixes tag?
>
> Fixes: 2f6b90da9192 ("drm/i915: Use vma resources for async unbinding")
> Cc: <stable@vger.kernel.org> # v5.18+
>
> I can add when pushing.


Yes, please add that.


Thanks,

Nirmoy

>
>> ---
>>   drivers/gpu/drm/i915/i915_vma.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>> index 7d044888ac33..5ac4c1c2403c 100644
>> --- a/drivers/gpu/drm/i915/i915_vma.c
>> +++ b/drivers/gpu/drm/i915/i915_vma.c
>> @@ -2149,7 +2149,7 @@ int i915_vma_unbind_async(struct i915_vma *vma, bool trylock_vm)
>>          if (!obj->mm.rsgt)
>>                  return -EBUSY;
>>
>> -       err = dma_resv_reserve_fences(obj->base.resv, 1);
>> +       err = dma_resv_reserve_fences(obj->base.resv, 2);
>>          if (err)
>>                  return -EBUSY;
>>
>> --
>> 2.38.0
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 7d044888ac33..5ac4c1c2403c 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -2149,7 +2149,7 @@  int i915_vma_unbind_async(struct i915_vma *vma, bool trylock_vm)
 	if (!obj->mm.rsgt)
 		return -EBUSY;
 
-	err = dma_resv_reserve_fences(obj->base.resv, 1);
+	err = dma_resv_reserve_fences(obj->base.resv, 2);
 	if (err)
 		return -EBUSY;