On 20/02/2018 21:40, Chris Wilson wrote:
Rather than iteratively disable and then immediately reenable a CPU,
turn off each in turn, forcing the PMU events onto the next CPU without
allowing them to retreat back to CPU0 after the first. If this fails,

Hm, interesting and I think it possibly makes sense to test both migration patterns.

immediately reboot the system.

Yes, we already agreed on this one and I was planning to do it so thanks for beating me to it.

Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
---
  lib/Makefile.sources |  2 ++
  lib/igt_sysrq.c      | 20 ++++++++++++++++++
  lib/igt_sysrq.h      | 30 +++++++++++++++++++++++++++
  lib/meson.build      |  2 ++
  tests/perf_pmu.c     | 57 ++++++++++++++++++++++++++++++++++------------------
  5 files changed, 92 insertions(+), 19 deletions(-)
  create mode 100644 lib/igt_sysrq.c
  create mode 100644 lib/igt_sysrq.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 86fbfeef..e4a9b059 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -33,6 +33,8 @@ lib_source_list =             \
        igt_stats.h             \
        igt_sysfs.c             \
        igt_sysfs.h             \
+       igt_sysrq.c             \
+       igt_sysrq.h             \
        igt_x86.h               \
        igt_x86.c               \
        igt_vgem.c              \
diff --git a/lib/igt_sysrq.c b/lib/igt_sysrq.c
new file mode 100644
index 00000000..32fb4a39
--- /dev/null
+++ b/lib/igt_sysrq.c
@@ -0,0 +1,20 @@
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <sys/reboot.h>
+
+#include "igt_sysrq.h"
+
+void igt_sysrq_reboot(void)
+{
+       sync();
+
+       /* Try to be nice at first, and if that fails pull the trigger */
+       if (reboot(RB_AUTOBOOT)) {
+               int fd = open("/proc/sysrq-trigger", O_WRONLY);
+               write(fd, "b", 2);
+               close(fd);
+       }
+
+       abort();
+}
diff --git a/lib/igt_sysrq.h b/lib/igt_sysrq.h
new file mode 100644
index 00000000..422473d2
--- /dev/null
+++ b/lib/igt_sysrq.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __IGT_SYSRQ_H__
+#define __IGT_SYSRQ_H__
+
+void igt_sysrq_reboot(void) __attribute__((noreturn));
+
+#endif /* __IGT_SYSRQ_H__ */
diff --git a/lib/meson.build b/lib/meson.build
index 94ea0799..2c611348 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -21,6 +21,7 @@ lib_headers = [
        'igt_stats.h',
        'igt_syncobj.h',
        'igt_sysfs.h',
+       'igt_sysrq.h',
        'igt_x86.h',
        'igt_vgem.h',
        'instdone.h',
@@ -67,6 +68,7 @@ lib_sources = [
        'igt_stats.c',
        'igt_syncobj.c',
        'igt_sysfs.c',
+       'igt_sysrq.c',
        'igt_vgem.c',
        'igt_x86.c',
        'instdone.c',
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 7fab73e2..1421fd9a 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -41,6 +41,7 @@
  #include "igt_core.h"
  #include "igt_perf.h"
  #include "igt_sysfs.h"
+#include "igt_sysrq.h"
  #include "igt_pm.h"
  #include "sw_sync.h"
@@ -957,6 +958,16 @@ static bool cpu0_hotplug_support(void)
        return access("/sys/devices/system/cpu/cpu0/online", W_OK) == 0;
  }
+static int open_cpu_online(int cpu)
+{
+       char name[128];
+
+       igt_assert_lt(snprintf(name, sizeof(name),
+                              "/sys/devices/system/cpu/cpu%d/online",
+                              cpu), sizeof(name));
+       return open(name, O_WRONLY);
+}
+
  static void cpu_hotplug(int gem_fd)
  {
        igt_spin_t *spin[2];
@@ -988,35 +999,43 @@ static void cpu_hotplug(int gem_fd)
         */
        igt_fork(child, 1) {
                int cpu = 0;
+               int cpufd;
+               int err;
close(link[0]); + /* Offline each cpu in turn */
                for (;;) {
-                       char name[128];
-                       int cpufd;
-
-                       igt_assert_lt(snprintf(name, sizeof(name),
-                                              
"/sys/devices/system/cpu/cpu%d/online",
-                                              cpu), sizeof(name));
-                       cpufd = open(name, O_WRONLY);
-                       if (cpufd == -1) {
-                               igt_assert(cpu > 0);
-                               /*
-                                * Signal parent that we cycled through all
-                                * CPUs and we are done.
-                                */
-                               igt_assert_eq(write(link[1], "*", 1), 1);
+                       cpufd = open_cpu_online(cpu);
+                       igt_assert(cpufd != -1);
+
+                       err = write(cpufd, "0", 2);
+                       close(cpufd);
+                       if (err < 0)
                                break;

Keep off-lining until no more CPUs to offline? I had to try it! :) Ok, last one will fail to offline. But I think it needs a comment.

-                       }
- /* Offline followed by online a CPU. */
-                       igt_assert_eq(write(cpufd, "0", 2), 2);
                        usleep(1e6);
-                       igt_assert_eq(write(cpufd, "1", 2), 2);
+                       cpu++;
+               }
+ /* Then bring them back online */
+               while (cpu--) {
+                       cpufd = open_cpu_online(cpu);
+                       err = write(cpufd, "1", 2);
                        close(cpufd);

Need to online in the same order or the PMU will stay on some higher CPU making the subsequent tests fail. Or I need to improve the helpers to hunt for the correct CPU, as perf tool does.

-                       cpu++;
+
+                       if (err < 0) {
+                               igt_warn("Failed to bring CPU%d back online\n",
+                                        cpu);
+                               igt_sysrq_reboot(); > +                      }
                }
+
+               /*
+                * Signal parent that we cycled through all
+                * CPUs and we are done.
+                */
+               igt_assert_eq(write(link[1], "*", 1), 1);
        }
close(link[1]);


Regards,

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

Reply via email to