[Intel-gfx] [PATCH 06/16] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock

[Joonas Lahtinen]

On ke, 2017-07-26 at 14:26 +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.

Please mention the global_link -> mm.link rename too.

> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

<SNIP>

> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1443,6 +1443,8 @@ struct i915_gem_mm {
>  	 * always the inner lock when overlapping with struct_mutex. */
>  	struct mutex stolen_lock;

Please comment what this lock achieves, here too. Maybe cross-link with
the other doc?

> +	spinlock_t obj_lock;
> +
>  	/** List of all objects in gtt_space. Used to restore gtt
>  	 * mappings on resume */

<SNIP>

> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -92,9 +92,6 @@ static bool swap_available(void)
>  
>  static bool can_release_pages(struct drm_i915_gem_object *obj)
>  {
> -	if (!i915_gem_object_has_pages(obj))
> -		return false;

So you think the inaccuracies we get for being lockless don't matter
compared to better forward progress?

Maybe the cons should be documented in the commit message to help in
bisecting.

> @@ -149,8 +149,6 @@ static int igt_overcommit(void *arg)
>  		goto cleanup;
>  	}
>  
> -	list_move(&obj->global_link, &i915->mm.unbound_list);

Accidentally dropped line?

> -
>  	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
>  	if (!IS_ERR(vma) || PTR_ERR(vma) != -ENOSPC) {
>  		pr_err("Failed to evict+insert, i915_gem_object_ggtt_pin returned err=%d\n", (int)PTR_ERR(vma));
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation