Re: [PATCH 06/17] drm/i915: Move obj->dirty:1 to obj->flags

[Dave Gordon <david.s.gordon@intel.com>]

On 22/08/16 09:03, Chris Wilson wrote:
> The obj->dirty bit is a companion to the obj->active bits that were
> moved to the obj->flags bitmask. Since we also update this bit inside
> the i915_vma_move_to_active() hotpath, we can aide gcc by also moving
> the obj->dirty bit to obj->flags bitmask.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c        |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h            | 22 +++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_gem.c            | 20 ++++++++++----------
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 +--
>  drivers/gpu/drm/i915/i915_gem_userptr.c    |  6 +++---
>  drivers/gpu/drm/i915/i915_gpu_error.c      |  2 +-
>  drivers/gpu/drm/i915/intel_lrc.c           |  6 +++---
>  7 files changed, 40 insertions(+), 21 deletions(-)
>

[snip]

> @@ -3272,7 +3272,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
>  	if (write) {
>  		obj->base.read_domains = I915_GEM_DOMAIN_GTT;
>  		obj->base.write_domain = I915_GEM_DOMAIN_GTT;
> -		obj->dirty = 1;
> +		i915_gem_object_set_dirty(obj);

At this point the object is or may not be pinned. From our conversation 
back in April ...

>> What I particularly dislike about the current obj->dirty is that it is
>> strictly only valid inside a pin_pages/unpin_pages section. That isn't
>> clear from the API atm.
>> -Chris
>
> So, I tried replacing all instances of "obj->dirty = true" with my
> newfunction i915_gem_object_mark_dirty(), and added an assertion that
 > it's called only when (pages_pin_count > 0) - and found a failure.
>
> Stack is:
>     i915_gem_object_mark_dirty
>     i915_gem_object_set_to_gtt_domain
>     i915_gem_set_domain_ioctl
>
> So is i915_gem_object_set_to_gtt_domain() wrong? It's done a
> get_pages but no pin_pages.

... and the same issue is still there. I think I worked out a 
hypothetical scenario where this would lead to data corruption:

* set to GTT => mark dirty
* BO paged out => flushed to swap, marked clean
* BO paged in => still clean
* update contents => still clean?
* get paged out => not written out?

But can this actually happen?

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx