Message ID | 20180219134037.28708-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Quoting Tvrtko Ursulin (2018-02-19 13:40:37) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Bail out from the cpu-hotplug test if we failed to bring a CPU back > online. > > This still leaves the machine in a quite bad state, but at least it > avoids hard hanging it. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > tests/perf_pmu.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c > index 7fab73e22c2d..a334d3b5770e 100644 > --- a/tests/perf_pmu.c > +++ b/tests/perf_pmu.c > @@ -965,6 +965,7 @@ static void cpu_hotplug(int gem_fd) > int link[2]; > int fd, ret; > int cur = 0; > + char buf; > > igt_require(cpu0_hotplug_support()); > > @@ -1012,7 +1013,15 @@ static void cpu_hotplug(int gem_fd) > /* Offline followed by online a CPU. */ > igt_assert_eq(write(cpufd, "0", 2), 2); > usleep(1e6); > - igt_assert_eq(write(cpufd, "1", 2), 2); > + ret = write(cpufd, "1", 2); > + if (ret < 0) { > + /* > + * Abort the test if we failed to bring a CPU > + * back online. > + */ > + igt_assert_eq(write(link[1], "s", 1), 1); > + break; > + } > > close(cpufd); > cpu++; > @@ -1026,7 +1035,6 @@ static void cpu_hotplug(int gem_fd) > * until the CPU core shuffler finishes one loop. > */ > for (;;) { > - char buf; > int ret2; > > usleep(500e3); > @@ -1053,6 +1061,9 @@ static void cpu_hotplug(int gem_fd) > close(fd); > close(link[0]); > > + /* Skip if child signals a problem with bringing a CPU back online. */ > + igt_skip_on(buf == 's'); The logic makes sense. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> What happens if we try to online an already on cpu? Will that report the failure or just tell us to stop wasting its time? -Chris
On 19/02/2018 13:54, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-02-19 13:40:37) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Bail out from the cpu-hotplug test if we failed to bring a CPU back >> online. >> >> This still leaves the machine in a quite bad state, but at least it >> avoids hard hanging it. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> --- >> tests/perf_pmu.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c >> index 7fab73e22c2d..a334d3b5770e 100644 >> --- a/tests/perf_pmu.c >> +++ b/tests/perf_pmu.c >> @@ -965,6 +965,7 @@ static void cpu_hotplug(int gem_fd) >> int link[2]; >> int fd, ret; >> int cur = 0; >> + char buf; >> >> igt_require(cpu0_hotplug_support()); >> >> @@ -1012,7 +1013,15 @@ static void cpu_hotplug(int gem_fd) >> /* Offline followed by online a CPU. */ >> igt_assert_eq(write(cpufd, "0", 2), 2); >> usleep(1e6); >> - igt_assert_eq(write(cpufd, "1", 2), 2); >> + ret = write(cpufd, "1", 2); >> + if (ret < 0) { >> + /* >> + * Abort the test if we failed to bring a CPU >> + * back online. >> + */ >> + igt_assert_eq(write(link[1], "s", 1), 1); >> + break; >> + } >> >> close(cpufd); >> cpu++; >> @@ -1026,7 +1035,6 @@ static void cpu_hotplug(int gem_fd) >> * until the CPU core shuffler finishes one loop. >> */ >> for (;;) { >> - char buf; >> int ret2; >> >> usleep(500e3); >> @@ -1053,6 +1061,9 @@ static void cpu_hotplug(int gem_fd) >> close(fd); >> close(link[0]); >> >> + /* Skip if child signals a problem with bringing a CPU back online. */ >> + igt_skip_on(buf == 's'); > > The logic makes sense. > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> I wasn't so sure after I've sent it. There was an assert on write error already so that would have aborted the child. Will see what shards will say.. > What happens if we try to online an already on cpu? Will that report the > failure or just tell us to stop wasting its time? Seems the attempt is simply ignored. Regards, Tvrtko
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c index 7fab73e22c2d..a334d3b5770e 100644 --- a/tests/perf_pmu.c +++ b/tests/perf_pmu.c @@ -965,6 +965,7 @@ static void cpu_hotplug(int gem_fd) int link[2]; int fd, ret; int cur = 0; + char buf; igt_require(cpu0_hotplug_support()); @@ -1012,7 +1013,15 @@ static void cpu_hotplug(int gem_fd) /* Offline followed by online a CPU. */ igt_assert_eq(write(cpufd, "0", 2), 2); usleep(1e6); - igt_assert_eq(write(cpufd, "1", 2), 2); + ret = write(cpufd, "1", 2); + if (ret < 0) { + /* + * Abort the test if we failed to bring a CPU + * back online. + */ + igt_assert_eq(write(link[1], "s", 1), 1); + break; + } close(cpufd); cpu++; @@ -1026,7 +1035,6 @@ static void cpu_hotplug(int gem_fd) * until the CPU core shuffler finishes one loop. */ for (;;) { - char buf; int ret2; usleep(500e3); @@ -1053,6 +1061,9 @@ static void cpu_hotplug(int gem_fd) close(fd); close(link[0]); + /* Skip if child signals a problem with bringing a CPU back online. */ + igt_skip_on(buf == 's'); + assert_within_epsilon(val, ts[1] - ts[0], tolerance); }