| Message ID | 1447680143-11895-1-git-send-email-joonas.lahtinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote: > Cc: Thomas Wood <thomas.wood@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Damien Lespiau <damien.lespiau@intel.com> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > lib/igt_core.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++--- > tests/Makefile.sources | 1 + > tests/igt_capture.c | 93 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 200 insertions(+), 7 deletions(-) > create mode 100644 tests/igt_capture.c > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 04a0ab2..e73175a 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -211,6 +211,8 @@ > * "--help" command line option. > */ > > +#define IGT_KMSG_CAPTURE_DUMP_BUF_SIZE 4096 > + > static unsigned int exit_handler_count; > const char *igt_interactive_debug; > > @@ -247,6 +249,10 @@ enum { > static int igt_exitcode = IGT_EXIT_SUCCESS; > static const char *command_str; > > +static int igt_kmsg_capture_fd; > +static char* igt_kmsg_capture_dump_buf; > +static pthread_mutex_t kmsg_mutex = PTHREAD_MUTEX_INITIALIZER; > + > static char* igt_log_domain_filter; > static struct { > char *entries[256]; > @@ -312,6 +318,71 @@ static void _igt_log_buffer_dump(void) > pthread_mutex_unlock(&log_buffer_mutex); > } > > +static void _igt_kmsg_capture_reset(void) > +{ > + if (igt_kmsg_capture_fd == -1) > + return; > + > + pthread_mutex_lock(&kmsg_mutex); > + > + lseek(igt_kmsg_capture_fd, 0, SEEK_END); > + > + pthread_mutex_unlock(&kmsg_mutex); > +} Since close() is signal-safe, you can avoid the mutex (which is not signal-safe, but we can hope not to generate reentrancy here!) by opening /dev/kmsg for each subtest, and close it again in exit_subtest(). Note that we also want to detect dmesg spew during igt when the tests report all is well. Having the full log associated with a fail, preferrably with --debug is good, but filtering the <prio> for errors is also useful. -Chris
On ma, 2015-11-16 at 14:06 +0000, Chris Wilson wrote: > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote: > > Cc: Thomas Wood <thomas.wood@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Damien Lespiau <damien.lespiau@intel.com> > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > --- > > lib/igt_core.c | 113 > > ++++++++++++++++++++++++++++++++++++++++++++++--- > > tests/Makefile.sources | 1 + > > tests/igt_capture.c | 93 > > ++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 200 insertions(+), 7 deletions(-) > > create mode 100644 tests/igt_capture.c > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c > > index 04a0ab2..e73175a 100644 > > --- a/lib/igt_core.c > > +++ b/lib/igt_core.c > > @@ -211,6 +211,8 @@ > > * "--help" command line option. > > */ > > > > +#define IGT_KMSG_CAPTURE_DUMP_BUF_SIZE 4096 > > + > > static unsigned int exit_handler_count; > > const char *igt_interactive_debug; > > > > @@ -247,6 +249,10 @@ enum { > > static int igt_exitcode = IGT_EXIT_SUCCESS; > > static const char *command_str; > > > > +static int igt_kmsg_capture_fd; > > +static char* igt_kmsg_capture_dump_buf; > > +static pthread_mutex_t kmsg_mutex = PTHREAD_MUTEX_INITIALIZER; > > + > > static char* igt_log_domain_filter; > > static struct { > > char *entries[256]; > > @@ -312,6 +318,71 @@ static void _igt_log_buffer_dump(void) > > pthread_mutex_unlock(&log_buffer_mutex); > > } > > > > +static void _igt_kmsg_capture_reset(void) > > +{ > > + if (igt_kmsg_capture_fd == -1) > > + return; > > + > > + pthread_mutex_lock(&kmsg_mutex); > > + > > + lseek(igt_kmsg_capture_fd, 0, SEEK_END); > > + > > + pthread_mutex_unlock(&kmsg_mutex); > > +} > > Since close() is signal-safe, you can avoid the mutex (which is not > signal-safe, but we can hope not to generate reentrancy here!) by > opening /dev/kmsg for each subtest, and close it again in > exit_subtest(). > I actually put the mutex around that block while it was still using FILE*, when I was trying to cope with one open /dev/kmsg for reading and writing for duration of the whole run, so it's a left over (it was doing SEEK_CUR and storing that value etc.). Your suggestion sounds cleaner. > Note that we also want to detect dmesg spew during igt when the tests > report all is well. Having the full log associated with a fail, > preferrably with --debug is good, but filtering the <prio> for errors > is > also useful. Yes, exactly what I was thinking too, because there were tests in the SKL hang cases where the test would seem to run OK, but kmsg was full of errors. I wasn't sure if I should enable the kmsg logging with own command line flag (or environment variable, like Thomas added for disabling colors) or by default, as I am not exactly sure how many consumers we have that try to parse the IGT outputs. It would be a breaking change there. Maybe filtering the dmesg by locality kernel, and further with priority and then displaying the log if there are warning or more serious output during the test, in that case make the test fail and dump the kmsg? I think this is also what piglit does (status DMESG fail), so add an own return value for that, too. Regards, Joonas > -Chris >
On 16 November 2015 at 13:22, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote: > Cc: Thomas Wood <thomas.wood@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Damien Lespiau <damien.lespiau@intel.com> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > lib/igt_core.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++--- > tests/Makefile.sources | 1 + > tests/igt_capture.c | 93 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 200 insertions(+), 7 deletions(-) > create mode 100644 tests/igt_capture.c > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 04a0ab2..e73175a 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -211,6 +211,8 @@ > * "--help" command line option. > */ > > +#define IGT_KMSG_CAPTURE_DUMP_BUF_SIZE 4096 > + > static unsigned int exit_handler_count; > const char *igt_interactive_debug; > > @@ -247,6 +249,10 @@ enum { > static int igt_exitcode = IGT_EXIT_SUCCESS; > static const char *command_str; > > +static int igt_kmsg_capture_fd; > +static char* igt_kmsg_capture_dump_buf; > +static pthread_mutex_t kmsg_mutex = PTHREAD_MUTEX_INITIALIZER; > + > static char* igt_log_domain_filter; > static struct { > char *entries[256]; > @@ -312,6 +318,71 @@ static void _igt_log_buffer_dump(void) > pthread_mutex_unlock(&log_buffer_mutex); > } > > +static void _igt_kmsg_capture_reset(void) > +{ > + if (igt_kmsg_capture_fd == -1) > + return; > + > + pthread_mutex_lock(&kmsg_mutex); > + > + lseek(igt_kmsg_capture_fd, 0, SEEK_END); > + > + pthread_mutex_unlock(&kmsg_mutex); > +} > + > +static void _igt_kmsg_capture_dump(void) > +{ > + size_t nbytes; > + int nlines; > + char *p; > + char *p0; > + int c; > + > + if (igt_kmsg_capture_fd == -1 || > + igt_kmsg_capture_dump_buf == NULL) > + return; > + > + pthread_mutex_lock(&kmsg_mutex); > + > + > + nlines = 0; > + do { > + errno = 0; > + nbytes = read(igt_kmsg_capture_fd, > + igt_kmsg_capture_dump_buf, > + IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); > + > + if (nbytes == -1) > + continue; > + > + if (!nlines) > + fprintf(stderr, "**** KMSG ****\n"); > + > + p = p0 = strchr(igt_kmsg_capture_dump_buf, ';') + 1; > + while (p - p0 < nbytes) { > + if (*p != '\\') { > + fputc(*p++, stderr); > + continue; > + } > + sscanf(p, "\\x%x", &c); > + fputc(c, stderr); > + p += 4; > + } > + //fputs(strchr(igt_kmsg_capture_dump_buf, ';') + 1, stderr); This line can be removed if it is no longer needed. > + nlines++; > + } while(errno == 0); > + > + if (nlines) > + fprintf(stderr, "**** END ****\n"); > + else > + fprintf(stderr, "No kmsg.\n"); > + > + if (errno != EAGAIN) > + fprintf(stderr, "Incomplete kmsg.\n"); > + > + pthread_mutex_unlock(&kmsg_mutex); > +} > + > __attribute__((format(printf, 1, 2))) > static void kmsg(const char *format, ...) > #define KERN_EMER "<0>" > @@ -330,11 +401,15 @@ static void kmsg(const char *format, ...) > if (file == NULL) > return; > > + pthread_mutex_lock(&kmsg_mutex); > + > va_start(ap, format); > vfprintf(file, format, ap); > va_end(ap); > > fclose(file); > + > + pthread_mutex_unlock(&kmsg_mutex); > } > > static void gettime(struct timespec *ts) > @@ -527,6 +602,15 @@ static int common_init(int *argc, char **argv, > int ret = 0; > const char *env; > > + igt_kmsg_capture_fd = open("/dev/kmsg", O_RDWR | O_NONBLOCK); > + if (igt_kmsg_capture_fd != -1) { > + igt_kmsg_capture_dump_buf = > + malloc(IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); > + if(igt_kmsg_capture_dump_buf == NULL) > + igt_warn("Unable to allocate memory, " > + "will not dump kmsg.\n"); > + } > + > if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT")) > __igt_plain_output = true; > > @@ -796,6 +880,7 @@ bool __igt_run_subtest(const char *subtest_name) > igt_debug("Starting subtest: %s\n", subtest_name); > > _igt_log_buffer_reset(); > + _igt_kmsg_capture_reset(); > > gettime(&subtest_time); > return (in_subtest = subtest_name); > @@ -972,6 +1057,7 @@ void igt_fail(int exitcode) > exit(exitcode); > > _igt_log_buffer_dump(); > + _igt_kmsg_capture_dump(); > > if (in_subtest) { > if (exitcode == IGT_EXIT_TIMEOUT) > @@ -1072,16 +1158,21 @@ void __igt_fail_assert(const char *domain, const char *file, const int line, > */ > void igt_exit(void) > { > + int status = IGT_EXIT_SUCCESS; > + > igt_exit_called = true; > > if (run_single_subtest && !run_single_subtest_found) { > igt_warn("Unknown subtest: %s\n", run_single_subtest); > - exit(IGT_EXIT_INVALID); > + status = IGT_EXIT_INVALID; > + goto do_exit; > } > > > - if (igt_only_list_subtests()) > - exit(IGT_EXIT_SUCCESS); > + if (igt_only_list_subtests()) { > + status = IGT_EXIT_SUCCESS; > + goto do_exit; It might be good to avoid allocating the kmsg buffer and file descriptor altogether if only listing subtests. There are also a few cases where igt_exit won't be called, such as when using the help options or when a test has no subtests and one of the subtest options is specified. > + } > > kmsg(KERN_INFO "%s: exiting, ret=%d\n", command_str, igt_exitcode); > igt_debug("Exiting with status code %d\n", igt_exitcode); > @@ -1111,18 +1202,26 @@ void igt_exit(void) > > > printf("%s (%.3fs)\n", result, elapsed); > - exit(igt_exitcode); > + status = igt_exitcode; > + goto do_exit; > } > > /* Calling this without calling one of the above is a failure */ > assert(skipped_one || succeeded_one || failed_one); > > if (failed_one) > - exit(igt_exitcode); > + status = igt_exitcode; > else if (succeeded_one) > - exit(IGT_EXIT_SUCCESS); > + status = IGT_EXIT_SUCCESS; > else > - exit(IGT_EXIT_SKIP); > + status = IGT_EXIT_SKIP; > +do_exit: > + if (igt_kmsg_capture_fd != -1) > + close(igt_kmsg_capture_fd); > + if (igt_kmsg_capture_dump_buf != NULL) > + free(igt_kmsg_capture_dump_buf); Since this always needs to be run, would it be simpler to move it to the top of this function? > + > + exit(status); > } > > /* fork support code */ > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index 8fb2de8..25f0c4a 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -62,6 +62,7 @@ TESTS_progs_M = \ > gem_tiled_partial_pwrite_pread \ > gem_userptr_blits \ > gem_write_read_ring_switch \ > + igt_capture \ > kms_addfb_basic \ > kms_atomic \ > kms_cursor_crc \ > diff --git a/tests/igt_capture.c b/tests/igt_capture.c > new file mode 100644 > index 0000000..fd008d0 > --- /dev/null > +++ b/tests/igt_capture.c Since this is an infrastructure test, it would be better placed in lib/tests. The binary will also need adding to .gitignore. > @@ -0,0 +1,93 @@ > +/* > + * Copyright © 2015 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. > + * > + * Authors: > + * Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > + * > + */ > + > +#include "igt.h" > +#include <unistd.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include <string.h> > +#include <fcntl.h> > +#include <inttypes.h> > +#include <errno.h> > +#include <sys/stat.h> > +#include <sys/ioctl.h> > + > +static FILE* kmsg; > + > +static void > +test_kmsg(void) > +{ > + fputs("TEST (KMSG)\n", kmsg); > + fflush(kmsg); > + igt_fail(IGT_EXIT_FAILURE); > +} > + > +static void > +test_warn(void) > +{ > + igt_warn("TEST (WARN)\n"); > + igt_fail(IGT_EXIT_FAILURE); > +} > + > +static void > +test_debug(void) > +{ > + igt_debug("TEST (DEBUG)\n"); > + igt_fail(IGT_EXIT_FAILURE); > +} > + > +static void > +test_combined(void) > +{ > + igt_warn("TEST #1 (WARN)\n"); > + fputs("TEST #1\n", kmsg); > + igt_warn("TEST #2 (WARN)\n"); > + fputs("TEST #2\n", kmsg); > + fflush(kmsg); > + igt_fail(IGT_EXIT_FAILURE); > +} > + > +igt_main > +{ > + igt_fixture { > + kmsg = fopen("/dev/kmsg", "w"); > + igt_require(kmsg != NULL); > + } > + > + igt_subtest("kmsg") > + test_kmsg(); > + igt_subtest("warn") > + test_warn(); > + igt_subtest("debug") > + test_debug(); > + igt_subtest("combined") > + test_combined(); > + > + igt_fixture { > + fclose(kmsg); > + } > +} > -- > 2.4.3 >
On ti, 2015-11-17 at 13:05 +0000, Thomas Wood wrote: > On 16 November 2015 at 13:22, Joonas Lahtinen > <joonas.lahtinen@linux.intel.com> wrote: > > +static void _igt_kmsg_capture_dump(void) > > +{ > > <SNIP> > > + //fputs(strchr(igt_kmsg_capture_dump_buf, ';') + 1, > > stderr); > > This line can be removed if it is no longer needed. > Yeah, already removed from my v2. > > > @@ -1072,16 +1158,21 @@ void __igt_fail_assert(const char *domain, > > const char *file, const int line, > > */ > > void igt_exit(void) > > { > > + int status = IGT_EXIT_SUCCESS; > > + > > igt_exit_called = true; > > > > if (run_single_subtest && !run_single_subtest_found) { > > igt_warn("Unknown subtest: %s\n", > > run_single_subtest); > > - exit(IGT_EXIT_INVALID); > > + status = IGT_EXIT_INVALID; > > + goto do_exit; > > } > > > > > > - if (igt_only_list_subtests()) > > - exit(IGT_EXIT_SUCCESS); > > + if (igt_only_list_subtests()) { > > + status = IGT_EXIT_SUCCESS; > > + goto do_exit; > > It might be good to avoid allocating the kmsg buffer and file > descriptor altogether if only listing subtests. There are also a few > cases where igt_exit won't be called, such as when using the help > options or when a test has no subtests and one of the subtest options > is specified. Yes, Chris suggested only opening the FD when a subtest starts and closing after each one. Which comes to the point that the "single test or 1 or more subtests" leads to quite a few different codepaths, and I think it should be considered converting all the single tests to tests with only one subtest, to make the code maintainable. > > if (failed_one) > > - exit(igt_exitcode); > > + status = igt_exitcode; > > else if (succeeded_one) > > - exit(IGT_EXIT_SUCCESS); > > + status = IGT_EXIT_SUCCESS; > > else > > - exit(IGT_EXIT_SKIP); > > + status = IGT_EXIT_SKIP; > > +do_exit: > > + if (igt_kmsg_capture_fd != -1) > > + close(igt_kmsg_capture_fd); > > + if (igt_kmsg_capture_dump_buf != NULL) > > + free(igt_kmsg_capture_dump_buf); > > Since this always needs to be run, would it be simpler to move it to > the top of this function? > True, originally I did the dumping in this function so the position is kind of leftover from there. > > > + > > + exit(status); > > } > > > > /* fork support code */ > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > > index 8fb2de8..25f0c4a 100644 > > --- a/tests/Makefile.sources > > +++ b/tests/Makefile.sources > > @@ -62,6 +62,7 @@ TESTS_progs_M = \ > > gem_tiled_partial_pwrite_pread \ > > gem_userptr_blits \ > > gem_write_read_ring_switch \ > > + igt_capture \ > > kms_addfb_basic \ > > kms_atomic \ > > kms_cursor_crc \ > > diff --git a/tests/igt_capture.c b/tests/igt_capture.c > > new file mode 100644 > > index 0000000..fd008d0 > > --- /dev/null > > +++ b/tests/igt_capture.c > > Since this is an infrastructure test, it would be better placed in > lib/tests. The binary will also need adding to .gitignore. > Ok, will move it. I do notice there already is igt_stats in there. Regards, Joonas > > > @@ -0,0 +1,93 @@ > > +/* > > + * Copyright © 2015 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. > > + * > > + * Authors: > > + * Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > + * > > + */ > > + > > +#include "igt.h" > > +#include <unistd.h> > > +#include <stdlib.h> > > +#include <stdio.h> > > +#include <string.h> > > +#include <fcntl.h> > > +#include <inttypes.h> > > +#include <errno.h> > > +#include <sys/stat.h> > > +#include <sys/ioctl.h> > > + > > +static FILE* kmsg; > > + > > +static void > > +test_kmsg(void) > > +{ > > + fputs("TEST (KMSG)\n", kmsg); > > + fflush(kmsg); > > + igt_fail(IGT_EXIT_FAILURE); > > +} > > + > > +static void > > +test_warn(void) > > +{ > > + igt_warn("TEST (WARN)\n"); > > + igt_fail(IGT_EXIT_FAILURE); > > +} > > + > > +static void > > +test_debug(void) > > +{ > > + igt_debug("TEST (DEBUG)\n"); > > + igt_fail(IGT_EXIT_FAILURE); > > +} > > + > > +static void > > +test_combined(void) > > +{ > > + igt_warn("TEST #1 (WARN)\n"); > > + fputs("TEST #1\n", kmsg); > > + igt_warn("TEST #2 (WARN)\n"); > > + fputs("TEST #2\n", kmsg); > > + fflush(kmsg); > > + igt_fail(IGT_EXIT_FAILURE); > > +} > > + > > +igt_main > > +{ > > + igt_fixture { > > + kmsg = fopen("/dev/kmsg", "w"); > > + igt_require(kmsg != NULL); > > + } > > + > > + igt_subtest("kmsg") > > + test_kmsg(); > > + igt_subtest("warn") > > + test_warn(); > > + igt_subtest("debug") > > + test_debug(); > > + igt_subtest("combined") > > + test_combined(); > > + > > + igt_fixture { > > + fclose(kmsg); > > + } > > +} > > -- > > 2.4.3 > >
On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote: > Cc: Thomas Wood <thomas.wood@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Damien Lespiau <damien.lespiau@intel.com> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Given that we have all that in piglit already the commit message is a bit thin on justification. Why do we need this in igt too? How does this interact with the piglit dmesg capture? > --- > lib/igt_core.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++--- > tests/Makefile.sources | 1 + > tests/igt_capture.c | 93 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 200 insertions(+), 7 deletions(-) > create mode 100644 tests/igt_capture.c > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 04a0ab2..e73175a 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -211,6 +211,8 @@ > * "--help" command line option. > */ > > +#define IGT_KMSG_CAPTURE_DUMP_BUF_SIZE 4096 > + > static unsigned int exit_handler_count; > const char *igt_interactive_debug; > > @@ -247,6 +249,10 @@ enum { > static int igt_exitcode = IGT_EXIT_SUCCESS; > static const char *command_str; > > +static int igt_kmsg_capture_fd; > +static char* igt_kmsg_capture_dump_buf; > +static pthread_mutex_t kmsg_mutex = PTHREAD_MUTEX_INITIALIZER; > + > static char* igt_log_domain_filter; > static struct { > char *entries[256]; > @@ -312,6 +318,71 @@ static void _igt_log_buffer_dump(void) > pthread_mutex_unlock(&log_buffer_mutex); > } > > +static void _igt_kmsg_capture_reset(void) > +{ > + if (igt_kmsg_capture_fd == -1) > + return; > + > + pthread_mutex_lock(&kmsg_mutex); > + > + lseek(igt_kmsg_capture_fd, 0, SEEK_END); > + > + pthread_mutex_unlock(&kmsg_mutex); > +} > + > +static void _igt_kmsg_capture_dump(void) > +{ > + size_t nbytes; > + int nlines; > + char *p; > + char *p0; > + int c; > + > + if (igt_kmsg_capture_fd == -1 || > + igt_kmsg_capture_dump_buf == NULL) > + return; > + > + pthread_mutex_lock(&kmsg_mutex); > + > + > + nlines = 0; > + do { > + errno = 0; > + nbytes = read(igt_kmsg_capture_fd, > + igt_kmsg_capture_dump_buf, > + IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); > + > + if (nbytes == -1) > + continue; > + > + if (!nlines) > + fprintf(stderr, "**** KMSG ****\n"); > + > + p = p0 = strchr(igt_kmsg_capture_dump_buf, ';') + 1; > + while (p - p0 < nbytes) { > + if (*p != '\\') { > + fputc(*p++, stderr); > + continue; > + } > + sscanf(p, "\\x%x", &c); > + fputc(c, stderr); > + p += 4; > + } > + //fputs(strchr(igt_kmsg_capture_dump_buf, ';') + 1, stderr); > + nlines++; > + } while(errno == 0); > + > + if (nlines) > + fprintf(stderr, "**** END ****\n"); > + else > + fprintf(stderr, "No kmsg.\n"); > + > + if (errno != EAGAIN) > + fprintf(stderr, "Incomplete kmsg.\n"); > + > + pthread_mutex_unlock(&kmsg_mutex); > +} > + > __attribute__((format(printf, 1, 2))) > static void kmsg(const char *format, ...) > #define KERN_EMER "<0>" > @@ -330,11 +401,15 @@ static void kmsg(const char *format, ...) > if (file == NULL) > return; > > + pthread_mutex_lock(&kmsg_mutex); > + > va_start(ap, format); > vfprintf(file, format, ap); > va_end(ap); > > fclose(file); > + > + pthread_mutex_unlock(&kmsg_mutex); > } > > static void gettime(struct timespec *ts) > @@ -527,6 +602,15 @@ static int common_init(int *argc, char **argv, > int ret = 0; > const char *env; > > + igt_kmsg_capture_fd = open("/dev/kmsg", O_RDWR | O_NONBLOCK); > + if (igt_kmsg_capture_fd != -1) { > + igt_kmsg_capture_dump_buf = > + malloc(IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); > + if(igt_kmsg_capture_dump_buf == NULL) > + igt_warn("Unable to allocate memory, " > + "will not dump kmsg.\n"); > + } > + > if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT")) > __igt_plain_output = true; > > @@ -796,6 +880,7 @@ bool __igt_run_subtest(const char *subtest_name) > igt_debug("Starting subtest: %s\n", subtest_name); > > _igt_log_buffer_reset(); > + _igt_kmsg_capture_reset(); > > gettime(&subtest_time); > return (in_subtest = subtest_name); > @@ -972,6 +1057,7 @@ void igt_fail(int exitcode) > exit(exitcode); > > _igt_log_buffer_dump(); > + _igt_kmsg_capture_dump(); > > if (in_subtest) { > if (exitcode == IGT_EXIT_TIMEOUT) > @@ -1072,16 +1158,21 @@ void __igt_fail_assert(const char *domain, const char *file, const int line, > */ > void igt_exit(void) > { > + int status = IGT_EXIT_SUCCESS; > + > igt_exit_called = true; > > if (run_single_subtest && !run_single_subtest_found) { > igt_warn("Unknown subtest: %s\n", run_single_subtest); > - exit(IGT_EXIT_INVALID); > + status = IGT_EXIT_INVALID; > + goto do_exit; > } > > > - if (igt_only_list_subtests()) > - exit(IGT_EXIT_SUCCESS); > + if (igt_only_list_subtests()) { > + status = IGT_EXIT_SUCCESS; > + goto do_exit; > + } > > kmsg(KERN_INFO "%s: exiting, ret=%d\n", command_str, igt_exitcode); > igt_debug("Exiting with status code %d\n", igt_exitcode); > @@ -1111,18 +1202,26 @@ void igt_exit(void) > > > printf("%s (%.3fs)\n", result, elapsed); > - exit(igt_exitcode); > + status = igt_exitcode; > + goto do_exit; > } > > /* Calling this without calling one of the above is a failure */ > assert(skipped_one || succeeded_one || failed_one); > > if (failed_one) > - exit(igt_exitcode); > + status = igt_exitcode; > else if (succeeded_one) > - exit(IGT_EXIT_SUCCESS); > + status = IGT_EXIT_SUCCESS; > else > - exit(IGT_EXIT_SKIP); > + status = IGT_EXIT_SKIP; > +do_exit: > + if (igt_kmsg_capture_fd != -1) > + close(igt_kmsg_capture_fd); > + if (igt_kmsg_capture_dump_buf != NULL) > + free(igt_kmsg_capture_dump_buf); > + > + exit(status); > } > > /* fork support code */ > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index 8fb2de8..25f0c4a 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -62,6 +62,7 @@ TESTS_progs_M = \ > gem_tiled_partial_pwrite_pread \ > gem_userptr_blits \ > gem_write_read_ring_switch \ > + igt_capture \ > kms_addfb_basic \ > kms_atomic \ > kms_cursor_crc \ > diff --git a/tests/igt_capture.c b/tests/igt_capture.c > new file mode 100644 > index 0000000..fd008d0 > --- /dev/null > +++ b/tests/igt_capture.c > @@ -0,0 +1,93 @@ > +/* > + * Copyright © 2015 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. > + * > + * Authors: > + * Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > + * > + */ > + > +#include "igt.h" > +#include <unistd.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include <string.h> > +#include <fcntl.h> > +#include <inttypes.h> > +#include <errno.h> > +#include <sys/stat.h> > +#include <sys/ioctl.h> > + > +static FILE* kmsg; > + > +static void > +test_kmsg(void) > +{ > + fputs("TEST (KMSG)\n", kmsg); > + fflush(kmsg); > + igt_fail(IGT_EXIT_FAILURE); > +} > + > +static void > +test_warn(void) > +{ > + igt_warn("TEST (WARN)\n"); > + igt_fail(IGT_EXIT_FAILURE); > +} > + > +static void > +test_debug(void) > +{ > + igt_debug("TEST (DEBUG)\n"); > + igt_fail(IGT_EXIT_FAILURE); > +} > + > +static void > +test_combined(void) > +{ > + igt_warn("TEST #1 (WARN)\n"); > + fputs("TEST #1\n", kmsg); > + igt_warn("TEST #2 (WARN)\n"); > + fputs("TEST #2\n", kmsg); > + fflush(kmsg); > + igt_fail(IGT_EXIT_FAILURE); > +} > + > +igt_main > +{ > + igt_fixture { > + kmsg = fopen("/dev/kmsg", "w"); > + igt_require(kmsg != NULL); > + } > + > + igt_subtest("kmsg") > + test_kmsg(); > + igt_subtest("warn") > + test_warn(); > + igt_subtest("debug") > + test_debug(); > + igt_subtest("combined") > + test_combined(); > + > + igt_fixture { > + fclose(kmsg); > + } > +} > -- > 2.4.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Nov 18, 2015 at 04:44:20PM +0100, Daniel Vetter wrote: > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote: > > Cc: Thomas Wood <thomas.wood@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Damien Lespiau <damien.lespiau@intel.com> > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Given that we have all that in piglit already the commit message is a bit > thin on justification. Why do we need this in igt too? How does this > interact with the piglit dmesg capture? It's doesn't interfere with anyone else parsing kmsg/dmesg for themselves, but it adds very useful functionality to standalone igt. Which to me is significantly more valuable and I have been patching it into igt for over a year and wished it was taken more seriously given the number of incorrect bug reports generated. -Chris
On 18 November 2015 at 15:44, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote: >> Cc: Thomas Wood <thomas.wood@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Damien Lespiau <damien.lespiau@intel.com> >> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Given that we have all that in piglit already the commit message is a bit > thin on justification. Why do we need this in igt too? How does this > interact with the piglit dmesg capture? There are a few tests that rely on checking dmesg to determine if the test failed (such as gem_hangcheck_forcewake and kms_flip_event_leak). Although not implemented in this patch, it might be useful to use this feature to actually report a failure in those cases. > >> --- >> lib/igt_core.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++--- >> tests/Makefile.sources | 1 + >> tests/igt_capture.c | 93 ++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 200 insertions(+), 7 deletions(-) >> create mode 100644 tests/igt_capture.c >> >> diff --git a/lib/igt_core.c b/lib/igt_core.c >> index 04a0ab2..e73175a 100644 >> --- a/lib/igt_core.c >> +++ b/lib/igt_core.c >> @@ -211,6 +211,8 @@ >> * "--help" command line option. >> */ >> >> +#define IGT_KMSG_CAPTURE_DUMP_BUF_SIZE 4096 >> + >> static unsigned int exit_handler_count; >> const char *igt_interactive_debug; >> >> @@ -247,6 +249,10 @@ enum { >> static int igt_exitcode = IGT_EXIT_SUCCESS; >> static const char *command_str; >> >> +static int igt_kmsg_capture_fd; >> +static char* igt_kmsg_capture_dump_buf; >> +static pthread_mutex_t kmsg_mutex = PTHREAD_MUTEX_INITIALIZER; >> + >> static char* igt_log_domain_filter; >> static struct { >> char *entries[256]; >> @@ -312,6 +318,71 @@ static void _igt_log_buffer_dump(void) >> pthread_mutex_unlock(&log_buffer_mutex); >> } >> >> +static void _igt_kmsg_capture_reset(void) >> +{ >> + if (igt_kmsg_capture_fd == -1) >> + return; >> + >> + pthread_mutex_lock(&kmsg_mutex); >> + >> + lseek(igt_kmsg_capture_fd, 0, SEEK_END); >> + >> + pthread_mutex_unlock(&kmsg_mutex); >> +} >> + >> +static void _igt_kmsg_capture_dump(void) >> +{ >> + size_t nbytes; >> + int nlines; >> + char *p; >> + char *p0; >> + int c; >> + >> + if (igt_kmsg_capture_fd == -1 || >> + igt_kmsg_capture_dump_buf == NULL) >> + return; >> + >> + pthread_mutex_lock(&kmsg_mutex); >> + >> + >> + nlines = 0; >> + do { >> + errno = 0; >> + nbytes = read(igt_kmsg_capture_fd, >> + igt_kmsg_capture_dump_buf, >> + IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); >> + >> + if (nbytes == -1) >> + continue; >> + >> + if (!nlines) >> + fprintf(stderr, "**** KMSG ****\n"); >> + >> + p = p0 = strchr(igt_kmsg_capture_dump_buf, ';') + 1; >> + while (p - p0 < nbytes) { >> + if (*p != '\\') { >> + fputc(*p++, stderr); >> + continue; >> + } >> + sscanf(p, "\\x%x", &c); >> + fputc(c, stderr); >> + p += 4; >> + } >> + //fputs(strchr(igt_kmsg_capture_dump_buf, ';') + 1, stderr); >> + nlines++; >> + } while(errno == 0); >> + >> + if (nlines) >> + fprintf(stderr, "**** END ****\n"); >> + else >> + fprintf(stderr, "No kmsg.\n"); >> + >> + if (errno != EAGAIN) >> + fprintf(stderr, "Incomplete kmsg.\n"); >> + >> + pthread_mutex_unlock(&kmsg_mutex); >> +} >> + >> __attribute__((format(printf, 1, 2))) >> static void kmsg(const char *format, ...) >> #define KERN_EMER "<0>" >> @@ -330,11 +401,15 @@ static void kmsg(const char *format, ...) >> if (file == NULL) >> return; >> >> + pthread_mutex_lock(&kmsg_mutex); >> + >> va_start(ap, format); >> vfprintf(file, format, ap); >> va_end(ap); >> >> fclose(file); >> + >> + pthread_mutex_unlock(&kmsg_mutex); >> } >> >> static void gettime(struct timespec *ts) >> @@ -527,6 +602,15 @@ static int common_init(int *argc, char **argv, >> int ret = 0; >> const char *env; >> >> + igt_kmsg_capture_fd = open("/dev/kmsg", O_RDWR | O_NONBLOCK); >> + if (igt_kmsg_capture_fd != -1) { >> + igt_kmsg_capture_dump_buf = >> + malloc(IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); >> + if(igt_kmsg_capture_dump_buf == NULL) >> + igt_warn("Unable to allocate memory, " >> + "will not dump kmsg.\n"); >> + } >> + >> if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT")) >> __igt_plain_output = true; >> >> @@ -796,6 +880,7 @@ bool __igt_run_subtest(const char *subtest_name) >> igt_debug("Starting subtest: %s\n", subtest_name); >> >> _igt_log_buffer_reset(); >> + _igt_kmsg_capture_reset(); >> >> gettime(&subtest_time); >> return (in_subtest = subtest_name); >> @@ -972,6 +1057,7 @@ void igt_fail(int exitcode) >> exit(exitcode); >> >> _igt_log_buffer_dump(); >> + _igt_kmsg_capture_dump(); >> >> if (in_subtest) { >> if (exitcode == IGT_EXIT_TIMEOUT) >> @@ -1072,16 +1158,21 @@ void __igt_fail_assert(const char *domain, const char *file, const int line, >> */ >> void igt_exit(void) >> { >> + int status = IGT_EXIT_SUCCESS; >> + >> igt_exit_called = true; >> >> if (run_single_subtest && !run_single_subtest_found) { >> igt_warn("Unknown subtest: %s\n", run_single_subtest); >> - exit(IGT_EXIT_INVALID); >> + status = IGT_EXIT_INVALID; >> + goto do_exit; >> } >> >> >> - if (igt_only_list_subtests()) >> - exit(IGT_EXIT_SUCCESS); >> + if (igt_only_list_subtests()) { >> + status = IGT_EXIT_SUCCESS; >> + goto do_exit; >> + } >> >> kmsg(KERN_INFO "%s: exiting, ret=%d\n", command_str, igt_exitcode); >> igt_debug("Exiting with status code %d\n", igt_exitcode); >> @@ -1111,18 +1202,26 @@ void igt_exit(void) >> >> >> printf("%s (%.3fs)\n", result, elapsed); >> - exit(igt_exitcode); >> + status = igt_exitcode; >> + goto do_exit; >> } >> >> /* Calling this without calling one of the above is a failure */ >> assert(skipped_one || succeeded_one || failed_one); >> >> if (failed_one) >> - exit(igt_exitcode); >> + status = igt_exitcode; >> else if (succeeded_one) >> - exit(IGT_EXIT_SUCCESS); >> + status = IGT_EXIT_SUCCESS; >> else >> - exit(IGT_EXIT_SKIP); >> + status = IGT_EXIT_SKIP; >> +do_exit: >> + if (igt_kmsg_capture_fd != -1) >> + close(igt_kmsg_capture_fd); >> + if (igt_kmsg_capture_dump_buf != NULL) >> + free(igt_kmsg_capture_dump_buf); >> + >> + exit(status); >> } >> >> /* fork support code */ >> diff --git a/tests/Makefile.sources b/tests/Makefile.sources >> index 8fb2de8..25f0c4a 100644 >> --- a/tests/Makefile.sources >> +++ b/tests/Makefile.sources >> @@ -62,6 +62,7 @@ TESTS_progs_M = \ >> gem_tiled_partial_pwrite_pread \ >> gem_userptr_blits \ >> gem_write_read_ring_switch \ >> + igt_capture \ >> kms_addfb_basic \ >> kms_atomic \ >> kms_cursor_crc \ >> diff --git a/tests/igt_capture.c b/tests/igt_capture.c >> new file mode 100644 >> index 0000000..fd008d0 >> --- /dev/null >> +++ b/tests/igt_capture.c >> @@ -0,0 +1,93 @@ >> +/* >> + * Copyright © 2015 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. >> + * >> + * Authors: >> + * Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> + * >> + */ >> + >> +#include "igt.h" >> +#include <unistd.h> >> +#include <stdlib.h> >> +#include <stdio.h> >> +#include <string.h> >> +#include <fcntl.h> >> +#include <inttypes.h> >> +#include <errno.h> >> +#include <sys/stat.h> >> +#include <sys/ioctl.h> >> + >> +static FILE* kmsg; >> + >> +static void >> +test_kmsg(void) >> +{ >> + fputs("TEST (KMSG)\n", kmsg); >> + fflush(kmsg); >> + igt_fail(IGT_EXIT_FAILURE); >> +} >> + >> +static void >> +test_warn(void) >> +{ >> + igt_warn("TEST (WARN)\n"); >> + igt_fail(IGT_EXIT_FAILURE); >> +} >> + >> +static void >> +test_debug(void) >> +{ >> + igt_debug("TEST (DEBUG)\n"); >> + igt_fail(IGT_EXIT_FAILURE); >> +} >> + >> +static void >> +test_combined(void) >> +{ >> + igt_warn("TEST #1 (WARN)\n"); >> + fputs("TEST #1\n", kmsg); >> + igt_warn("TEST #2 (WARN)\n"); >> + fputs("TEST #2\n", kmsg); >> + fflush(kmsg); >> + igt_fail(IGT_EXIT_FAILURE); >> +} >> + >> +igt_main >> +{ >> + igt_fixture { >> + kmsg = fopen("/dev/kmsg", "w"); >> + igt_require(kmsg != NULL); >> + } >> + >> + igt_subtest("kmsg") >> + test_kmsg(); >> + igt_subtest("warn") >> + test_warn(); >> + igt_subtest("debug") >> + test_debug(); >> + igt_subtest("combined") >> + test_combined(); >> + >> + igt_fixture { >> + fclose(kmsg); >> + } >> +} >> -- >> 2.4.3 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On ke, 2015-11-18 at 17:41 +0000, Thomas Wood wrote: > On 18 November 2015 at 15:44, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote: > > > Cc: Thomas Wood <thomas.wood@intel.com> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Damien Lespiau <damien.lespiau@intel.com> > > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > > Given that we have all that in piglit already the commit message is > > a bit > > thin on justification. Why do we need this in igt too? How does > > this > > interact with the piglit dmesg capture? > In short, it makes IGT be able to run the tests on its own with only small additions to the current code, and the output can then be parsed with piglit outside of the DUT. I will add a canonical option to produce a CSV-like output. It's just about moving a small bit of the test running logic to IGT to make it produce meaningful output on its own. > There are a few tests that rely on checking dmesg to determine if the > test failed (such as gem_hangcheck_forcewake and > kms_flip_event_leak). > Although not implemented in this patch, it might be useful to use > this > feature to actually report a failure in those cases. > Next version makes any test fail if kernel domain messages with priority LOG_NOTICE or higher are output during test. Regards, Joonas > > > > > > --- > > > lib/igt_core.c | 113 > > > ++++++++++++++++++++++++++++++++++++++++++++++--- > > > tests/Makefile.sources | 1 + > > > tests/igt_capture.c | 93 > > > ++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 200 insertions(+), 7 deletions(-) > > > create mode 100644 tests/igt_capture.c > > > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c > > > index 04a0ab2..e73175a 100644 > > > --- a/lib/igt_core.c > > > +++ b/lib/igt_core.c > > > @@ -211,6 +211,8 @@ > > > * "--help" command line option. > > > */ > > > > > > +#define IGT_KMSG_CAPTURE_DUMP_BUF_SIZE 4096 > > > + > > > static unsigned int exit_handler_count; > > > const char *igt_interactive_debug; > > > > > > @@ -247,6 +249,10 @@ enum { > > > static int igt_exitcode = IGT_EXIT_SUCCESS; > > > static const char *command_str; > > > > > > +static int igt_kmsg_capture_fd; > > > +static char* igt_kmsg_capture_dump_buf; > > > +static pthread_mutex_t kmsg_mutex = PTHREAD_MUTEX_INITIALIZER; > > > + > > > static char* igt_log_domain_filter; > > > static struct { > > > char *entries[256]; > > > @@ -312,6 +318,71 @@ static void _igt_log_buffer_dump(void) > > > pthread_mutex_unlock(&log_buffer_mutex); > > > } > > > > > > +static void _igt_kmsg_capture_reset(void) > > > +{ > > > + if (igt_kmsg_capture_fd == -1) > > > + return; > > > + > > > + pthread_mutex_lock(&kmsg_mutex); > > > + > > > + lseek(igt_kmsg_capture_fd, 0, SEEK_END); > > > + > > > + pthread_mutex_unlock(&kmsg_mutex); > > > +} > > > + > > > +static void _igt_kmsg_capture_dump(void) > > > +{ > > > + size_t nbytes; > > > + int nlines; > > > + char *p; > > > + char *p0; > > > + int c; > > > + > > > + if (igt_kmsg_capture_fd == -1 || > > > + igt_kmsg_capture_dump_buf == NULL) > > > + return; > > > + > > > + pthread_mutex_lock(&kmsg_mutex); > > > + > > > + > > > + nlines = 0; > > > + do { > > > + errno = 0; > > > + nbytes = read(igt_kmsg_capture_fd, > > > + igt_kmsg_capture_dump_buf, > > > + IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); > > > + > > > + if (nbytes == -1) > > > + continue; > > > + > > > + if (!nlines) > > > + fprintf(stderr, "**** KMSG ****\n"); > > > + > > > + p = p0 = strchr(igt_kmsg_capture_dump_buf, ';') + > > > 1; > > > + while (p - p0 < nbytes) { > > > + if (*p != '\\') { > > > + fputc(*p++, stderr); > > > + continue; > > > + } > > > + sscanf(p, "\\x%x", &c); > > > + fputc(c, stderr); > > > + p += 4; > > > + } > > > + //fputs(strchr(igt_kmsg_capture_dump_buf, ';') + 1, > > > stderr); > > > + nlines++; > > > + } while(errno == 0); > > > + > > > + if (nlines) > > > + fprintf(stderr, "**** END ****\n"); > > > + else > > > + fprintf(stderr, "No kmsg.\n"); > > > + > > > + if (errno != EAGAIN) > > > + fprintf(stderr, "Incomplete kmsg.\n"); > > > + > > > + pthread_mutex_unlock(&kmsg_mutex); > > > +} > > > + > > > __attribute__((format(printf, 1, 2))) > > > static void kmsg(const char *format, ...) > > > #define KERN_EMER "<0>" > > > @@ -330,11 +401,15 @@ static void kmsg(const char *format, ...) > > > if (file == NULL) > > > return; > > > > > > + pthread_mutex_lock(&kmsg_mutex); > > > + > > > va_start(ap, format); > > > vfprintf(file, format, ap); > > > va_end(ap); > > > > > > fclose(file); > > > + > > > + pthread_mutex_unlock(&kmsg_mutex); > > > } > > > > > > static void gettime(struct timespec *ts) > > > @@ -527,6 +602,15 @@ static int common_init(int *argc, char > > > **argv, > > > int ret = 0; > > > const char *env; > > > > > > + igt_kmsg_capture_fd = open("/dev/kmsg", O_RDWR | > > > O_NONBLOCK); > > > + if (igt_kmsg_capture_fd != -1) { > > > + igt_kmsg_capture_dump_buf = > > > + malloc(IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); > > > + if(igt_kmsg_capture_dump_buf == NULL) > > > + igt_warn("Unable to allocate memory, " > > > + "will not dump kmsg.\n"); > > > + } > > > + > > > if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT")) > > > __igt_plain_output = true; > > > > > > @@ -796,6 +880,7 @@ bool __igt_run_subtest(const char > > > *subtest_name) > > > igt_debug("Starting subtest: %s\n", subtest_name); > > > > > > _igt_log_buffer_reset(); > > > + _igt_kmsg_capture_reset(); > > > > > > gettime(&subtest_time); > > > return (in_subtest = subtest_name); > > > @@ -972,6 +1057,7 @@ void igt_fail(int exitcode) > > > exit(exitcode); > > > > > > _igt_log_buffer_dump(); > > > + _igt_kmsg_capture_dump(); > > > > > > if (in_subtest) { > > > if (exitcode == IGT_EXIT_TIMEOUT) > > > @@ -1072,16 +1158,21 @@ void __igt_fail_assert(const char > > > *domain, const char *file, const int line, > > > */ > > > void igt_exit(void) > > > { > > > + int status = IGT_EXIT_SUCCESS; > > > + > > > igt_exit_called = true; > > > > > > if (run_single_subtest && !run_single_subtest_found) { > > > igt_warn("Unknown subtest: %s\n", > > > run_single_subtest); > > > - exit(IGT_EXIT_INVALID); > > > + status = IGT_EXIT_INVALID; > > > + goto do_exit; > > > } > > > > > > > > > - if (igt_only_list_subtests()) > > > - exit(IGT_EXIT_SUCCESS); > > > + if (igt_only_list_subtests()) { > > > + status = IGT_EXIT_SUCCESS; > > > + goto do_exit; > > > + } > > > > > > kmsg(KERN_INFO "%s: exiting, ret=%d\n", command_str, > > > igt_exitcode); > > > igt_debug("Exiting with status code %d\n", igt_exitcode); > > > @@ -1111,18 +1202,26 @@ void igt_exit(void) > > > > > > > > > printf("%s (%.3fs)\n", result, elapsed); > > > - exit(igt_exitcode); > > > + status = igt_exitcode; > > > + goto do_exit; > > > } > > > > > > /* Calling this without calling one of the above is a > > > failure */ > > > assert(skipped_one || succeeded_one || failed_one); > > > > > > if (failed_one) > > > - exit(igt_exitcode); > > > + status = igt_exitcode; > > > else if (succeeded_one) > > > - exit(IGT_EXIT_SUCCESS); > > > + status = IGT_EXIT_SUCCESS; > > > else > > > - exit(IGT_EXIT_SKIP); > > > + status = IGT_EXIT_SKIP; > > > +do_exit: > > > + if (igt_kmsg_capture_fd != -1) > > > + close(igt_kmsg_capture_fd); > > > + if (igt_kmsg_capture_dump_buf != NULL) > > > + free(igt_kmsg_capture_dump_buf); > > > + > > > + exit(status); > > > } > > > > > > /* fork support code */ > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > > > index 8fb2de8..25f0c4a 100644 > > > --- a/tests/Makefile.sources > > > +++ b/tests/Makefile.sources > > > @@ -62,6 +62,7 @@ TESTS_progs_M = \ > > > gem_tiled_partial_pwrite_pread \ > > > gem_userptr_blits \ > > > gem_write_read_ring_switch \ > > > + igt_capture \ > > > kms_addfb_basic \ > > > kms_atomic \ > > > kms_cursor_crc \ > > > diff --git a/tests/igt_capture.c b/tests/igt_capture.c > > > new file mode 100644 > > > index 0000000..fd008d0 > > > --- /dev/null > > > +++ b/tests/igt_capture.c > > > @@ -0,0 +1,93 @@ > > > +/* > > > + * Copyright © 2015 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. > > > + * > > > + * Authors: > > > + * Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > + * > > > + */ > > > + > > > +#include "igt.h" > > > +#include <unistd.h> > > > +#include <stdlib.h> > > > +#include <stdio.h> > > > +#include <string.h> > > > +#include <fcntl.h> > > > +#include <inttypes.h> > > > +#include <errno.h> > > > +#include <sys/stat.h> > > > +#include <sys/ioctl.h> > > > + > > > +static FILE* kmsg; > > > + > > > +static void > > > +test_kmsg(void) > > > +{ > > > + fputs("TEST (KMSG)\n", kmsg); > > > + fflush(kmsg); > > > + igt_fail(IGT_EXIT_FAILURE); > > > +} > > > + > > > +static void > > > +test_warn(void) > > > +{ > > > + igt_warn("TEST (WARN)\n"); > > > + igt_fail(IGT_EXIT_FAILURE); > > > +} > > > + > > > +static void > > > +test_debug(void) > > > +{ > > > + igt_debug("TEST (DEBUG)\n"); > > > + igt_fail(IGT_EXIT_FAILURE); > > > +} > > > + > > > +static void > > > +test_combined(void) > > > +{ > > > + igt_warn("TEST #1 (WARN)\n"); > > > + fputs("TEST #1\n", kmsg); > > > + igt_warn("TEST #2 (WARN)\n"); > > > + fputs("TEST #2\n", kmsg); > > > + fflush(kmsg); > > > + igt_fail(IGT_EXIT_FAILURE); > > > +} > > > + > > > +igt_main > > > +{ > > > + igt_fixture { > > > + kmsg = fopen("/dev/kmsg", "w"); > > > + igt_require(kmsg != NULL); > > > + } > > > + > > > + igt_subtest("kmsg") > > > + test_kmsg(); > > > + igt_subtest("warn") > > > + test_warn(); > > > + igt_subtest("debug") > > > + test_debug(); > > > + igt_subtest("combined") > > > + test_combined(); > > > + > > > + igt_fixture { > > > + fclose(kmsg); > > > + } > > > +} > > > -- > > > 2.4.3 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch
On Wed, Nov 18, 2015 at 05:32:59PM +0000, Chris Wilson wrote: > On Wed, Nov 18, 2015 at 04:44:20PM +0100, Daniel Vetter wrote: > > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote: > > > Cc: Thomas Wood <thomas.wood@intel.com> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Damien Lespiau <damien.lespiau@intel.com> > > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > > Given that we have all that in piglit already the commit message is a bit > > thin on justification. Why do we need this in igt too? How does this > > interact with the piglit dmesg capture? > > It's doesn't interfere with anyone else parsing kmsg/dmesg for > themselves, but it adds very useful functionality to standalone igt. > Which to me is significantly more valuable and I have been patching it > into igt for over a year and wished it was taken more seriously given > the number of incorrect bug reports generated. Ah, the "It doesn't interfere ..." is the crucial part I missed, I didn't know you could read dmesg in parallel without eating message for other consumers. Jonaas, with the above used as commit message (or something similar) this is Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cheers, Daniel
On Thu, Nov 19, 2015 at 10:38 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Nov 18, 2015 at 05:32:59PM +0000, Chris Wilson wrote: >> On Wed, Nov 18, 2015 at 04:44:20PM +0100, Daniel Vetter wrote: >> > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote: >> > > Cc: Thomas Wood <thomas.wood@intel.com> >> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> >> > > Cc: Damien Lespiau <damien.lespiau@intel.com> >> > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> > >> > Given that we have all that in piglit already the commit message is a bit >> > thin on justification. Why do we need this in igt too? How does this >> > interact with the piglit dmesg capture? >> >> It's doesn't interfere with anyone else parsing kmsg/dmesg for >> themselves, but it adds very useful functionality to standalone igt. >> Which to me is significantly more valuable and I have been patching it >> into igt for over a year and wished it was taken more seriously given >> the number of incorrect bug reports generated. > > Ah, the "It doesn't interfere ..." is the crucial part I missed, I didn't > know you could read dmesg in parallel without eating message for other > consumers. Jonaas, with the above used as commit message (or something > similar) this is Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Ok, I need to retract this. piglit does some dmesg filtering, how do we make sure these two definitions of what's considered failing dmesg noise match up? -Daniel
On to, 2015-11-19 at 10:41 +0100, Daniel Vetter wrote: > On Thu, Nov 19, 2015 at 10:38 AM, Daniel Vetter <daniel@ffwll.ch> > wrote: > > On Wed, Nov 18, 2015 at 05:32:59PM +0000, Chris Wilson wrote: > > > On Wed, Nov 18, 2015 at 04:44:20PM +0100, Daniel Vetter wrote: > > > > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen > > > > wrote: > > > > > Cc: Thomas Wood <thomas.wood@intel.com> > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > > > Cc: Damien Lespiau <damien.lespiau@intel.com> > > > > > Signed-off-by: Joonas Lahtinen < > > > > > joonas.lahtinen@linux.intel.com> > > > > > > > > Given that we have all that in piglit already the commit > > > > message is a bit > > > > thin on justification. Why do we need this in igt too? How does > > > > this > > > > interact with the piglit dmesg capture? > > > > > > It's doesn't interfere with anyone else parsing kmsg/dmesg for > > > themselves, but it adds very useful functionality to standalone > > > igt. > > > Which to me is significantly more valuable and I have been > > > patching it > > > into igt for over a year and wished it was taken more seriously > > > given > > > the number of incorrect bug reports generated. > > > > Ah, the "It doesn't interfere ..." is the crucial part I missed, I > > didn't > > know you could read dmesg in parallel without eating message for > > other > > consumers. Jonaas, with the above used as commit message (or > > something > > similar) this is Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Ok, I need to retract this. piglit does some dmesg filtering, how do > we make sure these two definitions of what's considered failing dmesg > noise match up? I would move that decision to I-G-T, and just let piglit interpret the FAIL (KMSG) status. Currently my proposal is that any LOG_NOTICE or higher priority message (in any facility) causes the test to fail. Regards, Joonas > -Daniel
On Fri, Nov 20, 2015 at 01:22:51PM +0200, Joonas Lahtinen wrote: > On to, 2015-11-19 at 10:41 +0100, Daniel Vetter wrote: > > On Thu, Nov 19, 2015 at 10:38 AM, Daniel Vetter <daniel@ffwll.ch> > > wrote: > > > On Wed, Nov 18, 2015 at 05:32:59PM +0000, Chris Wilson wrote: > > > > On Wed, Nov 18, 2015 at 04:44:20PM +0100, Daniel Vetter wrote: > > > > > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen > > > > > wrote: > > > > > > Cc: Thomas Wood <thomas.wood@intel.com> > > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > > > > Cc: Damien Lespiau <damien.lespiau@intel.com> > > > > > > Signed-off-by: Joonas Lahtinen < > > > > > > joonas.lahtinen@linux.intel.com> > > > > > > > > > > Given that we have all that in piglit already the commit > > > > > message is a bit > > > > > thin on justification. Why do we need this in igt too? How does > > > > > this > > > > > interact with the piglit dmesg capture? > > > > > > > > It's doesn't interfere with anyone else parsing kmsg/dmesg for > > > > themselves, but it adds very useful functionality to standalone > > > > igt. > > > > Which to me is significantly more valuable and I have been > > > > patching it > > > > into igt for over a year and wished it was taken more seriously > > > > given > > > > the number of incorrect bug reports generated. > > > > > > Ah, the "It doesn't interfere ..." is the crucial part I missed, I > > > didn't > > > know you could read dmesg in parallel without eating message for > > > other > > > consumers. Jonaas, with the above used as commit message (or > > > something > > > similar) this is Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Ok, I need to retract this. piglit does some dmesg filtering, how do > > we make sure these two definitions of what's considered failing dmesg > > noise match up? > > I would move that decision to I-G-T, and just let piglit interpret the > FAIL (KMSG) status. Currently my proposal is that any LOG_NOTICE or > higher priority message (in any facility) causes the test to fail. Not NOTICE. WARN or above, since NOTICE is a "normal but significant condition". I have been pushing for us to use NOTICE more effectively, many of our ERRORs can just be NOTICEs since we are able to take corrective action (and we expect to take such action). -Chris
On pe, 2015-11-20 at 11:34 +0000, Chris Wilson wrote: > On Fri, Nov 20, 2015 at 01:22:51PM +0200, Joonas Lahtinen wrote: > > On to, 2015-11-19 at 10:41 +0100, Daniel Vetter wrote: > > > On Thu, Nov 19, 2015 at 10:38 AM, Daniel Vetter <daniel@ffwll.ch> > > > wrote: > > > > On Wed, Nov 18, 2015 at 05:32:59PM +0000, Chris Wilson wrote: > > > > > On Wed, Nov 18, 2015 at 04:44:20PM +0100, Daniel Vetter > > > > > wrote: > > > > > > On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen > > > > > > wrote: > > > > > > > Cc: Thomas Wood <thomas.wood@intel.com> > > > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > > > > > Cc: Damien Lespiau <damien.lespiau@intel.com> > > > > > > > Signed-off-by: Joonas Lahtinen < > > > > > > > joonas.lahtinen@linux.intel.com> > > > > > > > > > > > > Given that we have all that in piglit already the commit > > > > > > message is a bit > > > > > > thin on justification. Why do we need this in igt too? How > > > > > > does > > > > > > this > > > > > > interact with the piglit dmesg capture? > > > > > > > > > > It's doesn't interfere with anyone else parsing kmsg/dmesg > > > > > for > > > > > themselves, but it adds very useful functionality to > > > > > standalone > > > > > igt. > > > > > Which to me is significantly more valuable and I have been > > > > > patching it > > > > > into igt for over a year and wished it was taken more > > > > > seriously > > > > > given > > > > > the number of incorrect bug reports generated. > > > > > > > > Ah, the "It doesn't interfere ..." is the crucial part I > > > > missed, I > > > > didn't > > > > know you could read dmesg in parallel without eating message > > > > for > > > > other > > > > consumers. Jonaas, with the above used as commit message (or > > > > something > > > > similar) this is Acked-by: Daniel Vetter < > > > > daniel.vetter@ffwll.ch> > > > > > > Ok, I need to retract this. piglit does some dmesg filtering, how > > > do > > > we make sure these two definitions of what's considered failing > > > dmesg > > > noise match up? > > > > I would move that decision to I-G-T, and just let piglit interpret > > the > > FAIL (KMSG) status. Currently my proposal is that any LOG_NOTICE or > > higher priority message (in any facility) causes the test to fail. > > Not NOTICE. WARN or above, since NOTICE is a "normal but significant > condition". I have been pushing for us to use NOTICE more > effectively, > many of our ERRORs can just be NOTICEs since we are able to take > corrective action (and we expect to take such action). > -Chris > We could have two FD's open on /dev/kmsg, we'd first scan out just the message priorities and then at convenient display time all the messages if the warn or above priority is in the log. It's not ideal, but doesn't require allocating arbitary amount of buffers. Regards, Joonas
diff --git a/lib/igt_core.c b/lib/igt_core.c index 04a0ab2..e73175a 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -211,6 +211,8 @@ * "--help" command line option. */ +#define IGT_KMSG_CAPTURE_DUMP_BUF_SIZE 4096 + static unsigned int exit_handler_count; const char *igt_interactive_debug; @@ -247,6 +249,10 @@ enum { static int igt_exitcode = IGT_EXIT_SUCCESS; static const char *command_str; +static int igt_kmsg_capture_fd; +static char* igt_kmsg_capture_dump_buf; +static pthread_mutex_t kmsg_mutex = PTHREAD_MUTEX_INITIALIZER; + static char* igt_log_domain_filter; static struct { char *entries[256]; @@ -312,6 +318,71 @@ static void _igt_log_buffer_dump(void) pthread_mutex_unlock(&log_buffer_mutex); } +static void _igt_kmsg_capture_reset(void) +{ + if (igt_kmsg_capture_fd == -1) + return; + + pthread_mutex_lock(&kmsg_mutex); + + lseek(igt_kmsg_capture_fd, 0, SEEK_END); + + pthread_mutex_unlock(&kmsg_mutex); +} + +static void _igt_kmsg_capture_dump(void) +{ + size_t nbytes; + int nlines; + char *p; + char *p0; + int c; + + if (igt_kmsg_capture_fd == -1 || + igt_kmsg_capture_dump_buf == NULL) + return; + + pthread_mutex_lock(&kmsg_mutex); + + + nlines = 0; + do { + errno = 0; + nbytes = read(igt_kmsg_capture_fd, + igt_kmsg_capture_dump_buf, + IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); + + if (nbytes == -1) + continue; + + if (!nlines) + fprintf(stderr, "**** KMSG ****\n"); + + p = p0 = strchr(igt_kmsg_capture_dump_buf, ';') + 1; + while (p - p0 < nbytes) { + if (*p != '\\') { + fputc(*p++, stderr); + continue; + } + sscanf(p, "\\x%x", &c); + fputc(c, stderr); + p += 4; + } + //fputs(strchr(igt_kmsg_capture_dump_buf, ';') + 1, stderr); + nlines++; + } while(errno == 0); + + if (nlines) + fprintf(stderr, "**** END ****\n"); + else + fprintf(stderr, "No kmsg.\n"); + + if (errno != EAGAIN) + fprintf(stderr, "Incomplete kmsg.\n"); + + pthread_mutex_unlock(&kmsg_mutex); +} + __attribute__((format(printf, 1, 2))) static void kmsg(const char *format, ...) #define KERN_EMER "<0>" @@ -330,11 +401,15 @@ static void kmsg(const char *format, ...) if (file == NULL) return; + pthread_mutex_lock(&kmsg_mutex); + va_start(ap, format); vfprintf(file, format, ap); va_end(ap); fclose(file); + + pthread_mutex_unlock(&kmsg_mutex); } static void gettime(struct timespec *ts) @@ -527,6 +602,15 @@ static int common_init(int *argc, char **argv, int ret = 0; const char *env; + igt_kmsg_capture_fd = open("/dev/kmsg", O_RDWR | O_NONBLOCK); + if (igt_kmsg_capture_fd != -1) { + igt_kmsg_capture_dump_buf = + malloc(IGT_KMSG_CAPTURE_DUMP_BUF_SIZE); + if(igt_kmsg_capture_dump_buf == NULL) + igt_warn("Unable to allocate memory, " + "will not dump kmsg.\n"); + } + if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT")) __igt_plain_output = true; @@ -796,6 +880,7 @@ bool __igt_run_subtest(const char *subtest_name) igt_debug("Starting subtest: %s\n", subtest_name); _igt_log_buffer_reset(); + _igt_kmsg_capture_reset(); gettime(&subtest_time); return (in_subtest = subtest_name); @@ -972,6 +1057,7 @@ void igt_fail(int exitcode) exit(exitcode); _igt_log_buffer_dump(); + _igt_kmsg_capture_dump(); if (in_subtest) { if (exitcode == IGT_EXIT_TIMEOUT) @@ -1072,16 +1158,21 @@ void __igt_fail_assert(const char *domain, const char *file, const int line, */ void igt_exit(void) { + int status = IGT_EXIT_SUCCESS; + igt_exit_called = true; if (run_single_subtest && !run_single_subtest_found) { igt_warn("Unknown subtest: %s\n", run_single_subtest); - exit(IGT_EXIT_INVALID); + status = IGT_EXIT_INVALID; + goto do_exit; } - if (igt_only_list_subtests()) - exit(IGT_EXIT_SUCCESS); + if (igt_only_list_subtests()) { + status = IGT_EXIT_SUCCESS; + goto do_exit; + } kmsg(KERN_INFO "%s: exiting, ret=%d\n", command_str, igt_exitcode); igt_debug("Exiting with status code %d\n", igt_exitcode); @@ -1111,18 +1202,26 @@ void igt_exit(void) printf("%s (%.3fs)\n", result, elapsed); - exit(igt_exitcode); + status = igt_exitcode; + goto do_exit; } /* Calling this without calling one of the above is a failure */ assert(skipped_one || succeeded_one || failed_one); if (failed_one) - exit(igt_exitcode); + status = igt_exitcode; else if (succeeded_one) - exit(IGT_EXIT_SUCCESS); + status = IGT_EXIT_SUCCESS; else - exit(IGT_EXIT_SKIP); + status = IGT_EXIT_SKIP; +do_exit: + if (igt_kmsg_capture_fd != -1) + close(igt_kmsg_capture_fd); + if (igt_kmsg_capture_dump_buf != NULL) + free(igt_kmsg_capture_dump_buf); + + exit(status); } /* fork support code */ diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 8fb2de8..25f0c4a 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -62,6 +62,7 @@ TESTS_progs_M = \ gem_tiled_partial_pwrite_pread \ gem_userptr_blits \ gem_write_read_ring_switch \ + igt_capture \ kms_addfb_basic \ kms_atomic \ kms_cursor_crc \ diff --git a/tests/igt_capture.c b/tests/igt_capture.c new file mode 100644 index 0000000..fd008d0 --- /dev/null +++ b/tests/igt_capture.c @@ -0,0 +1,93 @@ +/* + * Copyright © 2015 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. + * + * Authors: + * Joonas Lahtinen <joonas.lahtinen@linux.intel.com> + * + */ + +#include "igt.h" +#include <unistd.h> +#include <stdlib.h> +#include <stdio.h> +#include <string.h> +#include <fcntl.h> +#include <inttypes.h> +#include <errno.h> +#include <sys/stat.h> +#include <sys/ioctl.h> + +static FILE* kmsg; + +static void +test_kmsg(void) +{ + fputs("TEST (KMSG)\n", kmsg); + fflush(kmsg); + igt_fail(IGT_EXIT_FAILURE); +} + +static void +test_warn(void) +{ + igt_warn("TEST (WARN)\n"); + igt_fail(IGT_EXIT_FAILURE); +} + +static void +test_debug(void) +{ + igt_debug("TEST (DEBUG)\n"); + igt_fail(IGT_EXIT_FAILURE); +} + +static void +test_combined(void) +{ + igt_warn("TEST #1 (WARN)\n"); + fputs("TEST #1\n", kmsg); + igt_warn("TEST #2 (WARN)\n"); + fputs("TEST #2\n", kmsg); + fflush(kmsg); + igt_fail(IGT_EXIT_FAILURE); +} + +igt_main +{ + igt_fixture { + kmsg = fopen("/dev/kmsg", "w"); + igt_require(kmsg != NULL); + } + + igt_subtest("kmsg") + test_kmsg(); + igt_subtest("warn") + test_warn(); + igt_subtest("debug") + test_debug(); + igt_subtest("combined") + test_combined(); + + igt_fixture { + fclose(kmsg); + } +}
Cc: Thomas Wood <thomas.wood@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- lib/igt_core.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++--- tests/Makefile.sources | 1 + tests/igt_capture.c | 93 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 tests/igt_capture.c