external/trace: Introduce structure for reading traces

Currently the trace_get and trace_empty functions operate on a tracebuf
struct. This requires being able to write to that struct.  If dump_trace
were able to use these functions it would be convenient as it would
reduce code duplication and these functions are already unit tested.
However, a tracebuf accessed via mmaping will not be able to be written.

The tracebuf struct fields that need to be written are only to be used
by a reader.  The fields are never used by a writer. Add a new structure
for readers, trace_reader,  which contains these fields and remove them
from the tracebuf struct.  Change trace_get and trace_empty to use the
trace_reader struct and update the unit tests accordingly.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
Signed-off-by: Stewart Smith <stewart@linux.ibm.com>

2 files changed, 48 insertions, 23 deletions

diff --git a/external/trace/trace.c b/external/trace/trace.c
index bb5a9bf..aa9fa15 100644
--- a/external/trace/trace.c
+++ b/external/trace/trace.c
@@ -17,14 +17,21 @@
 #include <external/trace/trace.h>
 #include "../ccan/endian/endian.h"
 #include "../ccan/short_types/short_types.h"
+#include "trace.h"
 #include <trace_types.h>
 #include <errno.h>
 
-bool trace_empty(const struct tracebuf *tb)
+#if defined(__powerpc__) || defined(__powerpc64__)
+#define rmb() lwsync()
+#else
+#define rmb()
+#endif
+
+bool trace_empty(const struct trace_reader *tr)
 {
 	const struct trace_repeat *rep;
 
-	if (tb->rpos == tb->end)
+	if (tr->rpos == be64_to_cpu(tr->tb->end))
 		return true;
 
 	/*
@@ -32,29 +39,29 @@ bool trace_empty(const struct tracebuf *tb)
 	 * we've already seen every repeat for (yet which may be
 	 * incremented in future), we're also empty.
 	 */
-	rep = (void *)tb->buf + be64_to_cpu(tb->rpos) % be64_to_cpu(tb->buf_size);
-	if (be64_to_cpu(tb->end) != be64_to_cpu(tb->rpos) + sizeof(*rep))
+	rep = (void *)tr->tb->buf + tr->rpos % be64_to_cpu(tr->tb->buf_size);
+	if (be64_to_cpu(tr->tb->end) != tr->rpos + sizeof(*rep))
 		return false;
 
 	if (rep->type != TRACE_REPEAT)
 		return false;
 
-	if (be16_to_cpu(rep->num) != be32_to_cpu(tb->last_repeat))
+	if (be16_to_cpu(rep->num) != tr->last_repeat)
 		return false;
 
 	return true;
 }
 
 /* You can't read in parallel, so some locking required in caller. */
-bool trace_get(union trace *t, struct tracebuf *tb)
+bool trace_get(union trace *t, struct trace_reader *tr)
 {
 	u64 start, rpos;
 	size_t len;
 
-	len = sizeof(*t) < be32_to_cpu(tb->max_size) ? sizeof(*t) :
-		be32_to_cpu(tb->max_size);
+	len = sizeof(*t) < be32_to_cpu(tr->tb->max_size) ? sizeof(*t) :
+		be32_to_cpu(tr->tb->max_size);
 
-	if (trace_empty(tb))
+	if (trace_empty(tr))
 		return false;
 
 again:
@@ -62,12 +69,12 @@ again:
 	 * The actual buffer is slightly larger than tbsize, so this
 	 * memcpy is always valid.
 	 */
-	memcpy(t, tb->buf + be64_to_cpu(tb->rpos) % be64_to_cpu(tb->buf_size), len);
+	memcpy(t, tr->tb->buf + tr->rpos % be64_to_cpu(tr->tb->buf_size), len);
 
-	rmb(); /* read barrier, so we read tb->start after copying record. */
+	rmb(); /* read barrier, so we read tr->tb->start after copying record. */
 
-	start = be64_to_cpu(tb->start);
-	rpos = be64_to_cpu(tb->rpos);
+	start = be64_to_cpu(tr->tb->start);
+	rpos = tr->rpos;
 
 	/* Now, was that overwritten? */
 	if (rpos < start) {
@@ -76,7 +83,7 @@ again:
 		t->overflow.type = TRACE_OVERFLOW;
 		t->overflow.len_div_8 = sizeof(t->overflow) / 8;
 		t->overflow.bytes_missed = cpu_to_be64(start - rpos);
-		tb->rpos = cpu_to_be64(start);
+		tr->rpos = start;
 		return true;
 	}
 
@@ -85,10 +92,10 @@ again:
 		u32 num = be16_to_cpu(t->repeat.num);
 
 		/* In case we've read some already... */
-		t->repeat.num = cpu_to_be16(num - be32_to_cpu(tb->last_repeat));
+		t->repeat.num = cpu_to_be16(num - tr->last_repeat);
 
 		/* Record how many repeats we saw this time. */
-		tb->last_repeat = cpu_to_be32(num);
+		tr->last_repeat = num;
 
 		/* Don't report an empty repeat buffer. */
 		if (t->repeat.num == 0) {
@@ -96,16 +103,16 @@ again:
 			 * This can't be the last buffer, otherwise
 			 * trace_empty would have returned true.
 			 */
-			assert(be64_to_cpu(tb->end) >
+			assert(be64_to_cpu(tr->tb->end) >
 			       rpos + t->hdr.len_div_8 * 8);
 			/* Skip to next entry. */
-			tb->rpos = cpu_to_be64(rpos + t->hdr.len_div_8 * 8);
-			tb->last_repeat = 0;
+			tr->rpos = rpos + t->hdr.len_div_8 * 8;
+			tr->last_repeat = 0;
 			goto again;
 		}
 	} else {
-		tb->last_repeat = 0;
-		tb->rpos = cpu_to_be64(rpos + t->hdr.len_div_8 * 8);
+		tr->last_repeat = 0;
+		tr->rpos = rpos + t->hdr.len_div_8 * 8;
 	}
 
 	return true;
diff --git a/external/trace/trace.h b/external/trace/trace.h
index 4d2dbc7..59ff8a6 100644
--- a/external/trace/trace.h
+++ b/external/trace/trace.h
@@ -13,8 +13,26 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#ifndef E_TRACE_H
+#define E_TRACE_H
+
+#include <stdbool.h>
+#include <types.h>
+#include <trace.h>
+#include <trace_types.h>
+
+struct trace_reader {
+	/* This is where the reader is up to. */
+	u64 rpos;
+	/* If the last one we read was a repeat, this shows how many. */
+	u32 last_repeat;
+	struct tracebuf *tb;
+};
+
 /* Is this tracebuf empty? */
-bool trace_empty(const struct tracebuf *tracebuf);
+bool trace_empty(const struct trace_reader *tr);
 
 /* Get the next trace from this buffer (false if empty). */
-bool trace_get(union trace *t, struct tracebuf *tb);
+bool trace_get(union trace *t, struct trace_reader *tr);
+
+#endif