* [PATH i-g-t 1/2] intel: Be consistent with test results on simulation
@ 2018-09-12 9:33 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-09-12 9:33 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Tests which call intel_require_memory currently always skip on simulation,
unless they fail first due insufficient memory. This can create different
outcomes depending on the simulation environment so move the simulation
skip to the start of the function for 100% consistency in results.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
---
lib/intel_os.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/intel_os.c b/lib/intel_os.c
index 29a27272e782..e1e31e23069b 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -375,6 +375,8 @@ void intel_require_memory(uint64_t count, uint64_t size, unsigned mode)
uint64_t required, total;
bool sufficient_memory;
+ igt_skip_on_simulation();
+
sufficient_memory = __intel_check_memory(count, size, mode,
&required, &total);
if (!sufficient_memory) {
@@ -406,8 +408,6 @@ void intel_require_memory(uint64_t count, uint64_t size, unsigned mode)
mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
mode & CHECK_SWAP ? " + swap": "",
(long long)vfs_file_max());
-
- igt_skip_on_simulation();
}
void intel_purge_vm_caches(int drm_fd)
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATH i-g-t 1/2] intel: Be consistent with test results on simulation
@ 2018-09-12 9:33 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-09-12 9:33 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Tests which call intel_require_memory currently always skip on simulation,
unless they fail first due insufficient memory. This can create different
outcomes depending on the simulation environment so move the simulation
skip to the start of the function for 100% consistency in results.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
---
lib/intel_os.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/intel_os.c b/lib/intel_os.c
index 29a27272e782..e1e31e23069b 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -375,6 +375,8 @@ void intel_require_memory(uint64_t count, uint64_t size, unsigned mode)
uint64_t required, total;
bool sufficient_memory;
+ igt_skip_on_simulation();
+
sufficient_memory = __intel_check_memory(count, size, mode,
&required, &total);
if (!sufficient_memory) {
@@ -406,8 +408,6 @@ void intel_require_memory(uint64_t count, uint64_t size, unsigned mode)
mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
mode & CHECK_SWAP ? " + swap": "",
(long long)vfs_file_max());
-
- igt_skip_on_simulation();
}
void intel_purge_vm_caches(int drm_fd)
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
2018-09-12 9:33 ` [igt-dev] " Tvrtko Ursulin
@ 2018-09-12 9:33 ` Tvrtko Ursulin
-1 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-09-12 9:33 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
igt_skip_on_simulation is called both directly from tests but also from
library helpers. In the latter case especially the logged caller name is
useless since it is always the helper itself. What we instead want to know
is who is the caller.
Trivial approach would be to move the helper to a header as static inline,
but due the longjmp in it it can never be inlined. Alternative option is
to print a backtrace from it.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
---
lib/igt_core.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 23bb858fd886..990abc5a36b3 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
*/
void igt_skip_on_simulation(void)
{
+ bool in_simulation;
+
if (igt_only_list_subtests())
return;
+ in_simulation = igt_run_in_simulation();
+
if (!igt_can_fail()) {
- igt_fixture
- igt_require(!igt_run_in_simulation());
- } else
- igt_require(!igt_run_in_simulation());
+ igt_fixture {
+ if (in_simulation) {
+ print_backtrace();
+ igt_require(!in_simulation);
+ }
+ }
+ } else {
+ if (in_simulation) {
+ print_backtrace();
+ igt_require(!in_simulation);
+ }
+ }
}
/* structured logging */
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Intel-gfx] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
@ 2018-09-12 9:33 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-09-12 9:33 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
igt_skip_on_simulation is called both directly from tests but also from
library helpers. In the latter case especially the logged caller name is
useless since it is always the helper itself. What we instead want to know
is who is the caller.
Trivial approach would be to move the helper to a header as static inline,
but due the longjmp in it it can never be inlined. Alternative option is
to print a backtrace from it.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
---
lib/igt_core.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 23bb858fd886..990abc5a36b3 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
*/
void igt_skip_on_simulation(void)
{
+ bool in_simulation;
+
if (igt_only_list_subtests())
return;
+ in_simulation = igt_run_in_simulation();
+
if (!igt_can_fail()) {
- igt_fixture
- igt_require(!igt_run_in_simulation());
- } else
- igt_require(!igt_run_in_simulation());
+ igt_fixture {
+ if (in_simulation) {
+ print_backtrace();
+ igt_require(!in_simulation);
+ }
+ }
+ } else {
+ if (in_simulation) {
+ print_backtrace();
+ igt_require(!in_simulation);
+ }
+ }
}
/* structured logging */
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [PATH,i-g-t,1/2] intel: Be consistent with test results on simulation
2018-09-12 9:33 ` [igt-dev] " Tvrtko Ursulin
(?)
(?)
@ 2018-09-12 10:15 ` Patchwork
-1 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-09-12 10:15 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev
== Series Details ==
Series: series starting with [PATH,i-g-t,1/2] intel: Be consistent with test results on simulation
URL : https://patchwork.freedesktop.org/series/49542/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4807 -> IGTPW_1829 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/49542/revisions/1/mbox/
== Known issues ==
Here are the changes found in IGTPW_1829 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_hangcheck:
fi-kbl-guc: PASS -> DMESG-FAIL (fdo#107710, fdo#107860)
igt@gem_exec_suspend@basic-s3:
fi-kbl-soraka: NOTRUN -> INCOMPLETE (fdo#107774, fdo#107556)
igt@gem_mmap@basic-small-bo:
fi-glk-dsi: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
igt@kms_psr@primary_mmap_gtt:
{fi-cnl-u}: NOTRUN -> FAIL (fdo#107383) +3
igt@kms_psr@primary_page_flip:
fi-kbl-r: PASS -> FAIL (fdo#107336)
igt@kms_psr@sprite_plane_onoff:
fi-bdw-samus: NOTRUN -> FAIL (fdo#107383, fdo#107360)
igt@pm_rpm@module-reload:
fi-hsw-peppy: NOTRUN -> DMESG-WARN (fdo#107603)
fi-bdw-samus: NOTRUN -> DMESG-WARN (fdo#107603)
==== Possible fixes ====
igt@gem_exec_suspend@basic-s3:
fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
fi-cfl-8109u: INCOMPLETE (fdo#106070) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
fdo#107360 https://bugs.freedesktop.org/show_bug.cgi?id=107360
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
fdo#107603 https://bugs.freedesktop.org/show_bug.cgi?id=107603
fdo#107710 https://bugs.freedesktop.org/show_bug.cgi?id=107710
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
fdo#107860 https://bugs.freedesktop.org/show_bug.cgi?id=107860
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (46 -> 46) ==
Additional (5): fi-kbl-soraka fi-cnl-u fi-icl-u fi-hsw-peppy fi-bdw-samus
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* IGT: IGT_4639 -> IGTPW_1829
CI_DRM_4807: 55b148b84b254f61adbfeb89c4f6674664f01c46 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_1829: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1829/
IGT_4639: c7fa2ea9fbce87206474748100b825558eebe08e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1829/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATH i-g-t 1/2] intel: Be consistent with test results on simulation
2018-09-12 9:33 ` [igt-dev] " Tvrtko Ursulin
@ 2018-09-12 12:02 ` Chris Wilson
-1 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2018-09-12 12:02 UTC (permalink / raw)
To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx
Quoting Tvrtko Ursulin (2018-09-12 10:33:05)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Tests which call intel_require_memory currently always skip on simulation,
> unless they fail first due insufficient memory. This can create different
> outcomes depending on the simulation environment so move the simulation
> skip to the start of the function for 100% consistency in results.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
Makes sense.
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] 18+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATH i-g-t 1/2] intel: Be consistent with test results on simulation
@ 2018-09-12 12:02 ` Chris Wilson
0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2018-09-12 12:02 UTC (permalink / raw)
To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx
Quoting Tvrtko Ursulin (2018-09-12 10:33:05)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Tests which call intel_require_memory currently always skip on simulation,
> unless they fail first due insufficient memory. This can create different
> outcomes depending on the simulation environment so move the simulation
> skip to the start of the function for 100% consistency in results.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
Makes sense.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [PATH,i-g-t,1/2] intel: Be consistent with test results on simulation
2018-09-12 9:33 ` [igt-dev] " Tvrtko Ursulin
` (3 preceding siblings ...)
(?)
@ 2018-09-12 13:05 ` Patchwork
-1 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-09-12 13:05 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev
== Series Details ==
Series: series starting with [PATH,i-g-t,1/2] intel: Be consistent with test results on simulation
URL : https://patchwork.freedesktop.org/series/49542/
State : success
== Summary ==
= CI Bug Log - changes from IGT_4639_full -> IGTPW_1829_full =
== Summary - WARNING ==
Minor unknown changes coming with IGTPW_1829_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_1829_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/49542/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in IGTPW_1829_full:
=== IGT changes ===
==== Warnings ====
igt@pm_rc6_residency@rc6-accuracy:
shard-snb: PASS -> SKIP
== Known issues ==
Here are the changes found in IGTPW_1829_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ppgtt@blt-vs-render-ctxn:
shard-kbl: PASS -> INCOMPLETE (fdo#106023, fdo#103665)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
shard-glk: PASS -> FAIL (fdo#103167)
shard-kbl: PASS -> FAIL (fdo#103167)
igt@kms_setmode@basic:
shard-glk: PASS -> FAIL (fdo#99912)
igt@perf_pmu@rc6-runtime-pm:
shard-apl: PASS -> FAIL (fdo#105010)
==== Possible fixes ====
igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
shard-hsw: FAIL (fdo#105767) -> PASS
igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
shard-snb: FAIL (fdo#103166) -> PASS
igt@perf_pmu@all-busy-check-all:
shard-snb: INCOMPLETE (fdo#105411) -> PASS
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* IGT: IGT_4639 -> IGTPW_1829
* Linux: CI_DRM_4806 -> CI_DRM_4807
CI_DRM_4806: feeccde66999c5e87be3550f2159e5d7eeb61c67 @ git://anongit.freedesktop.org/gfx-ci/linux
CI_DRM_4807: 55b148b84b254f61adbfeb89c4f6674664f01c46 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_1829: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1829/
IGT_4639: c7fa2ea9fbce87206474748100b825558eebe08e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1829/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
2018-09-12 9:33 ` [Intel-gfx] " Tvrtko Ursulin
@ 2018-09-14 9:12 ` Daniel Vetter
-1 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2018-09-14 9:12 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> igt_skip_on_simulation is called both directly from tests but also from
> library helpers. In the latter case especially the logged caller name is
> useless since it is always the helper itself. What we instead want to know
> is who is the caller.
>
> Trivial approach would be to move the helper to a header as static inline,
> but due the longjmp in it it can never be inlined. Alternative option is
> to print a backtrace from it.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> ---
> lib/igt_core.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 23bb858fd886..990abc5a36b3 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
> */
> void igt_skip_on_simulation(void)
> {
> + bool in_simulation;
> +
> if (igt_only_list_subtests())
> return;
>
> + in_simulation = igt_run_in_simulation();
> +
> if (!igt_can_fail()) {
> - igt_fixture
> - igt_require(!igt_run_in_simulation());
> - } else
> - igt_require(!igt_run_in_simulation());
> + igt_fixture {
> + if (in_simulation) {
> + print_backtrace();
> + igt_require(!in_simulation);
> + }
> + }
> + } else {
> + if (in_simulation) {
> + print_backtrace();
> + igt_require(!in_simulation);
Hm, why don't we go right ahead and push this into igt_skip()? There's a
pile of other igt_require, and we tend to push a lot of these into the
library. So they have all the same problem.
-Daniel
> + }
> + }
> }
>
> /* structured logging */
> --
> 2.17.1
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
@ 2018-09-14 9:12 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2018-09-14 9:12 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Tvrtko Ursulin
On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> igt_skip_on_simulation is called both directly from tests but also from
> library helpers. In the latter case especially the logged caller name is
> useless since it is always the helper itself. What we instead want to know
> is who is the caller.
>
> Trivial approach would be to move the helper to a header as static inline,
> but due the longjmp in it it can never be inlined. Alternative option is
> to print a backtrace from it.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> ---
> lib/igt_core.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 23bb858fd886..990abc5a36b3 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
> */
> void igt_skip_on_simulation(void)
> {
> + bool in_simulation;
> +
> if (igt_only_list_subtests())
> return;
>
> + in_simulation = igt_run_in_simulation();
> +
> if (!igt_can_fail()) {
> - igt_fixture
> - igt_require(!igt_run_in_simulation());
> - } else
> - igt_require(!igt_run_in_simulation());
> + igt_fixture {
> + if (in_simulation) {
> + print_backtrace();
> + igt_require(!in_simulation);
> + }
> + }
> + } else {
> + if (in_simulation) {
> + print_backtrace();
> + igt_require(!in_simulation);
Hm, why don't we go right ahead and push this into igt_skip()? There's a
pile of other igt_require, and we tend to push a lot of these into the
library. So they have all the same problem.
-Daniel
> + }
> + }
> }
>
> /* structured logging */
> --
> 2.17.1
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
2018-09-14 9:12 ` Daniel Vetter
@ 2018-09-14 9:19 ` Tvrtko Ursulin
-1 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-09-14 9:19 UTC (permalink / raw)
To: Daniel Vetter, Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
On 14/09/2018 10:12, Daniel Vetter wrote:
> On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> igt_skip_on_simulation is called both directly from tests but also from
>> library helpers. In the latter case especially the logged caller name is
>> useless since it is always the helper itself. What we instead want to know
>> is who is the caller.
>>
>> Trivial approach would be to move the helper to a header as static inline,
>> but due the longjmp in it it can never be inlined. Alternative option is
>> to print a backtrace from it.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
>> ---
>> lib/igt_core.c | 20 ++++++++++++++++----
>> 1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/igt_core.c b/lib/igt_core.c
>> index 23bb858fd886..990abc5a36b3 100644
>> --- a/lib/igt_core.c
>> +++ b/lib/igt_core.c
>> @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
>> */
>> void igt_skip_on_simulation(void)
>> {
>> + bool in_simulation;
>> +
>> if (igt_only_list_subtests())
>> return;
>>
>> + in_simulation = igt_run_in_simulation();
>> +
>> if (!igt_can_fail()) {
>> - igt_fixture
>> - igt_require(!igt_run_in_simulation());
>> - } else
>> - igt_require(!igt_run_in_simulation());
>> + igt_fixture {
>> + if (in_simulation) {
>> + print_backtrace();
>> + igt_require(!in_simulation);
>> + }
>> + }
>> + } else {
>> + if (in_simulation) {
>> + print_backtrace();
>> + igt_require(!in_simulation);
>
> Hm, why don't we go right ahead and push this into igt_skip()? There's a
> pile of other igt_require, and we tend to push a lot of these into the
> library. So they have all the same problem.
Maybe.. I wasn't 100% this was a good idea to start with, or in another
words, that other people would consider it a problem. Since the downside
is test output gets more verbose on skips, or some could say more noisy.
So I basically floated the patch to see if it will provoke some
responses. :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
@ 2018-09-14 9:19 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-09-14 9:19 UTC (permalink / raw)
To: Daniel Vetter, Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Tvrtko Ursulin
On 14/09/2018 10:12, Daniel Vetter wrote:
> On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> igt_skip_on_simulation is called both directly from tests but also from
>> library helpers. In the latter case especially the logged caller name is
>> useless since it is always the helper itself. What we instead want to know
>> is who is the caller.
>>
>> Trivial approach would be to move the helper to a header as static inline,
>> but due the longjmp in it it can never be inlined. Alternative option is
>> to print a backtrace from it.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
>> ---
>> lib/igt_core.c | 20 ++++++++++++++++----
>> 1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/igt_core.c b/lib/igt_core.c
>> index 23bb858fd886..990abc5a36b3 100644
>> --- a/lib/igt_core.c
>> +++ b/lib/igt_core.c
>> @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
>> */
>> void igt_skip_on_simulation(void)
>> {
>> + bool in_simulation;
>> +
>> if (igt_only_list_subtests())
>> return;
>>
>> + in_simulation = igt_run_in_simulation();
>> +
>> if (!igt_can_fail()) {
>> - igt_fixture
>> - igt_require(!igt_run_in_simulation());
>> - } else
>> - igt_require(!igt_run_in_simulation());
>> + igt_fixture {
>> + if (in_simulation) {
>> + print_backtrace();
>> + igt_require(!in_simulation);
>> + }
>> + }
>> + } else {
>> + if (in_simulation) {
>> + print_backtrace();
>> + igt_require(!in_simulation);
>
> Hm, why don't we go right ahead and push this into igt_skip()? There's a
> pile of other igt_require, and we tend to push a lot of these into the
> library. So they have all the same problem.
Maybe.. I wasn't 100% this was a good idea to start with, or in another
words, that other people would consider it a problem. Since the downside
is test output gets more verbose on skips, or some could say more noisy.
So I basically floated the patch to see if it will provoke some
responses. :)
Regards,
Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
2018-09-14 9:19 ` Tvrtko Ursulin
@ 2018-09-14 9:46 ` Daniel Vetter
-1 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2018-09-14 9:46 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
On Fri, Sep 14, 2018 at 10:19:29AM +0100, Tvrtko Ursulin wrote:
>
> On 14/09/2018 10:12, Daniel Vetter wrote:
> > On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >
> > > igt_skip_on_simulation is called both directly from tests but also from
> > > library helpers. In the latter case especially the logged caller name is
> > > useless since it is always the helper itself. What we instead want to know
> > > is who is the caller.
> > >
> > > Trivial approach would be to move the helper to a header as static inline,
> > > but due the longjmp in it it can never be inlined. Alternative option is
> > > to print a backtrace from it.
> > >
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> > > ---
> > > lib/igt_core.c | 20 ++++++++++++++++----
> > > 1 file changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > index 23bb858fd886..990abc5a36b3 100644
> > > --- a/lib/igt_core.c
> > > +++ b/lib/igt_core.c
> > > @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
> > > */
> > > void igt_skip_on_simulation(void)
> > > {
> > > + bool in_simulation;
> > > +
> > > if (igt_only_list_subtests())
> > > return;
> > > + in_simulation = igt_run_in_simulation();
> > > +
> > > if (!igt_can_fail()) {
> > > - igt_fixture
> > > - igt_require(!igt_run_in_simulation());
> > > - } else
> > > - igt_require(!igt_run_in_simulation());
> > > + igt_fixture {
> > > + if (in_simulation) {
> > > + print_backtrace();
> > > + igt_require(!in_simulation);
> > > + }
> > > + }
> > > + } else {
> > > + if (in_simulation) {
> > > + print_backtrace();
> > > + igt_require(!in_simulation);
> >
> > Hm, why don't we go right ahead and push this into igt_skip()? There's a
> > pile of other igt_require, and we tend to push a lot of these into the
> > library. So they have all the same problem.
>
> Maybe.. I wasn't 100% this was a good idea to start with, or in another
> words, that other people would consider it a problem. Since the downside is
> test output gets more verbose on skips, or some could say more noisy.
>
> So I basically floated the patch to see if it will provoke some responses.
> :)
We have the backtrace already in igt_fail, makes total sense to add it to
igt_skip too. Has my ack at least.
Aside: I kinda wonder whether we need an __igt_require, which embeds the
if (igt_can_fail) igt_require() else igt_fixture igt_require() thing. But
that's just an aside.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
@ 2018-09-14 9:46 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2018-09-14 9:46 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Tvrtko Ursulin, Daniel Vetter
On Fri, Sep 14, 2018 at 10:19:29AM +0100, Tvrtko Ursulin wrote:
>
> On 14/09/2018 10:12, Daniel Vetter wrote:
> > On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >
> > > igt_skip_on_simulation is called both directly from tests but also from
> > > library helpers. In the latter case especially the logged caller name is
> > > useless since it is always the helper itself. What we instead want to know
> > > is who is the caller.
> > >
> > > Trivial approach would be to move the helper to a header as static inline,
> > > but due the longjmp in it it can never be inlined. Alternative option is
> > > to print a backtrace from it.
> > >
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> > > ---
> > > lib/igt_core.c | 20 ++++++++++++++++----
> > > 1 file changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > index 23bb858fd886..990abc5a36b3 100644
> > > --- a/lib/igt_core.c
> > > +++ b/lib/igt_core.c
> > > @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
> > > */
> > > void igt_skip_on_simulation(void)
> > > {
> > > + bool in_simulation;
> > > +
> > > if (igt_only_list_subtests())
> > > return;
> > > + in_simulation = igt_run_in_simulation();
> > > +
> > > if (!igt_can_fail()) {
> > > - igt_fixture
> > > - igt_require(!igt_run_in_simulation());
> > > - } else
> > > - igt_require(!igt_run_in_simulation());
> > > + igt_fixture {
> > > + if (in_simulation) {
> > > + print_backtrace();
> > > + igt_require(!in_simulation);
> > > + }
> > > + }
> > > + } else {
> > > + if (in_simulation) {
> > > + print_backtrace();
> > > + igt_require(!in_simulation);
> >
> > Hm, why don't we go right ahead and push this into igt_skip()? There's a
> > pile of other igt_require, and we tend to push a lot of these into the
> > library. So they have all the same problem.
>
> Maybe.. I wasn't 100% this was a good idea to start with, or in another
> words, that other people would consider it a problem. Since the downside is
> test output gets more verbose on skips, or some could say more noisy.
>
> So I basically floated the patch to see if it will provoke some responses.
> :)
We have the backtrace already in igt_fail, makes total sense to add it to
igt_skip too. Has my ack at least.
Aside: I kinda wonder whether we need an __igt_require, which embeds the
if (igt_can_fail) igt_require() else igt_fixture igt_require() thing. But
that's just an aside.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
2018-09-14 9:46 ` Daniel Vetter
@ 2018-09-14 9:49 ` Chris Wilson
-1 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2018-09-14 9:49 UTC (permalink / raw)
To: Daniel Vetter, Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
Quoting Daniel Vetter (2018-09-14 10:46:25)
> On Fri, Sep 14, 2018 at 10:19:29AM +0100, Tvrtko Ursulin wrote:
> >
> > On 14/09/2018 10:12, Daniel Vetter wrote:
> > > On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > >
> > > > igt_skip_on_simulation is called both directly from tests but also from
> > > > library helpers. In the latter case especially the logged caller name is
> > > > useless since it is always the helper itself. What we instead want to know
> > > > is who is the caller.
> > > >
> > > > Trivial approach would be to move the helper to a header as static inline,
> > > > but due the longjmp in it it can never be inlined. Alternative option is
> > > > to print a backtrace from it.
> > > >
> > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> > > > ---
> > > > lib/igt_core.c | 20 ++++++++++++++++----
> > > > 1 file changed, 16 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > > index 23bb858fd886..990abc5a36b3 100644
> > > > --- a/lib/igt_core.c
> > > > +++ b/lib/igt_core.c
> > > > @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
> > > > */
> > > > void igt_skip_on_simulation(void)
> > > > {
> > > > + bool in_simulation;
> > > > +
> > > > if (igt_only_list_subtests())
> > > > return;
> > > > + in_simulation = igt_run_in_simulation();
> > > > +
> > > > if (!igt_can_fail()) {
> > > > - igt_fixture
> > > > - igt_require(!igt_run_in_simulation());
> > > > - } else
> > > > - igt_require(!igt_run_in_simulation());
> > > > + igt_fixture {
> > > > + if (in_simulation) {
> > > > + print_backtrace();
> > > > + igt_require(!in_simulation);
> > > > + }
> > > > + }
> > > > + } else {
> > > > + if (in_simulation) {
> > > > + print_backtrace();
> > > > + igt_require(!in_simulation);
> > >
> > > Hm, why don't we go right ahead and push this into igt_skip()? There's a
> > > pile of other igt_require, and we tend to push a lot of these into the
> > > library. So they have all the same problem.
> >
> > Maybe.. I wasn't 100% this was a good idea to start with, or in another
> > words, that other people would consider it a problem. Since the downside is
> > test output gets more verbose on skips, or some could say more noisy.
> >
> > So I basically floated the patch to see if it will provoke some responses.
> > :)
>
> We have the backtrace already in igt_fail, makes total sense to add it to
> igt_skip too. Has my ack at least.
Skip is rather more frequent than fail, and more often than not,
expected. So I am not entirely enjoying the prospect of a lot more noise,
the requirement message was supposed to be sufficient to understand why
we skipped. Maybe enforce that we have no igt_skip without a message?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
@ 2018-09-14 9:49 ` Chris Wilson
0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2018-09-14 9:49 UTC (permalink / raw)
To: Daniel Vetter, Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
Quoting Daniel Vetter (2018-09-14 10:46:25)
> On Fri, Sep 14, 2018 at 10:19:29AM +0100, Tvrtko Ursulin wrote:
> >
> > On 14/09/2018 10:12, Daniel Vetter wrote:
> > > On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > >
> > > > igt_skip_on_simulation is called both directly from tests but also from
> > > > library helpers. In the latter case especially the logged caller name is
> > > > useless since it is always the helper itself. What we instead want to know
> > > > is who is the caller.
> > > >
> > > > Trivial approach would be to move the helper to a header as static inline,
> > > > but due the longjmp in it it can never be inlined. Alternative option is
> > > > to print a backtrace from it.
> > > >
> > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> > > > ---
> > > > lib/igt_core.c | 20 ++++++++++++++++----
> > > > 1 file changed, 16 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > > index 23bb858fd886..990abc5a36b3 100644
> > > > --- a/lib/igt_core.c
> > > > +++ b/lib/igt_core.c
> > > > @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
> > > > */
> > > > void igt_skip_on_simulation(void)
> > > > {
> > > > + bool in_simulation;
> > > > +
> > > > if (igt_only_list_subtests())
> > > > return;
> > > > + in_simulation = igt_run_in_simulation();
> > > > +
> > > > if (!igt_can_fail()) {
> > > > - igt_fixture
> > > > - igt_require(!igt_run_in_simulation());
> > > > - } else
> > > > - igt_require(!igt_run_in_simulation());
> > > > + igt_fixture {
> > > > + if (in_simulation) {
> > > > + print_backtrace();
> > > > + igt_require(!in_simulation);
> > > > + }
> > > > + }
> > > > + } else {
> > > > + if (in_simulation) {
> > > > + print_backtrace();
> > > > + igt_require(!in_simulation);
> > >
> > > Hm, why don't we go right ahead and push this into igt_skip()? There's a
> > > pile of other igt_require, and we tend to push a lot of these into the
> > > library. So they have all the same problem.
> >
> > Maybe.. I wasn't 100% this was a good idea to start with, or in another
> > words, that other people would consider it a problem. Since the downside is
> > test output gets more verbose on skips, or some could say more noisy.
> >
> > So I basically floated the patch to see if it will provoke some responses.
> > :)
>
> We have the backtrace already in igt_fail, makes total sense to add it to
> igt_skip too. Has my ack at least.
Skip is rather more frequent than fail, and more often than not,
expected. So I am not entirely enjoying the prospect of a lot more noise,
the requirement message was supposed to be sufficient to understand why
we skipped. Maybe enforce that we have no igt_skip without a message?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
2018-09-14 9:49 ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2018-09-14 15:27 ` Daniel Vetter
-1 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2018-09-14 15:27 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, Intel-gfx
On Fri, Sep 14, 2018 at 10:49:47AM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2018-09-14 10:46:25)
> > On Fri, Sep 14, 2018 at 10:19:29AM +0100, Tvrtko Ursulin wrote:
> > >
> > > On 14/09/2018 10:12, Daniel Vetter wrote:
> > > > On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
> > > > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > >
> > > > > igt_skip_on_simulation is called both directly from tests but also from
> > > > > library helpers. In the latter case especially the logged caller name is
> > > > > useless since it is always the helper itself. What we instead want to know
> > > > > is who is the caller.
> > > > >
> > > > > Trivial approach would be to move the helper to a header as static inline,
> > > > > but due the longjmp in it it can never be inlined. Alternative option is
> > > > > to print a backtrace from it.
> > > > >
> > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > > Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> > > > > ---
> > > > > lib/igt_core.c | 20 ++++++++++++++++----
> > > > > 1 file changed, 16 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > > > index 23bb858fd886..990abc5a36b3 100644
> > > > > --- a/lib/igt_core.c
> > > > > +++ b/lib/igt_core.c
> > > > > @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
> > > > > */
> > > > > void igt_skip_on_simulation(void)
> > > > > {
> > > > > + bool in_simulation;
> > > > > +
> > > > > if (igt_only_list_subtests())
> > > > > return;
> > > > > + in_simulation = igt_run_in_simulation();
> > > > > +
> > > > > if (!igt_can_fail()) {
> > > > > - igt_fixture
> > > > > - igt_require(!igt_run_in_simulation());
> > > > > - } else
> > > > > - igt_require(!igt_run_in_simulation());
> > > > > + igt_fixture {
> > > > > + if (in_simulation) {
> > > > > + print_backtrace();
> > > > > + igt_require(!in_simulation);
> > > > > + }
> > > > > + }
> > > > > + } else {
> > > > > + if (in_simulation) {
> > > > > + print_backtrace();
> > > > > + igt_require(!in_simulation);
> > > >
> > > > Hm, why don't we go right ahead and push this into igt_skip()? There's a
> > > > pile of other igt_require, and we tend to push a lot of these into the
> > > > library. So they have all the same problem.
> > >
> > > Maybe.. I wasn't 100% this was a good idea to start with, or in another
> > > words, that other people would consider it a problem. Since the downside is
> > > test output gets more verbose on skips, or some could say more noisy.
> > >
> > > So I basically floated the patch to see if it will provoke some responses.
> > > :)
> >
> > We have the backtrace already in igt_fail, makes total sense to add it to
> > igt_skip too. Has my ack at least.
>
> Skip is rather more frequent than fail, and more often than not,
> expected. So I am not entirely enjoying the prospect of a lot more noise,
> the requirement message was supposed to be sufficient to understand why
> we skipped. Maybe enforce that we have no igt_skip without a message?
Hm yeah, adding a const char * to igt_skip_on_simulation that explains why
we're skipping could be the pretty solution here. Would also serve as some
self-documentation.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
@ 2018-09-14 15:27 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2018-09-14 15:27 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, Intel-gfx, Daniel Vetter
On Fri, Sep 14, 2018 at 10:49:47AM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2018-09-14 10:46:25)
> > On Fri, Sep 14, 2018 at 10:19:29AM +0100, Tvrtko Ursulin wrote:
> > >
> > > On 14/09/2018 10:12, Daniel Vetter wrote:
> > > > On Wed, Sep 12, 2018 at 10:33:06AM +0100, Tvrtko Ursulin wrote:
> > > > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > >
> > > > > igt_skip_on_simulation is called both directly from tests but also from
> > > > > library helpers. In the latter case especially the logged caller name is
> > > > > useless since it is always the helper itself. What we instead want to know
> > > > > is who is the caller.
> > > > >
> > > > > Trivial approach would be to move the helper to a header as static inline,
> > > > > but due the longjmp in it it can never be inlined. Alternative option is
> > > > > to print a backtrace from it.
> > > > >
> > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > > Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> > > > > ---
> > > > > lib/igt_core.c | 20 ++++++++++++++++----
> > > > > 1 file changed, 16 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > > > index 23bb858fd886..990abc5a36b3 100644
> > > > > --- a/lib/igt_core.c
> > > > > +++ b/lib/igt_core.c
> > > > > @@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
> > > > > */
> > > > > void igt_skip_on_simulation(void)
> > > > > {
> > > > > + bool in_simulation;
> > > > > +
> > > > > if (igt_only_list_subtests())
> > > > > return;
> > > > > + in_simulation = igt_run_in_simulation();
> > > > > +
> > > > > if (!igt_can_fail()) {
> > > > > - igt_fixture
> > > > > - igt_require(!igt_run_in_simulation());
> > > > > - } else
> > > > > - igt_require(!igt_run_in_simulation());
> > > > > + igt_fixture {
> > > > > + if (in_simulation) {
> > > > > + print_backtrace();
> > > > > + igt_require(!in_simulation);
> > > > > + }
> > > > > + }
> > > > > + } else {
> > > > > + if (in_simulation) {
> > > > > + print_backtrace();
> > > > > + igt_require(!in_simulation);
> > > >
> > > > Hm, why don't we go right ahead and push this into igt_skip()? There's a
> > > > pile of other igt_require, and we tend to push a lot of these into the
> > > > library. So they have all the same problem.
> > >
> > > Maybe.. I wasn't 100% this was a good idea to start with, or in another
> > > words, that other people would consider it a problem. Since the downside is
> > > test output gets more verbose on skips, or some could say more noisy.
> > >
> > > So I basically floated the patch to see if it will provoke some responses.
> > > :)
> >
> > We have the backtrace already in igt_fail, makes total sense to add it to
> > igt_skip too. Has my ack at least.
>
> Skip is rather more frequent than fail, and more often than not,
> expected. So I am not entirely enjoying the prospect of a lot more noise,
> the requirement message was supposed to be sufficient to understand why
> we skipped. Maybe enforce that we have no igt_skip without a message?
Hm yeah, adding a const char * to igt_skip_on_simulation that explains why
we're skipping could be the pretty solution here. Would also serve as some
self-documentation.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-09-14 15:27 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-12 9:33 [PATH i-g-t 1/2] intel: Be consistent with test results on simulation Tvrtko Ursulin
2018-09-12 9:33 ` [igt-dev] " Tvrtko Ursulin
2018-09-12 9:33 ` [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation Tvrtko Ursulin
2018-09-12 9:33 ` [Intel-gfx] " Tvrtko Ursulin
2018-09-14 9:12 ` [igt-dev] " Daniel Vetter
2018-09-14 9:12 ` Daniel Vetter
2018-09-14 9:19 ` Tvrtko Ursulin
2018-09-14 9:19 ` Tvrtko Ursulin
2018-09-14 9:46 ` Daniel Vetter
2018-09-14 9:46 ` Daniel Vetter
2018-09-14 9:49 ` Chris Wilson
2018-09-14 9:49 ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-09-14 15:27 ` [igt-dev] " Daniel Vetter
2018-09-14 15:27 ` [igt-dev] [Intel-gfx] " Daniel Vetter
2018-09-12 10:15 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [PATH,i-g-t,1/2] intel: Be consistent with test results on simulation Patchwork
2018-09-12 12:02 ` [PATH i-g-t 1/2] " Chris Wilson
2018-09-12 12:02 ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-09-12 13:05 ` [igt-dev] ✓ Fi.CI.IGT: success for series starting with [PATH,i-g-t,1/2] " Patchwork
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.