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

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

[Chris Wilson]

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.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2bb9ef91a243..c3652e09d272 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3738,7 +3738,7 @@ static __always_inline unsigned int __busy_read_flag(unsigned int id)
 
 static __always_inline unsigned int __busy_write_id(unsigned int id)
 {
-	return id;
+	return id | __busy_read_flag(id);
 }
 
 static __always_inline unsigned int
@@ -3847,9 +3847,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
-- 
2.8.1

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

✓ Ro.CI.BAT: success for drm/i915: Always mark the writer as also a read for busy ioctl

[Patchwork]

== Series Details ==

Series: drm/i915: Always mark the writer as also a read for busy ioctl
URL   : https://patchwork.freedesktop.org/series/10830/
State : success

== Summary ==

Series 10830v1 drm/i915: Always mark the writer as also a read for busy ioctl
http://patchwork.freedesktop.org/api/1.0/series/10830/revisions/1/mbox


fi-hsw-i7-4770k  total:107  pass:91   dwarn:0   dfail:0   fail:0   skip:15 
fi-kbl-qkkr      total:107  pass:82   dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-i5-6260u  total:107  pass:98   dwarn:0   dfail:0   fail:0   skip:8  
fi-skl-i7-6700k  total:107  pass:84   dwarn:0   dfail:0   fail:0   skip:22 
fi-snb-i7-2600   total:107  pass:77   dwarn:0   dfail:0   fail:0   skip:29 
ro-bdw-i5-5250u  total:107  pass:97   dwarn:0   dfail:0   fail:0   skip:9  
ro-bdw-i7-5557U  total:107  pass:97   dwarn:0   dfail:0   fail:0   skip:9  
ro-bdw-i7-5600u  total:107  pass:79   dwarn:0   dfail:0   fail:0   skip:27 
ro-bsw-n3050     total:240  pass:194  dwarn:0   dfail:0   fail:4   skip:42 
ro-byt-n2820     total:240  pass:197  dwarn:0   dfail:0   fail:3   skip:40 
ro-hsw-i3-4010u  total:107  pass:87   dwarn:0   dfail:0   fail:0   skip:19 
ro-ilk1-i5-650   total:235  pass:173  dwarn:0   dfail:0   fail:2   skip:60 
ro-ivb-i7-3770   total:107  pass:80   dwarn:0   dfail:0   fail:0   skip:26 
ro-skl3-i5-6260u total:107  pass:98   dwarn:0   dfail:0   fail:0   skip:8  

Results at /archive/results/CI_IGT_test/RO_Patchwork_1775/

e220d47 drm-intel-nightly: 2016y-08m-09d-09h-18m-12s UTC integration manifest
636fb6e drm/i915: Always mark the writer as also a read for busy ioctl

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

[PATCH v2] drm/i915: Always mark the writer as also a read for busy ioctl

[Chris Wilson]

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
---
 drivers/gpu/drm/i915/i915_gem.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d71fa9a93afa..a7d118e6c8e8 100644
--- 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
-- 
2.8.1

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

✗ Ro.CI.BAT: failure for drm/i915: Always mark the writer as also a read for busy ioctl (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Always mark the writer as also a read for busy ioctl (rev2)
URL   : https://patchwork.freedesktop.org/series/10830/
State : failure

== Summary ==

Series 10830v2 drm/i915: Always mark the writer as also a read for busy ioctl
http://patchwork.freedesktop.org/api/1.0/series/10830/revisions/2/mbox

Test drv_module_reload_basic:
                pass       -> SKIP       (fi-skl-i5-6260u)
Test kms_cursor_legacy:
        Subgroup basic-flip-vs-cursor-legacy:
                pass       -> FAIL       (ro-byt-n2820)
                pass       -> FAIL       (ro-skl3-i5-6260u)
        Subgroup basic-flip-vs-cursor-varying-size:
                pass       -> FAIL       (ro-bdw-i5-5250u)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c-frame-sequence:
                fail       -> PASS       (ro-ivb-i7-3770)
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (fi-hsw-i7-4770k)
        Subgroup suspend-read-crc-pipe-b:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (ro-bdw-i7-5600u)
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-hsw-i7-4770k  total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-qkkr      total:244  pass:185  dwarn:29  dfail:0   fail:3   skip:27 
fi-skl-i5-6260u  total:244  pass:223  dwarn:4   dfail:0   fail:2   skip:15 
fi-snb-i7-2600   total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
ro-bdw-i5-5250u  total:240  pass:219  dwarn:2   dfail:0   fail:1   skip:18 
ro-bdw-i7-5557U  total:240  pass:220  dwarn:1   dfail:0   fail:0   skip:19 
ro-bdw-i7-5600u  total:240  pass:206  dwarn:0   dfail:0   fail:2   skip:32 
ro-bsw-n3050     total:240  pass:194  dwarn:0   dfail:0   fail:4   skip:42 
ro-byt-n2820     total:240  pass:197  dwarn:0   dfail:0   fail:3   skip:40 
ro-hsw-i3-4010u  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26 
ro-hsw-i7-4770r  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26 
ro-ilk1-i5-650   total:235  pass:173  dwarn:0   dfail:0   fail:2   skip:60 
ro-ivb-i7-3770   total:240  pass:205  dwarn:0   dfail:0   fail:0   skip:35 
ro-ivb2-i7-3770  total:240  pass:209  dwarn:0   dfail:0   fail:0   skip:31 
ro-skl3-i5-6260u total:240  pass:223  dwarn:0   dfail:0   fail:3   skip:14 
fi-skl-i7-6700k failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1802/

d0e3a4b drm-intel-nightly: 2016y-08m-09d-20h-18m-38s UTC integration manifest
30752a4 drm/i915: Always mark the writer as also a read for busy ioctl

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

Re: [PATCH v2] drm/i915: Always mark the writer as also a read for busy ioctl

[Chris Wilson]

On Tue, Aug 09, 2016 at 06:08:25PM +0100, Chris Wilson wrote:
> 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

From irc, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx