drm/i915: Always mark the writer as also a read for busy ioctl

One of the few guarantees we want the busy ioctl to provide is that the
reported busy writer is included in the set of busy read engines. This
should be provided by the ordering of setting and retiring the active
trackers, but we can do better by explicitly setting the busy read
engine flag for the last writer.

v2: More comments inside __busy_write_id() to explain why both fields
are set.

Fixes: 3fdc13c7a3cb ("drm/i915: Remove (struct_mutex) locking for busy-ioctl")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470762505-12799-1-git-send-email-chris@chris-wilson.co.uk
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 373136f57d7b3e1f5747cd0e3755e25d72ea5ef5..3fb5749743431e3cad739d6e0c15c75c7fdcb658 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3748,7 +3748,15 @@ static __always_inline unsigned int __busy_read_flag(unsigned int id)
 
 static __always_inline unsigned int __busy_write_id(unsigned int id)
 {
-       return id;
+       /* The uABI guarantees an active writer is also amongst the read
+        * engines. This would be true if we accessed the activity tracking
+        * under the lock, but as we perform the lookup of the object and
+        * its activity locklessly we can not guarantee that the last_write
+        * being active implies that we have set the same engine flag from
+        * last_read - hence we always set both read and write busy for
+        * last_write.
+        */
+       return id | __busy_read_flag(id);
 }
 
 static __always_inline unsigned int
@@ -3857,9 +3865,11 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
                        args->busy |= busy_check_reader(&obj->last_read[idx]);
 
                /* For ABI sanity, we only care that the write engine is in
-                * the set of read engines. This is ensured by the ordering
-                * of setting last_read/last_write in i915_vma_move_to_active,
-                * and then in reverse in retire.
+                * the set of read engines. This should be ensured by the
+                * ordering of setting last_read/last_write in
+                * i915_vma_move_to_active(), and then in reverse in retire.
+                * However, for good measure, we always report the last_write
+                * request as a busy read as well as being a busy write.
                 *
                 * We don't care that the set of active read/write engines
                 * may change during construction of the result, as it is