[Intel-gfx] [PATCH 1/2] drm/i915: reform i915_gem_verify_gtt() function

[Daniel Vetter]

On Fri, Jul 05, 2013 at 06:53:01PM +0800, Xiong Zhang wrote:
> dev_priv->mm.gtt_list has been removed. All the gtt are tracked in
> dev_pirv->mm.bound_list and dev_priv->mm.unbound_list
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>

Tbh I've only tried to use this once, and besides that it made everything
really slow it didn't really help. And since it's usually disabled it
tends to bit-rot pretty badly. So I see two options:

a) rip it out if it's useless, but I suspect you've used this to track
down the bug in the inactive shrinker. So that's probably not the right
thing.

b) rip out the compile-time disabling and replace it with a module option.
with static inline wrappers and unlikely the overhead of that should be
pretty much nothing, and we can change module options at runtime. On top
of that we then also need an igt which enables this and runs a few basic
gem tests (and then disables the option again ofc).

I think the current way of reapply the duct-tape every time it falls of
isn't sustainable.

Yours, Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c |   63 ++++++++++++++++++++++++---------------
>  1 file changed, 39 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3ea54c8..44d890b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3028,6 +3028,40 @@ static bool i915_gem_valid_gtt_space(struct drm_device *dev,
>  	return true;
>  }
>  
> +#if WATCH_GIT
> +static bool
> +i915_gem_object_valid_gtt(struct drm_i915_gem_object *obj)
> +{
> +	bool gtt_ok = true;
> +
> +	if (obj->gtt_space == NULL) {
> +		printk(KERN_ERR "object found on GTT list with no space reserved\n");
> +		gtt_ok = false;
> +	}
> +
> +	if (obj->cache_level != obj->gtt_space->color) {
> +		printk(KERN_ERR "object reserved space [%08lx, %08lx] with wrong color, cache_level=%x, color=%lx\n",
> +			    obj->gtt_space->start,
> +			    obj->gtt_space->start + obj->gtt_space->size,
> +			    obj->cache_level,
> +			    obj->gtt_space->color);
> +		gtt_ok = false;
> +	}
> +
> +	if (!i915_gem_valid_gtt_space(dev,
> +					    obj->gtt_space,
> +					    obj->cache_level)) {
> +		printk(KERN_ERR "invalid GTT space found at [%08lx, %08lx] - color=%x\n",
> +			    obj->gtt_space->start,
> +			    obj->gtt_space->start + obj->gtt_space->size,
> +			    obj->cache_level);
> +		gtt_ok = false
> +	}
> +
> +	return gtt_ok;
> +}
> +#endif
> +
>  static void i915_gem_verify_gtt(struct drm_device *dev)
>  {
>  #if WATCH_GTT
> @@ -3035,33 +3069,14 @@ static void i915_gem_verify_gtt(struct drm_device *dev)
>  	struct drm_i915_gem_object *obj;
>  	int err = 0;
>  
> -	list_for_each_entry(obj, &dev_priv->mm.gtt_list, global_list) {
> -		if (obj->gtt_space == NULL) {
> -			printk(KERN_ERR "object found on GTT list with no space reserved\n");
> -			err++;
> -			continue;
> -		}
> -
> -		if (obj->cache_level != obj->gtt_space->color) {
> -			printk(KERN_ERR "object reserved space [%08lx, %08lx] with wrong color, cache_level=%x, color=%lx\n",
> -			       i915_gem_obj_ggtt_offset(obj),
> -			       i915_gem_obj_ggtt_offset(obj) + i915_gem_obj_ggtt_size(obj),
> -			       obj->cache_level,
> -			       obj->gtt_space->color);
> +	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> +		if (!i915_gem_object_valid_gtt(obj))
>  			err++;
> -			continue;
> -		}
> +	}
>  
> -		if (!i915_gem_valid_gtt_space(dev,
> -					      obj->gtt_space,
> -					      obj->cache_level)) {
> -			printk(KERN_ERR "invalid GTT space found at [%08lx, %08lx] - color=%x\n",
> -			       i915_gem_obj_ggtt_offset(obj),
> -			       i915_gem_obj_ggtt_offset(obj) + i915_gem_obj_ggtt_size(obj),
> -			       obj->cache_level);
> +	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list) {
> +		if (!i915_gem_object_valid_gtt(obj))
>  			err++;
> -			continue;
> -		}
>  	}
>  
>  	WARN_ON(err);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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