drm/i915: Claim vma while under closed_lock in i915_vma_parked()
diff mbox series

Message ID 20191205214159.829727-1-chris@chris-wilson.co.uk
State New
Headers show
Series
  • drm/i915: Claim vma while under closed_lock in i915_vma_parked()
Related show

Commit Message

Chris Wilson Dec. 5, 2019, 9:41 p.m. UTC
Remove the vma we wish to destroy from the gt->closed_list to avoid
having two i915_vma_parked() try and free it.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_vma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Tvrtko Ursulin Dec. 6, 2019, 11:12 a.m. UTC | #1
On 05/12/2019 21:41, Chris Wilson wrote:
> Remove the vma we wish to destroy from the gt->closed_list to avoid
> having two i915_vma_parked() try and free it.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_vma.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 6db35b9dead8..9ca6664c190c 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -1053,7 +1053,9 @@ void i915_vma_parked(struct intel_gt *gt)
>   		if (!kref_get_unless_zero(&obj->base.refcount))
>   			continue;
>   
> -		if (!i915_vm_tryopen(vm)) {
> +		if (i915_vm_tryopen(vm)) {
> +			list_del_init(&vma->closed_link);
> +		} else {
>   			i915_gem_object_put(obj);
>   			obj = NULL;
>   		}
> 

Fixes 2850748ef8763 I think. Until then there was a list_del_init in here.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
Chris Wilson Dec. 6, 2019, 11:16 a.m. UTC | #2
Quoting Tvrtko Ursulin (2019-12-06 11:12:42)
> 
> On 05/12/2019 21:41, Chris Wilson wrote:
> > Remove the vma we wish to destroy from the gt->closed_list to avoid
> > having two i915_vma_parked() try and free it.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/i915_vma.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> > index 6db35b9dead8..9ca6664c190c 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.c
> > +++ b/drivers/gpu/drm/i915/i915_vma.c
> > @@ -1053,7 +1053,9 @@ void i915_vma_parked(struct intel_gt *gt)
> >               if (!kref_get_unless_zero(&obj->base.refcount))
> >                       continue;
> >   
> > -             if (!i915_vm_tryopen(vm)) {
> > +             if (i915_vm_tryopen(vm)) {
> > +                     list_del_init(&vma->closed_link);
> > +             } else {
> >                       i915_gem_object_put(obj);
> >                       obj = NULL;
> >               }
> > 
> 
> Fixes 2850748ef8763 I think. Until then there was a list_del_init in here.

No. Until aa5e4453dc05 ("drm/i915/gem: Try to flush pending unbind
events") there as only a single caller (__gt_park) so we knew there
would only be one iterator. Still, either way, there's going to be a
massive fixup patch for Joonas to apply.
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 6db35b9dead8..9ca6664c190c 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1053,7 +1053,9 @@  void i915_vma_parked(struct intel_gt *gt)
 		if (!kref_get_unless_zero(&obj->base.refcount))
 			continue;
 
-		if (!i915_vm_tryopen(vm)) {
+		if (i915_vm_tryopen(vm)) {
+			list_del_init(&vma->closed_link);
+		} else {
 			i915_gem_object_put(obj);
 			obj = NULL;
 		}