lib/igt_aux: Polish docs for igt_interruptible

- Give __ prefix to internal funcstion and structs, only
  igt_interruptible is used by tests.

- Move docs to igt_interruptible and adjust.

- Explain more clearly how the timeout is getting doubled each
  iteration until no more interruptions happen. Also rename the
  argument to give it a more meaningful name in the docs.

- Link from other functions to this one for cross-referencing.

- Rename to igt_do_interruptible to make it clearer it's a loop,
  inspired by do {} while () loops.

v2: Rename instead to igt_while_interruptible and fix typos (Chris).
And add gtk-doc for igt_ioctl, too.

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

10 files changed, 55 insertions, 46 deletions

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 22962dcf..6c3eb1f4 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -84,7 +84,7 @@
 #define gettid() syscall(__NR_gettid)
 #define sigev_notify_thread_id _sigev_un._tid
 
-static struct __igt_sigiter {
+static struct __igt_sigiter_global {
 	pid_t tid;
 	timer_t timer;
 	struct timespec offset;
@@ -159,7 +159,7 @@ sig_ioctl(int fd, unsigned long request, void *arg)
 	return ret ? -1 : 0;
 }
 
-static bool igt_sigiter_start(struct igt_sigiter *iter, bool enable)
+static bool igt_sigiter_start(struct __igt_sigiter *iter, bool enable)
 {
 	/* Note that until we can automatically clean up on failed/skipped
 	 * tests, we cannot assume the state of the igt_ioctl indirection.
@@ -218,7 +218,7 @@ static bool igt_sigiter_start(struct igt_sigiter *iter, bool enable)
 	return true;
 }
 
-static bool igt_sigiter_stop(struct igt_sigiter *iter, bool enable)
+static bool igt_sigiter_stop(struct __igt_sigiter *iter, bool enable)
 {
 	if (enable) {
 		struct sigaction act;
@@ -240,32 +240,7 @@ static bool igt_sigiter_stop(struct igt_sigiter *iter, bool enable)
 	return false;
 }
 
-/**
- * igt_sigiter_continue:
- * @iter: the control struct
- * @enable: a boolean as to whether or not we want to enable interruptions
- *
- * Provides control flow such that all drmIoctl() (strictly igt_ioctl())
- * within the loop are forcibly injected with signals (SIGRTMIN).
- *
- * This is useful to exercise ioctl error paths, at least where those can be
- * exercises by interrupting blocking waits, like stalling for the gpu.
- *
- * igt_sigiter_continue() returns false when it has detected that it
- * cannot inject any more signals in the ioctls from previous runs.
- *
- * Typical usage is
- * 	struct igt_sigiter iter = {};
- * 	while (igt_sigiter_continue(&iter, test_flags & TEST_INTERRUPTIBLE))
- * 		do_test();
- *
- * This is condensed into the igt_interruptible() macro.
- *
- * Note that since this overloads the igt_ioctl(), this method is not useful
- * for widespread signal injection, for example providing coverage of
- * pagefaults. To interrupt everything, see igt_fork_signal_helper().
- */
-bool igt_sigiter_continue(struct igt_sigiter *iter, bool enable)
+bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool enable)
 {
 	if (iter->pass++ == 0)
 		return igt_sigiter_start(iter, enable);
@@ -329,6 +304,10 @@ static void sig_handler(int i)
  *
  * In tests with subtests this function can be called outside of failure
  * catching code blocks like #igt_fixture or #igt_subtest.
+ *
+ * Note that this just spews signals at the current process unconditionally and
+ * hence incurs quite a bit of overhead. For a more focused approach, with less
+ * overhead, look at the #igt_while_interruptible code block macro.
  */
 void igt_fork_signal_helper(void)
 {
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index f66de721..faaf56ac 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -43,13 +43,34 @@ void igt_stop_signal_helper(void);
 void igt_fork_hang_detector(int fd);
 void igt_stop_hang_detector(void);
 
-struct igt_sigiter {
+struct __igt_sigiter {
 	unsigned pass;
 };
 
-bool igt_sigiter_continue(struct igt_sigiter *iter, bool interrupt);
-#define igt_interruptible(E) \
-	for (struct igt_sigiter iter__={}; igt_sigiter_continue(&iter__, (E)); )
+bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool interrupt);
+
+/**
+ * igt_while_interruptible:
+ * @enable: enable igt_ioctl interrupting or not
+ *
+ * Provides control flow such that all drmIoctl() (strictly igt_ioctl())
+ * within the loop are forcibly injected with signals (SIGRTMIN).
+ *
+ * This is useful to exercise ioctl error paths, at least where those can be
+ * exercises by interrupting blocking waits, like stalling for the gpu.
+ *
+ * The code block attached to this macro is run in a loop with doubling the
+ * interrupt timeout on each ioctl for every run, until no ioctl gets
+ * interrupted any more. The starting timeout is taken to be the signal delivery
+ * latency, measured at runtime. This way the any ioctls called from this code
+ * block should be exhaustively tested for all signal interruption paths.
+ *
+ * Note that since this overloads the igt_ioctl(), this method is not useful
+ * for widespread signal injection, for example providing coverage of
+ * pagefaults. To interrupt everything, see igt_fork_signal_helper().
+ */
+#define igt_while_interruptible(enable) \
+	for (struct __igt_sigiter iter__={}; __igt_sigiter_continue(&iter__, (enable)); )
 
 #define igt_timeout(T) \
 	for (struct timespec t__={}; igt_seconds_elapsed(&t__) < (T); )
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 8fe35b0c..f3bd23f5 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -35,6 +35,15 @@
 #include <intel_bufmgr.h>
 #include <i915_drm.h>
 
+/**
+ * igt_ioctl:
+ * @fd: file descriptor
+ * @request: IOCTL request number
+ * @arg: argument pointer
+ *
+ * This is a wrapper around drmIoctl(), which can be augmented with special code
+ * blocks like #igt_while_interruptible.
+ */
 extern int (*igt_ioctl)(int fd, unsigned long request, void *arg);
 
 /* libdrm interfacing */
diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
index c0af60d4..019628ed 100644
--- a/tests/gem_concurrent_all.c
+++ b/tests/gem_concurrent_all.c
@@ -1294,7 +1294,7 @@ static void run_interruptible(struct buffers *buffers,
 			      do_hang do_hang_func)
 {
 	pass = 0;
-	igt_interruptible(true)
+	igt_while_interruptible(true)
 		do_test_func(buffers, do_copy_func, do_hang_func);
 	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
 }
@@ -1340,7 +1340,7 @@ static void __run_forked(struct buffers *buffers,
 		buffers_reset(buffers, true);
 		buffers_create(buffers);
 
-		igt_interruptible(interrupt) {
+		igt_while_interruptible(interrupt) {
 			for (pass = 0; pass < loops; pass++)
 				do_test_func(buffers,
 					     do_copy_func,
diff --git a/tests/gem_ctx_switch.c b/tests/gem_ctx_switch.c
index 004ba227..7b273368 100644
--- a/tests/gem_ctx_switch.c
+++ b/tests/gem_ctx_switch.c
@@ -116,7 +116,7 @@ static void single(int fd, uint32_t handle,
 
 		clock_gettime(CLOCK_MONOTONIC, &start);
 		do {
-			igt_interruptible(flags & INTERRUPTIBLE) {
+			igt_while_interruptible(flags & INTERRUPTIBLE) {
 				for (int loop = 0; loop < 1024; loop++) {
 					execbuf.rsvd1 = contexts[loop % 64];
 					reloc.presumed_offset = 0;
diff --git a/tests/gem_exec_flush.c b/tests/gem_exec_flush.c
index 705ef849..93265ae5 100644
--- a/tests/gem_exec_flush.c
+++ b/tests/gem_exec_flush.c
@@ -216,7 +216,7 @@ overwrite:
 
 			if (flags & SET_DOMAIN) {
 				unsigned domain = flags & WC ? I915_GEM_DOMAIN_GTT : I915_GEM_DOMAIN_CPU;
-				igt_interruptible(flags & INTERRUPTIBLE)
+				igt_while_interruptible(flags & INTERRUPTIBLE)
 					gem_set_domain(fd, obj[0].handle,
 						       domain, (flags & WRITE) ? domain : 0);
 
@@ -230,7 +230,7 @@ overwrite:
 			} else if (flags & KERNEL) {
 				uint32_t val;
 
-				igt_interruptible(flags & INTERRUPTIBLE)
+				igt_while_interruptible(flags & INTERRUPTIBLE)
 					gem_read(fd, obj[0].handle,
 						 i*sizeof(uint32_t),
 						 &val, sizeof(val));
@@ -242,13 +242,13 @@ overwrite:
 
 				if (flags & WRITE) {
 					val = 0xdeadbeef;
-					igt_interruptible(flags & INTERRUPTIBLE)
+					igt_while_interruptible(flags & INTERRUPTIBLE)
 						gem_write(fd, obj[0].handle,
 							  i*sizeof(uint32_t),
 							  &val, sizeof(val));
 				}
 			} else {
-				igt_interruptible(flags & INTERRUPTIBLE)
+				igt_while_interruptible(flags & INTERRUPTIBLE)
 					gem_sync(fd, obj[0].handle);
 
 				if (!(flags & (BEFORE | COHERENT)) &&
diff --git a/tests/gem_exec_whisper.c b/tests/gem_exec_whisper.c
index 8db475e2..e0739470 100644
--- a/tests/gem_exec_whisper.c
+++ b/tests/gem_exec_whisper.c
@@ -225,7 +225,7 @@ static void whisper(int fd, unsigned engine, unsigned flags)
 		gem_write(fd, batches[n].handle, 0, batch, sizeof(batch));
 	}
 
-	igt_interruptible(flags & INTERRUPTIBLE) {
+	igt_while_interruptible(flags & INTERRUPTIBLE) {
 		for (pass = 0; pass < 1024; pass++) {
 			uint64_t offset;
 
diff --git a/tests/gem_ringfill.c b/tests/gem_ringfill.c
index 52192d9b..e503913d 100644
--- a/tests/gem_ringfill.c
+++ b/tests/gem_ringfill.c
@@ -75,7 +75,7 @@ static void fill_ring(int fd,
 	 * doing this, we aren't likely to with this test.
 	 */
 	igt_debug("Executing execbuf %d times\n", 128*1024/(8*4));
-	igt_interruptible(flags & INTERRUPTIBLE) {
+	igt_while_interruptible(flags & INTERRUPTIBLE) {
 		for (int i = 0; i < 128*1024 / (8 * 4); i++)
 			gem_execbuf(fd, execbuf);
 	}
diff --git a/tests/gem_softpin.c b/tests/gem_softpin.c
index 1a9ef02b..a6d4274c 100644
--- a/tests/gem_softpin.c
+++ b/tests/gem_softpin.c
@@ -492,7 +492,7 @@ igt_main
 	igt_subtest("noreloc")
 		test_noreloc(fd, NOSLEEP);
 	igt_subtest("noreloc-interruptible")
-		igt_interruptible(true) test_noreloc(fd, NOSLEEP);
+		igt_while_interruptible(true) test_noreloc(fd, NOSLEEP);
 	igt_subtest("noreloc-S3")
 		test_noreloc(fd, SUSPEND);
 	igt_subtest("noreloc-S4")
@@ -500,9 +500,9 @@ igt_main
 
 	for (int signal = 0; signal <= 1; signal++) {
 		igt_subtest_f("evict-active%s", signal ? "-interruptible" : "")
-			igt_interruptible(signal) test_evict_active(fd);
+			igt_while_interruptible(signal) test_evict_active(fd);
 		igt_subtest_f("evict-snoop%s", signal ? "-interruptible" : "")
-			igt_interruptible(signal) test_evict_snoop(fd);
+			igt_while_interruptible(signal) test_evict_snoop(fd);
 	}
 	igt_subtest("evict-hang")
 		test_evict_hang(fd);
diff --git a/tests/prime_mmap_coherency.c b/tests/prime_mmap_coherency.c
index 5cacdc5d..fb67ef1f 100644
--- a/tests/prime_mmap_coherency.c
+++ b/tests/prime_mmap_coherency.c
@@ -277,7 +277,7 @@ static void test_ioctl_errors(void)
 		}
 
 		igt_fork(child, num_children)
-			igt_interruptible(true) blit_and_cmp();
+			igt_while_interruptible(true) blit_and_cmp();
 		igt_waitchildren();
 	}
 }