Re: [PATCH 06/22] drm/i915: introduce vm set_pages/clear_pages

Chris Wilson
2017-08-15T18:46:08Z

Quoting Matthew Auld (2017-08-15 19:11:59)
> @@ -517,6 +527,8 @@ i915_vma_remove(struct i915_vma *vma)
> GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
> GEM_BUG_ON(vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND));
>
> + vma->vm->clear_pages(vma);
> +
> drm_mm_remove_node(&vma->node);
> list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
>
> @@ -568,10 +580,8 @@ int __i915_vma_do_pin(struct i915_vma *vma,
> return 0;
>
> err_remove:
> - if ((bound & I915_VMA_BIND_MASK) == 0) {
> - GEM_BUG_ON(vma->pages);

Hmm. So we've run i915_vma_insert and so vma->pages will be set.
However, our invariant is that if we have an unbound, vma->pages will be
NULL. So we still need to clear the pages on error here.

That is now in i915_vma_remove itself (that took a few scans to spot,
even though it is right above!), I think that will be a good addition to
have a GEM_BUG_ON(vma->pages) following i915_vma_remove.

> + if ((bound & I915_VMA_BIND_MASK) == 0)
> i915_vma_remove(vma);
> - }