Re: [PATCH 3/6] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock

From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/6] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
Date: Wed, 16 Aug 2017 16:13:43 +0300	[thread overview]
Message-ID: <1502889223.6625.2.camel@linux.intel.com> (raw)
In-Reply-To: <20170812115122.10854-3-chris@chris-wilson.co.uk>

On Sat, 2017-08-12 at 12:51 +0100, Chris Wilson wrote:
> Remove the struct_mutex requirement around dev_priv->mm.bound_list and
> dev_priv->mm.unbound_list by giving it its own spinlock. This reduces
> one more requirement for struct_mutex and in the process gives us
> slightly more accurate unbound_list tracking, which should improve the
> shrinker - but the drawback is that we drop the retirement before
> counting so i915_gem_object_is_active() may be stale and lead us to
> underestimate the number of objects that may be shrunk (see commit
> bed50aea61df ("drm/i915/shrinker: Flush active on objects before
> counting")).
> 
> v2: Crosslink the spinlock to the lists it protects, and btw this
> changes s/obj->global_link/obj->mm.link/
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1536,9 +1536,12 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
>  		list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
>  	}
>  
> +	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));

Lift this precondition to the beginning of this func, there's no need
for the loop to be in front.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx