[Intel-gfx] [i-g-t PATCH v3 3/3] Convert shell script tests to C version
Arkadiusz Hiler
arkadiusz.hiler at intel.com
Mon Jun 12 11:14:45 UTC 2017
On Tue, Jun 06, 2017 at 11:54:14AM +0300, Abdiel Janulgue wrote:
> v3: Drop redundant test covered by drv_hangman/basic. Descend thru
> debugfs path when reading sysfs entries (Chris).
>
> v2: Use internal igt_debugfs functions instead of cat and document
> debugfs tests.
> Convert sysfs_l3_parity properly.
> Rename redundant names in tests.
>
> Converted:
> - check_drm_clients (ensures no other clients are running.
> functionality provided by drm_open_driver_master).
> - debugfs_emon_crash
> - debugfs_wedged
> - drv_debugfs_reader
> - sysfs_l3_parity
> - test_rte_check (same as check_drm_clients)
> - tools_test
> - ZZ_check_dmesg
Hey,
Doing each tool in a separate commit would help with readability.
>
> Cc: Petri Latvala <petri.latvala at intel.com>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
> tests/Makefile.sources | 9 +---
> tests/ZZ_check_dmesg | 11 -----
> tests/check_drm_clients | 6 ---
> tests/debugfs.c | 96 +++++++++++++++++++++++++++++++++++++
> tests/debugfs_emon_crash | 16 -------
> tests/debugfs_wedged | 10 ----
> tests/drv_debugfs_reader | 9 ----
> tests/sysfs_l3_parity | 22 ---------
> tests/test_rte_check | 6 ---
> tests/tools.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++
> tests/tools_test | 16 -------
> 11 files changed, 220 insertions(+), 103 deletions(-)
> delete mode 100755 tests/ZZ_check_dmesg
> delete mode 100755 tests/check_drm_clients
> create mode 100644 tests/debugfs.c
> delete mode 100755 tests/debugfs_emon_crash
> delete mode 100755 tests/debugfs_wedged
> delete mode 100755 tests/drv_debugfs_reader
> delete mode 100755 tests/sysfs_l3_parity
> delete mode 100755 tests/test_rte_check
> create mode 100644 tests/tools.c
> delete mode 100755 tests/tools_test
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 9553e4d..c4a78a9 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -243,6 +243,8 @@ TESTS_progs = \
> drv_module_reload \
> kms_sysfs_edid_timing \
> perf \
> + debugfs \
The name clashes on AOSP build:
build/core/base_rules.mk:238: error: external/igt/tests: MODULE.TARGET.EXECUTABLES.debugfs already defined by external/e2fsprogs/debugfs.
debugfs is a name of a binary that debugs ext{2,3,4} family of FSes.
I have to look into the build system and our Android.mks more deeply as
IGTs output binaries should be stored elsewhere and the clash is rather
enigmatic.
Also are we changing names of the test? I thought it was only a rewrite.
> + tools \
> $(NULL)
>
> # IMPORTANT: The ZZ_ tests need to be run last!
> @@ -251,11 +253,6 @@ TESTS_scripts_M = \
> $(NULL)
>
> TESTS_scripts = \
> - debugfs_emon_crash \
> - drv_debugfs_reader \
> - sysfs_l3_parity \
> - test_rte_check \
> - tools_test \
> $(NULL)
TESTS_scripts now is only $(NULL), so you may as well just remove it
from the `kernel_tests` variable and get rid of it completely.
>
> # This target contains testcases which support automagic subtest enumeration
> @@ -317,9 +314,7 @@ HANG = \
> $(NULL)
>
> scripts = \
> - check_drm_clients \
> ddx_intel_after_fbdev \
> - debugfs_wedged \
> drm_lib.sh \
> drm_getopt.sh \
> $(NULL)
<SNIP>
> diff --git a/tests/debugfs.c b/tests/debugfs.c
> new file mode 100644
> index 0000000..b9ae86c
> --- /dev/null
> +++ b/tests/debugfs.c
> @@ -0,0 +1,96 @@
> +/*
> + * Copyright © 2017 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.
> + */
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +
> +static void get_sysfs_entry(int path_fd)
I was a little bit confused by the function - we are not getting
anything from it. Just reading and discarding.
Maybe read_and_discard_sysfs_entry? A comment would also do.
> +{
> + struct dirent *dirent;
> + DIR *dir;
> +
> + dir = fdopendir(path_fd);
> + if (!dir)
> + return;
> +
> + while ((dirent = readdir(dir))) {
> + if (!strcmp(dirent->d_name, ".") ||
> + !strcmp(dirent->d_name, ".."))
> + continue;
> + if (dirent->d_type == DT_DIR) {
> + int sub_fd = -1;
> + igt_assert((sub_fd =
> + openat(path_fd, dirent->d_name, O_RDONLY |
> + O_DIRECTORY)) > 0);
> + get_sysfs_entry(sub_fd);
> + close(sub_fd);
> + } else {
> + char *buf = igt_sysfs_get(path_fd, dirent->d_name);
igt_sysfs_get may get us NULL.
Shouldn't we assert on that? It's an error-worthy.
Also we may end up trying to free it in with the free below.
> + free(buf);
> + }
> + }
> + closedir(dir);
> +}
> +
> +igt_main
> +{
> + int fd = -1, debugfs;
> + igt_skip_on_simulation();
> +
> + igt_fixture {
> + fd = drm_open_driver_master(DRIVER_INTEL);
> + igt_require_gem(fd);
> + debugfs = igt_debugfs_dir(fd);
> + }
> +
> + igt_subtest("drv_reader") {
> + get_sysfs_entry(debugfs);
> + }
> +
> + igt_subtest("emon_crash") {
> + int i;
> + /*
> + * This check if we can crash the kernel with
> + * segmentation-fault by reading
> + * /sys/kernel/debug/dri/0/i915_emon_status too quickly
> + */
> + for (i = 0; i < 1000; i++) {
> + char *buf = igt_sysfs_get(debugfs,
> + "i915_emon_status");
> + free(buf);
> + }
> +
> + /* If we got here, we haven't crashed */
> + igt_success();
> + }
> +
> + igt_fixture {
> + close(debugfs);
> + close(fd);
> + }
> +}
<SNIP>
> diff --git a/tests/tools.c b/tests/tools.c
> new file mode 100644
> index 0000000..2ec951a
> --- /dev/null
> +++ b/tests/tools.c
> @@ -0,0 +1,122 @@
> +/*
> + * Copyright © 2017 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.
> + */
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +#include "igt.h"
> +#include <stdio.h>
> +
> +/**
> + * Parse the r-value of a [cmd] string.
> + */
> +static bool check_cmd_return_value(const char *s, void *data)
> +{
> + int *val = data;
> + char *cmd, *found;
> + const char *delim = "[cmd]";
> + const int delim_len = strlen(delim);
> +
> + if (!(cmd = strstr(s, delim)))
> + return false;
> +
> + found = cmd + delim_len + 1;
> + igt_assert(delim_len + strlen(found) < strlen(cmd));
> +
> + *val = atoi(found);
> + return true;
> +}
> +
> +igt_main
> +{
> + int fd = -1;
> + igt_skip_on_simulation();
> +
> + igt_subtest("sysfs_l3_parity") {
> + int exec_return;
> +
> + igt_system_cmd(exec_return,
> + "../tools/intel_l3_parity -r 0 -b 0 "
> + "-s 0 -e");
> + igt_assert(exec_return == IGT_EXIT_SUCCESS);
> +
> + igt_system_cmd(exec_return,
> + "../tools/intel_l3_parity -l | "
> + "grep -c 'Row 0, Bank 0, Subbank 0 "
> + "is disabled'");
> + if (exec_return == IGT_EXIT_SUCCESS) {
> + int val = -1;
> + igt_log_buffer_inspect(check_cmd_return_value,
> + &val);
> + igt_assert(val == 1);
> + } else {
> + igt_fail(IGT_EXIT_FAILURE);
> + }
> +
> + igt_system_cmd(exec_return,
> + "../tools/intel_l3_parity -r 0 -b 0 "
> + "-s 0 -e");
> + igt_assert(exec_return == IGT_EXIT_SUCCESS);
> +
> + /* Check that we can clear remaps */
> + igt_system_cmd(exec_return,
> + "../tools/intel_l3_parity -l | "
> + "wc -l");
> + if (exec_return == IGT_EXIT_SUCCESS) {
> + int val = -1;
> + igt_log_buffer_inspect(check_cmd_return_value,
> + &val);
> + igt_assert(val == 1);
> + } else {
> + igt_fail(IGT_EXIT_FAILURE);
> + }
> + }
> +
> + igt_subtest("tools_test") {
> + char *cmd;
> +
> + igt_assert(asprintf(&cmd,
> + "../tools/intel_reg read 0x4030")
> + != -1);
> + igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
> + free(cmd);
> +
> + igt_assert(asprintf(&cmd, "../tools/intel_reg dump")
> + != -1);
> + igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
> + free(cmd);
> + }
> +
> + igt_subtest("check_dmesg") {
> + char *cmd;
> + igt_assert(asprintf(&cmd, "dmesg | grep '\\*ERROR\\*'")
> + != -1);
> + igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
> + free(cmd);
> +
> + igt_assert(asprintf(&cmd, "dmesg | grep "
> + "-- '------\\[ cut here \\]----'")
> + != -1);
> + igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
> + free(cmd);
Are we testing the dmesg here?
If not they why systeming out "dmesg | grep" instead of going through
/dev/kmsg?
> + }
> +}
--
Cheers,
Arek
More information about the Intel-gfx
mailing list