* [PATCH 1/2] drm/i915: Use ktime on wait_for
@ 2018-04-20 13:45 Mika Kuoppala
2018-04-20 13:45 ` [PATCH 2/2] drm/i915: Add compiler barrier to wait_for Mika Kuoppala
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Mika Kuoppala @ 2018-04-20 13:45 UTC (permalink / raw)
To: intel-gfx
Cc: Mika Kuoppala, Imre Deak, Chris Wilson, Ville Syrjälä, stable
We use jiffies to determine when wait expires. However
Imre did find out that jiffies can and will do a >1
increments on certain situations [1]. When this happens
in a wait_for loop, we return timeout errorneously
much earlier than what the real wallclock would say.
We can't afford our waits to timeout prematurely.
Discard jiffies and change to ktime to detect timeouts.
v2: added bugzilla entry (Imre), added stable (Chris)
Reported-by: Imre Deak <imre.deak@intel.com>
References: https://lkml.org/lkml/2018/4/18/798 [1]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105771
Cc: Imre Deak <imre.deak@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_drv.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8b20824e806e..ac7565220aa3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -49,12 +49,12 @@
* check the condition before the timeout.
*/
#define __wait_for(OP, COND, US, Wmin, Wmax) ({ \
- unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1; \
+ const ktime_t end__ = ktime_add_ns(ktime_get_raw(), 1000ll * (US)); \
long wait__ = (Wmin); /* recommended min for usleep is 10 us */ \
int ret__; \
might_sleep(); \
for (;;) { \
- bool expired__ = time_after(jiffies, timeout__); \
+ const bool expired__ = ktime_after(ktime_get_raw(), end__); \
OP; \
if (COND) { \
ret__ = 0; \
--
2.14.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] drm/i915: Add compiler barrier to wait_for
2018-04-20 13:45 [PATCH 1/2] drm/i915: Use ktime on wait_for Mika Kuoppala
@ 2018-04-20 13:45 ` Mika Kuoppala
2018-04-20 13:52 ` Chris Wilson
2018-04-20 16:10 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Use ktime on wait_for Patchwork
2018-04-20 16:55 ` ✗ Fi.CI.IGT: failure " Patchwork
2 siblings, 1 reply; 7+ messages in thread
From: Mika Kuoppala @ 2018-04-20 13:45 UTC (permalink / raw)
To: intel-gfx
We need to be careful to not let compiler evaluate
the expiration and the operation on it's terms.
Document and enforce that COND will be evaluated
before checking timeout expiration.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ac7565220aa3..4dc346716bb4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -56,6 +56,8 @@
for (;;) { \
const bool expired__ = ktime_after(ktime_get_raw(), end__); \
OP; \
+ /* Guarantee COND check prior to timeout */ \
+ barrier(); \
if (COND) { \
ret__ = 0; \
break; \
@@ -96,6 +98,8 @@
u64 now = local_clock(); \
if (!(ATOMIC)) \
preempt_enable(); \
+ /* Guarantee COND check prior to timeout */ \
+ barrier(); \
if (COND) { \
ret = 0; \
break; \
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] drm/i915: Add compiler barrier to wait_for
2018-04-20 13:45 ` [PATCH 2/2] drm/i915: Add compiler barrier to wait_for Mika Kuoppala
@ 2018-04-20 13:52 ` Chris Wilson
0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2018-04-20 13:52 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2018-04-20 14:45:50)
> We need to be careful to not let compiler evaluate
> the expiration and the operation on it's terms.
>
> Document and enforce that COND will be evaluated
> before checking timeout expiration.
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ac7565220aa3..4dc346716bb4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -56,6 +56,8 @@
> for (;;) { \
> const bool expired__ = ktime_after(ktime_get_raw(), end__); \
> OP; \
> + /* Guarantee COND check prior to timeout */ \
> + barrier(); \
Which side of OP is debatable, since OP is caller supplied and our
constraint is targeted at expired__. However, it is likely to be more
useful between OP/COND, so I can see some advantage there.
As Mika noted, moving to a function call should mean that the compiler
has less freedom to reorder/re-evaluate but doesn't completely prevent
it. As such being clear about the order of operations here is just as
important for the reader. There is a long history of gotchas here, and
this patch set continues the trend :)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Use ktime on wait_for
2018-04-20 13:45 [PATCH 1/2] drm/i915: Use ktime on wait_for Mika Kuoppala
2018-04-20 13:45 ` [PATCH 2/2] drm/i915: Add compiler barrier to wait_for Mika Kuoppala
@ 2018-04-20 16:10 ` Patchwork
2018-04-20 16:55 ` ✗ Fi.CI.IGT: failure " Patchwork
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-04-20 16:10 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Use ktime on wait_for
URL : https://patchwork.freedesktop.org/series/42035/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4072 -> Patchwork_8764 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42035/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_8764 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_module_reload@basic-no-display:
fi-cnl-psr: NOTRUN -> DMESG-WARN (fdo#105395) +2
igt@gem_exec_suspend@basic-s3:
fi-ivb-3520m: PASS -> DMESG-WARN (fdo#106084)
==== Possible fixes ====
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-ivb-3520m: DMESG-WARN (fdo#106084) -> PASS
fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395
fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
== Participating hosts (33 -> 33) ==
Additional (2): fi-kbl-7560u fi-cnl-psr
Missing (2): fi-ilk-m540 fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4072 -> Patchwork_8764
CI_DRM_4072: b35e59e5c6a9cae11d5183d2bf9c5c99ceedbc7c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4442: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8764: ae064ac61a135c5543fec9fe7a171f68bfc1a28e @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4442: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit
== Linux commits ==
ae064ac61a13 drm/i915: Add compiler barrier to wait_for
d5e6b76f3037 drm/i915: Use ktime on wait_for
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8764/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Use ktime on wait_for
2018-04-20 13:45 [PATCH 1/2] drm/i915: Use ktime on wait_for Mika Kuoppala
2018-04-20 13:45 ` [PATCH 2/2] drm/i915: Add compiler barrier to wait_for Mika Kuoppala
2018-04-20 16:10 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Use ktime on wait_for Patchwork
@ 2018-04-20 16:55 ` Patchwork
2018-04-24 12:22 ` Martin Peres
2 siblings, 1 reply; 7+ messages in thread
From: Patchwork @ 2018-04-20 16:55 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Use ktime on wait_for
URL : https://patchwork.freedesktop.org/series/42035/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4072_full -> Patchwork_8764_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_8764_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8764_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42035/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8764_full:
=== IGT changes ===
==== Possible regressions ====
igt@gem_exec_suspend@basic-s3-devices:
shard-hsw: PASS -> DMESG-WARN
==== Warnings ====
igt@gem_mocs_settings@mocs-rc6-render:
shard-kbl: SKIP -> PASS
igt@kms_vblank@pipe-b-wait-forked-busy-hang:
shard-glk: SKIP -> PASS +85
igt@pm_rpm@modeset-non-lpsp:
shard-glk: PASS -> SKIP +58
== Known issues ==
Here are the changes found in Patchwork_8764_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ppgtt@blt-vs-render-ctxn:
shard-kbl: PASS -> INCOMPLETE (fdo#103665, fdo#106023)
igt@kms_flip@flip-vs-wf_vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#100368) +1
igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt:
shard-kbl: PASS -> DMESG-WARN (fdo#105602, fdo#103558) +38
==== Possible fixes ====
igt@kms_flip@dpms-vs-vblank-race-interruptible:
shard-glk: FAIL (fdo#103060) -> SKIP
igt@kms_flip@plain-flip-fb-recreate:
shard-hsw: FAIL (fdo#100368) -> PASS
igt@kms_flip@plain-flip-ts-check-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS +1
igt@kms_setmode@basic:
shard-glk: FAIL (fdo#99912) -> PASS
igt@perf@blocking:
shard-hsw: FAIL (fdo#102252) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 5) ==
Missing (1): shard-glkb
== Build changes ==
* Linux: CI_DRM_4072 -> Patchwork_8764
CI_DRM_4072: b35e59e5c6a9cae11d5183d2bf9c5c99ceedbc7c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4442: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8764: ae064ac61a135c5543fec9fe7a171f68bfc1a28e @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4442: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8764/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-04-24 12:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-20 13:45 [PATCH 1/2] drm/i915: Use ktime on wait_for Mika Kuoppala
2018-04-20 13:45 ` [PATCH 2/2] drm/i915: Add compiler barrier to wait_for Mika Kuoppala
2018-04-20 13:52 ` Chris Wilson
2018-04-20 16:10 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Use ktime on wait_for Patchwork
2018-04-20 16:55 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-04-24 12:22 ` Martin Peres
2018-04-24 12:45 ` Mika Kuoppala
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.