[Skiboot] [PATCH v2 12/16] external/trace: mmap trace buffers in dump_trace

Jordan Niethe jniethe5 at gmail.com
Tue Apr 2 10:43:23 AEDT 2019


The current lseek/read approach used in dump_trace does not correctly
handle certain aspects of the buffers. It does not use the start and end
position that is part of the buffer so it will not begin from the
correct location. It does not move back to the beginning of the trace
buffer file as the buffer wraps around. It also does not handle the
overflow case of the writer overwriting when the reader is up to.

Mmap the trace buffer file so that the existing reading functions in
extra/trace.c can be used. These functions already handle the cases of
wrapping and overflow.  This reduces code duplication and uses functions
that are already unit tested. However this requires a kernel where the
trace buffer sysfs nodes are able to be mmaped (see
https://patchwork.ozlabs.org/patch/1056786/)

---
v2: use fstat to determine size to mmap

Signed-off-by: Jordan Niethe <jniethe5 at gmail.com>
---
 external/trace/Makefile     |  2 +-
 external/trace/dump_trace.c | 55 ++++++++++++++++---------------------
 2 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/external/trace/Makefile b/external/trace/Makefile
index 3828fea534ea..bff52f3a6ab2 100644
--- a/external/trace/Makefile
+++ b/external/trace/Makefile
@@ -1,7 +1,7 @@
 HOSTEND=$(shell uname -m | sed -e 's/^i.*86$$/LITTLE/' -e 's/^x86.*/LITTLE/' -e 's/^ppc64le/LITTLE/' -e 's/^ppc.*/BIG/')
 CFLAGS=-g -Wall -DHAVE_$(HOSTEND)_ENDIAN -I../../include -I../../
 
-dump_trace: dump_trace.c
+dump_trace: dump_trace.c trace.c
 
 clean:
 	rm -f dump_trace *.o
diff --git a/external/trace/dump_trace.c b/external/trace/dump_trace.c
index 4779dc42ec6d..650cd7a013f9 100644
--- a/external/trace/dump_trace.c
+++ b/external/trace/dump_trace.c
@@ -14,42 +14,24 @@
  * limitations under the License.
  */
 
+#include <trace.h>
 #include <err.h>
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/mman.h>
 #include <fcntl.h>
 #include <string.h>
 #include <inttypes.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <unistd.h>
+#include <stdlib.h>
 
 #include "../../ccan/endian/endian.h"
 #include "../../ccan/short_types/short_types.h"
-#include <trace_types.h>
+#include "trace.h"
 
-/* Handles trace from debugfs (one record at a time) or file */ 
-static bool get_trace(int fd, union trace *t, int *len)
-{
-	void *dest = t;
-	int r;
-
-	/* Move down any extra we read last time. */
-	if (*len >= sizeof(t->hdr) && *len >= t->hdr.len_div_8 * 8) {
-		u8 rlen = t->hdr.len_div_8 * 8;
-		memmove(dest, dest + rlen, *len - rlen);
-		*len -= rlen;
-	}
-
-	r = read(fd, dest + *len, sizeof(*t) - *len);
-	if (r < 0)
-		return false;
-
-	*len += r;
-	/* We should have a complete record. */
-	return *len >= sizeof(t->hdr) && *len >= t->hdr.len_div_8 * 8;
-}
 
 static void display_header(const struct trace_hdr *h)
 {
@@ -152,20 +134,31 @@ static void dump_uart(struct trace_uart *t)
 
 int main(int argc, char *argv[])
 {
-	int fd, len = 0;
+	struct trace_reader tr;
+	struct trace_info *ti;
+	struct stat sb;
 	union trace t;
-	const char *in = "/sys/kernel/debug/powerpc/opal-trace";
+	int fd;
 
-	if (argc > 2)
-		errx(1, "Usage: dump_trace [file]");
+	if (argc != 2)
+		errx(1, "Usage: dump_trace file");
 
-	if (argv[1])
-		in = argv[1];
-	fd = open(in, O_RDONLY);
+	fd = open(argv[1], O_RDONLY);
 	if (fd < 0)
-		err(1, "Opening %s", in);
+		err(1, "Opening %s", argv[1]);
+
+	if (fstat(fd, &sb) < 0)
+		err(1, "Stating %s", argv[1]);
+
+	ti = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (ti == MAP_FAILED)
+		err(1, "Mmaping %s", argv[1]);
+
+	memset(&tr, 0, sizeof(struct trace_reader));
+	tr.tb = &ti->tb;
+
 
-	while (get_trace(fd, &t, &len)) {
+	while (trace_get(&t, &tr)) {
 		display_header(&t.hdr);
 		switch (t.hdr.type) {
 		case TRACE_REPEAT:
-- 
2.20.1



More information about the Skiboot mailing list