diff mbox series

[i-g-t] igt/pm_rps: Wait for the load-helper to signal receipt of command

Message ID 20180904080659.10547-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [i-g-t] igt/pm_rps: Wait for the load-helper to signal receipt of command | expand

Commit Message

Chris Wilson Sept. 4, 2018, 8:06 a.m. UTC 
Actually wait for the load-helper to complete it switch over to the new
load by using a pipe() between the two.

References: https://bugs.freedesktop.org/show_bug.cgi?id=102250
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/pm_rps.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Katarzyna Dec Sept. 4, 2018, 10:48 a.m. UTC | #1 
On Tue, Sep 04, 2018 at 09:06:59AM +0100, Chris Wilson wrote:
> Actually wait for the load-helper to complete it switch over to the new
> load by using a pipe() between the two.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102250
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
I hope this will fix sporadic issues with pm_rps that still coming back.

Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
Chris Wilson Sept. 4, 2018, 10:57 a.m. UTC | #2 
Quoting Katarzyna Dec (2018-09-04 11:48:39)
> On Tue, Sep 04, 2018 at 09:06:59AM +0100, Chris Wilson wrote:
> > Actually wait for the load-helper to complete it switch over to the new
> > load by using a pipe() between the two.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=102250
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> I hope this will fix sporadic issues with pm_rps that still coming back.

I too hope, but I don't expect it to be the end of them. Another idea
would be to move the load control over to a shared page between the
helper and the test. Then we know that the request high/low are in
sync...
-Chris
diff mbox series

Patch

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 202132b1c..8dff5bbc5 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -181,8 +181,10 @@  enum load {
 };
 
 static struct load_helper {
+	int link;
 	enum load load;
 	bool exit;
+	bool signal;
 	struct igt_helper_process igt_proc;
 } lh;
 
@@ -190,6 +192,7 @@  static void load_helper_signal_handler(int sig)
 {
 	if (sig == SIGUSR2) {
 		lh.load = !lh.load;
+		lh.signal = true;
 		igt_debug("Switching background load to %s\n", lh.load ? "high" : "low");
 	} else
 		lh.exit = true;
@@ -199,6 +202,8 @@  static void load_helper_signal_handler(int sig)
 #define LOAD_HELPER_BO_SIZE (16*1024*1024)
 static void load_helper_set_load(enum load load)
 {
+	bool dummy;
+
 	igt_assert(lh.igt_proc.running);
 
 	if (lh.load == load)
@@ -207,11 +212,14 @@  static void load_helper_set_load(enum load load)
 	lh.load = load;
 	kill(lh.igt_proc.pid, SIGUSR2);
 
-	usleep(1000); /* wait for load-helper to switch */
+	/* wait for load-helper to switch */
+	igt_assert_eq(read(lh.link, &dummy, sizeof(dummy)), sizeof(dummy));
 }
 
 static void load_helper_run(enum load load)
 {
+	int link[2];
+
 	/*
 	 * FIXME fork helpers won't get cleaned up when started from within a
 	 * subtest, so handle the case where it sticks around a bit too long.
@@ -225,6 +233,10 @@  static void load_helper_run(enum load load)
 
 	lh.exit = false;
 	lh.load = load;
+	lh.signal = false;
+
+	pipe(link);
+	lh.link = link[1];
 
 	igt_fork_helper(&lh.igt_proc) {
 		igt_spin_t *spin[2] = {};
@@ -249,6 +261,11 @@  static void load_helper_run(enum load load)
 
 			spin[0] = spin[1];
 			spin[lh.load == HIGH] = __igt_spin_batch_new(drm_fd);
+
+			if (lh.signal) {
+				write(lh.link, &lh.signal, sizeof(lh.signal));
+				lh.signal = false;
+			}
 		}
 
 		handle = spin[0]->handle;
@@ -274,6 +291,8 @@  static void load_helper_run(enum load load)
 		igt_spin_batch_free(drm_fd, spin[1]);
 		igt_spin_batch_free(drm_fd, spin[0]);
 	}
+
+	lh.link = link[0];
 }
 
 static void load_helper_stop(void)