drm/i915: Remove leftover vma->obj->pages_pin_count on insert/remove
diff mbox series

Message ID 20191015100155.10376-1-chris@chris-wilson.co.uk
State New
Headers show
Series
  • drm/i915: Remove leftover vma->obj->pages_pin_count on insert/remove
Related show

Commit Message

Chris Wilson Oct. 15, 2019, 10:01 a.m. UTC
We now do the page pin count upfront in vma_get_pages/vma_put_pages, so
that we do the allocations before we enter the vm->mutex. Then our vma
page references are tracking in vma->pages_count and the extra
obj->pages_pin_count being performed in i915_vma_insert/i915_vma_remove
is redundant, and worse throws off the shrinker's logic on when it can
free an object by unbinding it.

Reported-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/i915_vma.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Matthew Auld Oct. 15, 2019, 10:22 a.m. UTC | #1
On 15/10/2019 11:01, Chris Wilson wrote:
> We now do the page pin count upfront in vma_get_pages/vma_put_pages, so
> that we do the allocations before we enter the vm->mutex. Then our vma
> page references are tracking in vma->pages_count and the extra
> obj->pages_pin_count being performed in i915_vma_insert/i915_vma_remove
> is redundant, and worse throws off the shrinker's logic on when it can
> free an object by unbinding it.
> 
Could you also add Reported-by: Daniele Ceraolo Spurio 
<daniele.ceraolospurio@intel.com>

> Reported-by: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 7d936254bde7..e90c4d0af8fd 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -703,7 +703,6 @@  i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 	list_add_tail(&vma->vm_link, &vma->vm->bound_list);
 
 	if (vma->obj) {
-		atomic_inc(&vma->obj->mm.pages_pin_count);
 		atomic_inc(&vma->obj->bind_count);
 		assert_bind_count(vma->obj);
 	}
@@ -726,14 +725,12 @@  i915_vma_remove(struct i915_vma *vma)
 	if (vma->obj) {
 		struct drm_i915_gem_object *obj = vma->obj;
 
-		atomic_dec(&obj->bind_count);
-
 		/*
 		 * And finally now the object is completely decoupled from this
 		 * vma, we can drop its hold on the backing storage and allow
 		 * it to be reaped by the shrinker.
 		 */
-		i915_gem_object_unpin_pages(obj);
+		atomic_dec(&obj->bind_count);
 		assert_bind_count(obj);
 	}