Fix: eliminate timestamp overlap between packets

By using the timestamp sampled at space reservation when the packet is
being filled as "end timestamp" for a packet, we can ensure there is no
overlap between packet timestamp ranges, so that packet timestamp end <=
following packets timestamp begin.

Overlap between consecutive packets becomes an issue when the end
timestamp of a packet is greater than the end timestamp of a following
packet, IOW a packet completely contains the timestamp range of a
following packet.  This kind of situation does not allow trace viewers
to do binary search within the packet timestamps. This kind of situation
will typically never occur if packets are significantly larger than
event size, but this fix ensures it can never even theoretically happen.

The only case where packets can still theoretically overlap is if they
have equal begin and end timestamps, which is valid.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/libringbuffer/frontend_api.h b/libringbuffer/frontend_api.h
index a2a9af39aea777998c5d96cdde5f632f500c8cf7..f25ff0781930f72c050992573d061e7aa42c6e6b 100644 (file)
--- a/libringbuffer/frontend_api.h
+++ b/libringbuffer/frontend_api.h
@@ -288,7 +288,7 @@ void lib_ring_buffer_commit(const struct lttng_ust_lib_ring_buffer_config *confi
        commit_count = v_read(config, &shmp_index(handle, buf->commit_hot, endidx)->cc);
 
        lib_ring_buffer_check_deliver(config, buf, chan, offset_end - 1,
-                                     commit_count, endidx, handle);
+                                     commit_count, endidx, handle, ctx->tsc);
        /*
         * Update used size at each commit. It's needed only for extracting
         * ring_buffer buffers from vmcore, after crash.

diff --git a/libringbuffer/frontend_internal.h b/libringbuffer/frontend_internal.h
index 4ada183791c2a4f4253bb10baf1cff36b4d97214..8a0f78f32ee774e19e853b3c7c511957ed7adb85 100644 (file)
--- a/libringbuffer/frontend_internal.h
+++ b/libringbuffer/frontend_internal.h
@@ -365,6 +365,13 @@ void lib_ring_buffer_wakeup(struct lttng_ust_lib_ring_buffer *buf,
        }
 }
 
+/*
+ * Receive end of subbuffer TSC as parameter. It has been read in the
+ * space reservation loop of either reserve or switch, which ensures it
+ * progresses monotonically with event records in the buffer. Therefore,
+ * it ensures that the end timestamp of a subbuffer is <= begin
+ * timestamp of the following subbuffers.
+ */
 static inline
 void lib_ring_buffer_check_deliver(const struct lttng_ust_lib_ring_buffer_config *config,
                                   struct lttng_ust_lib_ring_buffer *buf,
@@ -372,11 +379,11 @@ void lib_ring_buffer_check_deliver(const struct lttng_ust_lib_ring_buffer_config
                                   unsigned long offset,
                                   unsigned long commit_count,
                                   unsigned long idx,
-                                  struct lttng_ust_shm_handle *handle)
+                                  struct lttng_ust_shm_handle *handle,
+                                  uint64_t tsc)
 {
        unsigned long old_commit_count = commit_count
                                         - chan->backend.subbuf_size;
-       uint64_t tsc;
 
        /* Check if all commits have been done */
        if (caa_unlikely((buf_trunc(offset, chan) >> chan->backend.num_subbuf_order)
@@ -422,7 +429,6 @@ void lib_ring_buffer_check_deliver(const struct lttng_ust_lib_ring_buffer_config
                         * and any other writer trying to access this subbuffer
                         * in this state is required to drop records.
                         */
-                       tsc = config->cb.ring_buffer_clock_read(chan);
                        v_add(config,
                              subbuffer_get_records_count(config,
                                                          &buf->backend,

diff --git a/libringbuffer/ring_buffer_frontend.c b/libringbuffer/ring_buffer_frontend.c
index e99ba8a5942a77084b740f75ef80fc0781b89ed3..80a0c5f438c371aeca4e3d5fee0bf0853081f066 100644 (file)
--- a/libringbuffer/ring_buffer_frontend.c
+++ b/libringbuffer/ring_buffer_frontend.c
@@ -1333,7 +1333,7 @@ void lib_ring_buffer_switch_old_start(struct lttng_ust_lib_ring_buffer *buf,
        commit_count = v_read(config, &shmp_index(handle, buf->commit_hot, oldidx)->cc);
        /* Check if the written buffer has to be delivered */
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->old,
-                                     commit_count, oldidx, handle);
+                                     commit_count, oldidx, handle, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, oldidx,
                                             offsets->old, commit_count,
                                             config->cb.subbuffer_header_size(),
@@ -1372,7 +1372,7 @@ void lib_ring_buffer_switch_old_end(struct lttng_ust_lib_ring_buffer *buf,
        v_add(config, padding_size, &shmp_index(handle, buf->commit_hot, oldidx)->cc);
        commit_count = v_read(config, &shmp_index(handle, buf->commit_hot, oldidx)->cc);
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->old - 1,
-                                     commit_count, oldidx, handle);
+                                     commit_count, oldidx, handle, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, oldidx,
                                             offsets->old, commit_count,
                                             padding_size, handle);
@@ -1408,7 +1408,7 @@ void lib_ring_buffer_switch_new_start(struct lttng_ust_lib_ring_buffer *buf,
        commit_count = v_read(config, &shmp_index(handle, buf->commit_hot, beginidx)->cc);
        /* Check if the written buffer has to be delivered */
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->begin,
-                                     commit_count, beginidx, handle);
+                                     commit_count, beginidx, handle, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, beginidx,
                                             offsets->begin, commit_count,
                                             config->cb.subbuffer_header_size(),