[PATCH i-g-t] lib/igt_aux: Allow sysfs open to fail when setting suspend/resume delay

[PATCH i-g-t] lib/igt_aux: Allow sysfs open to fail when setting suspend/resume delay

[Paul Kocialkowski]

This removes the igt_require condition on the sysfs open call used to
write the suspend/resume delay so that it is allowed to fail. Intsead,
the code that depends on it is put in a conditional block.

This allows running test binaries as a non-privileged user for e.g.
listing the available tests with the SuspendResumeDelay parameter set
in igtrc configuration. Sysfs access would otherwise cause it to fail.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 lib/igt_aux.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index f428f159..d808fe3e 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -883,19 +883,21 @@ void igt_set_autoresume_delay(int delay_secs)
 
 	igt_skip_on_simulation();
 
-	igt_require((delay_fd = open("/sys/module/suspend/parameters/pm_test_delay",
-				    O_RDWR)) >= 0);
+	delay_fd = open("/sys/module/suspend/parameters/pm_test_delay", O_RDWR);
+
+	if (delay_fd >= 0) {
+		if (!original_autoresume_delay) {
+			igt_require(read(delay_fd, delay_str,
+					 sizeof(delay_str)));
+			original_autoresume_delay = atoi(delay_str);
+			igt_install_exit_handler(igt_restore_autoresume_delay);
+		}
 
-	if (!original_autoresume_delay) {
-		igt_require(read(delay_fd, delay_str, sizeof(delay_str)));
-		original_autoresume_delay = atoi(delay_str);
-		igt_install_exit_handler(igt_restore_autoresume_delay);
-	}
+		snprintf(delay_str, sizeof(delay_str), "%d", delay_secs);
+		igt_require(write(delay_fd, delay_str, strlen(delay_str)));
 
-	snprintf(delay_str, sizeof(delay_str), "%d", delay_secs);
-	igt_require(write(delay_fd, delay_str, strlen(delay_str)));
-
-	close(delay_fd);
+		close(delay_fd);
+	}
 
 	autoresume_delay = delay_secs;
 }
-- 
2.14.0

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

✓ Fi.CI.BAT: success for lib/igt_aux: Allow sysfs open to fail when setting suspend/resume delay

[Patchwork]

== Series Details ==

Series: lib/igt_aux: Allow sysfs open to fail when setting suspend/resume delay
URL   : https://patchwork.freedesktop.org/series/29549/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
fc6510887f8f45e18ca267e53eb564de043bd9d6 tools: Add intel_vbt_defs.h to Makefile.sources

with latest DRM-Tip kernel build CI_DRM_3018
6a305b78140a drm-tip: 2017y-08m-30d-08h-12m-34s UTC integration manifest

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215 +1
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l) fdo#101781

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:459s
fi-bdw-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:441s
fi-blb-e6850     total:288  pass:224  dwarn:1   dfail:0   fail:0   skip:63  time:364s
fi-bsw-n3050     total:288  pass:243  dwarn:0   dfail:0   fail:0   skip:45  time:565s
fi-bwr-2160      total:288  pass:184  dwarn:0   dfail:0   fail:0   skip:104 time:255s
fi-bxt-j4205     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:527s
fi-byt-j1900     total:288  pass:254  dwarn:1   dfail:0   fail:0   skip:33  time:524s
fi-byt-n2820     total:288  pass:250  dwarn:1   dfail:0   fail:0   skip:37  time:519s
fi-elk-e7500     total:288  pass:230  dwarn:0   dfail:0   fail:0   skip:58  time:438s
fi-glk-2a        total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:617s
fi-hsw-4770      total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:447s
fi-hsw-4770r     total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:426s
fi-ilk-650       total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:424s
fi-ivb-3520m     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-ivb-3770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:474s
fi-kbl-7500u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:482s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:595s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:598s
fi-pnv-d510      total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:532s
fi-skl-6260u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:482s
fi-skl-6700k     total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:543s
fi-skl-6770hq    total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:491s
fi-skl-gvtdvm    total:288  pass:266  dwarn:0   dfail:0   fail:0   skip:22  time:448s
fi-skl-x1585l    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:496s
fi-snb-2520m     total:288  pass:251  dwarn:0   dfail:0   fail:0   skip:37  time:551s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:2   skip:38  time:407s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_126/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for lib/igt_aux: Allow sysfs open to fail when setting suspend/resume delay

[Patchwork]

== Series Details ==

Series: lib/igt_aux: Allow sysfs open to fail when setting suspend/resume delay
URL   : https://patchwork.freedesktop.org/series/29549/
State : success

== Summary ==

Test kms_properties:
        Subgroup plane-properties-legacy:
                skip       -> PASS       (shard-hsw)
Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_plane_multiple:
        Subgroup legacy-pipe-E-tiling-y:
                incomplete -> SKIP       (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test kms_atomic_transition:
        Subgroup plane-all-transition-fencing:
                skip       -> PASS       (shard-hsw)
Test kms_flip:
        Subgroup plain-flip-fb-recreate-interruptible:
                fail       -> PASS       (shard-hsw)
Test vgem_basic:
        Subgroup unload:
                skip       -> PASS       (shard-hsw) fdo#102453
Test kms_plane:
        Subgroup plane-position-hole-dpms-pipe-C-planes:
                skip       -> PASS       (shard-hsw)

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102453 https://bugs.freedesktop.org/show_bug.cgi?id=102453

shard-hsw        total:2265 pass:1231 dwarn:0   dfail:0   fail:18  skip:1016 time:9676s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_126/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] lib/igt_aux: Allow sysfs open to fail when setting suspend/resume delay

[Arkadiusz Hiler]

On Wed, Aug 30, 2017 at 04:56:09PM +0300, Paul Kocialkowski wrote:
> This removes the igt_require condition on the sysfs open call used to
> write the suspend/resume delay so that it is allowed to fail. Intsead,
> the code that depends on it is put in a conditional block.
> 
> This allows running test binaries as a non-privileged user for e.g.
> listing the available tests with the SuspendResumeDelay parameter set
> in igtrc configuration. Sysfs access would otherwise cause it to fail.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

and pushed

thanks for fixing it on a short notice!

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

Re: [PATCH i-g-t] lib/igt_aux: Allow sysfs open to fail when setting suspend/resume delay

[Daniel Vetter]

On Wed, Aug 30, 2017 at 09:15:03PM +0300, Arkadiusz Hiler wrote:
> On Wed, Aug 30, 2017 at 04:56:09PM +0300, Paul Kocialkowski wrote:
> > This removes the igt_require condition on the sysfs open call used to
> > write the suspend/resume delay so that it is allowed to fail. Intsead,
> > the code that depends on it is put in a conditional block.
> > 
> > This allows running test binaries as a non-privileged user for e.g.
> > listing the available tests with the SuspendResumeDelay parameter set
> > in igtrc configuration. Sysfs access would otherwise cause it to fail.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

This is the wrong fix. When enumerating tests you're not supposed to touch
anything hw or system related at all. Allocating a bit of memory and stuff
like that is ok, but not anything with effects.

The correct way to handle this is by wrapping this into an igt_fixture
block. Hitting an igt_require/assert outside of an igt_fixture or
igt_subtest should result in a assert.

You still have igt_skip and igt_require in there, so not fixed properly
yet.

Aside: We need to come up with a way to have functions shared between
parts of the library, without documenting and exporting them to tests.
-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