[PATCH] Fix for the i915_vma_unbind() fix.

[PATCH] Fix for the i915_vma_unbind() fix.

[Chris Wilson]

We need to move the bogus warn out of the way and only do the
vma->vma_link decoupling before destroying the node.

Fixes gem_evict_everything, gem_evict_alignment

Squash in with Daniel's fixup to the fix -- I expect that he has already
done so...

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

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5788e9d..744f9a6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2725,6 +2725,9 @@ int i915_vma_unbind(struct i915_vma *vma)
 	if (list_empty(&vma->vma_link))
 		return 0;
 
+	 /* NB: Until we have real VMAs there will only ever be one */
+	WARN_ON(!list_is_singular(&obj->vma_list));
+
 	if (!drm_mm_node_allocated(&vma->node))
 		goto destroy;
 
@@ -2771,8 +2774,7 @@ destroy:
 
 	/* Since the unbound list is global, only move to that list if
 	 * no more VMAs exist.
-	 * NB: Until we have real VMAs there will only ever be one */
-	WARN_ON(!list_empty(&obj->vma_list));
+	 */
 	if (list_empty(&obj->vma_list))
 		list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list);
 
@@ -4279,12 +4281,12 @@ i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
 void i915_gem_vma_destroy(struct i915_vma *vma)
 {
 	WARN_ON(vma->node.allocated);
-	list_del(&vma->vma_link);
 
 	/* Keep the vma as a placeholder in the execbuffer reservation lists */
 	if (!list_empty(&vma->exec_list))
 		return;
 
+	list_del(&vma->vma_link);
 	kfree(vma);
 }
 
-- 
1.8.4.rc3

Re: [PATCH] Fix for the i915_vma_unbind() fix.

[Daniel Vetter]

On Fri, Aug 23, 2013 at 10:07:16AM +0100, Chris Wilson wrote:
> We need to move the bogus warn out of the way and only do the
> vma->vma_link decoupling before destroying the node.
> 
> Fixes gem_evict_everything, gem_evict_alignment
> 
> Squash in with Daniel's fixup to the fix -- I expect that he has already
> done so...
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Hm, I don't see the functional difference to the resend patch I've posted
last night... still mail woes?

And QA reported that this approach is still not good enough to make :(
Since I don't want to jeopardize other feature work I'll take out the vma
execbuf patch now so that I can cut a new -testing. We can resume the
head-banging next week ;-)
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5788e9d..744f9a6 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2725,6 +2725,9 @@ int i915_vma_unbind(struct i915_vma *vma)
>  	if (list_empty(&vma->vma_link))
>  		return 0;
>  
> +	 /* NB: Until we have real VMAs there will only ever be one */
> +	WARN_ON(!list_is_singular(&obj->vma_list));
> +
>  	if (!drm_mm_node_allocated(&vma->node))
>  		goto destroy;
>  
> @@ -2771,8 +2774,7 @@ destroy:
>  
>  	/* Since the unbound list is global, only move to that list if
>  	 * no more VMAs exist.
> -	 * NB: Until we have real VMAs there will only ever be one */
> -	WARN_ON(!list_empty(&obj->vma_list));
> +	 */
>  	if (list_empty(&obj->vma_list))
>  		list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list);
>  
> @@ -4279,12 +4281,12 @@ i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
>  void i915_gem_vma_destroy(struct i915_vma *vma)
>  {
>  	WARN_ON(vma->node.allocated);
> -	list_del(&vma->vma_link);
>  
>  	/* Keep the vma as a placeholder in the execbuffer reservation lists */
>  	if (!list_empty(&vma->exec_list))
>  		return;
>  
> +	list_del(&vma->vma_link);
>  	kfree(vma);
>  }
>  
> -- 
> 1.8.4.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch