On Thu, Nov 19, 2015 at 12:35:23PM +0200, Joonas Lahtinen wrote:
> +static void _igt_kmsg_capture_reset(void)
> +{
> + if (igt_kmsg_capture_fd != -1)
> + close(igt_kmsg_capture_fd);
> +
> + igt_kmsg_capture_fd = open("/dev/kmsg",
> + O_RDONLY | O_NONBLOCK);
We should O_CLOEXEC as well.
> +
> + if (igt_kmsg_capture_fd == -1)
> + return;
> +
> + lseek(igt_kmsg_capture_fd, 0, SEEK_END);
> +
> + if (igt_kmsg_capture_dump_buf == NULL)
> + 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 we allocate first, then bail, we know if we have the fd we have the
buffer as well.
> +}
> +
> +static int _igt_kmsg_capture_dump(bool notify_empty, int show_priority)
> +{
> + size_t nbytes;
> + int nlines;
> + int prefix;
> + int facility;
> + int priority;
> + char *p;
> + int c;
> +
> + if (igt_kmsg_capture_fd == -1 ||
> + igt_kmsg_capture_dump_buf == NULL)
> + return 0;
i.e. we can just do if (fd == -1) return 0;
> +
> + 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;
> +
> + sscanf(igt_kmsg_capture_dump_buf, "%d;", &prefix);
> + priority = prefix & 0x7;
> +
> + if (priority > show_priority)
> + continue;
> +
> + if (!nlines)
> + fprintf(stderr, "**** KMSG ****\n");
> +
> + p = strchr(igt_kmsg_capture_dump_buf, ';') + 1;
> + while (p - igt_kmsg_capture_dump_buf < nbytes) {
> + if (*p != '\\') {
> + fputc(*p++, stderr);
> + continue;
> + }
> + sscanf(p, "\\x%x", &c);
> + fputc(c, stderr);
> + p += 4;
Maybe:
/* Decode non-printable characters escaped by '\x01' */
int c = *p++;
if (c == '\\') {
if (p - igt_kmgs_capture_dump_buf > nbytes - 3)
break;
sscanf(p+1, "%x", &c);
p += 3;
}
fputc(c, stderr);
> + }
> + nlines++;
> + } while(errno == 0);
> +
> + if (nlines) {
> + fprintf(stderr, "**** END ****\n");
> + } else {
> + if (notify_empty)
> + fprintf(stderr, "No kmsg.\n");
> + }
> +
> + if (errno != EAGAIN)
> + fprintf(stderr, "Error: Incomplete kmsg!\n");
> +
> + close(igt_kmsg_capture_fd);
> + igt_kmsg_capture_fd = -1;
> +
> + free(igt_kmsg_capture_dump_buf);
> + igt_kmsg_capture_dump_buf = NULL;
Hmm, single-shot.
I have in mind more of an automatic debug feature coupled with error
detection like how we automatically go back and print the debug log if
the test fails. As I understand it, with a FAIL (KMSG) we currently lose
any lower priority output.
The integration looks good to me otherwise. But someone else will have
to vouch for test-runner/piglit handling of "FAIL (KMSG)" (I used WARN).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
