Fix: pass proper args when writing commit counter

lib_ring_buffer_write_commit_counter()'s 'buf_offset' argument should
contain offset of beginning of area used by the record being comitted.

However, lib_ring_buffer_commit() passes ctx->buf_offset, that gets
advanced by lib_ring_buffer_write() and thus points to just-after-
end-of-record at lib_ring_buffer_commit() time. This causes
lib_ring_buffer_write_commit_counter() to return without changing
commit_hot[idx].seq, due to

    if (unlikely(subbuf_offset(offset - commit_count, chan)))
            return;

Since after-crash data extraction tool checks 'seq' field to find out
how much data is in buffer, this results into inavailability of
data from partially-filled subbuffer for after-crash analysis.

This patch modifies lib_ring_buffer_write_commit_counter() and all its
callers to pass and expect the end of the area. So code works as it
should, and complete information becomes visible in crash dump.

[ Fix ported from lttng-modules. Changelog inspired from Nikita
Yushchenko's original patch. ]

Fixes #785

Reported-by: Nikita Yushchenko <nyoushchenko@mvista.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/libringbuffer/frontend_api.h b/libringbuffer/frontend_api.h
index f25ff0781930f72c050992573d061e7aa42c6e6b..56dbef2aa4326c8a4067ecc96ada66b887358afd 100644 (file)
--- a/libringbuffer/frontend_api.h
+++ b/libringbuffer/frontend_api.h
@@ -294,8 +294,7 @@ void lib_ring_buffer_commit(const struct lttng_ust_lib_ring_buffer_config *confi
         * ring_buffer buffers from vmcore, after crash.
         */
        lib_ring_buffer_write_commit_counter(config, buf, chan, endidx,
-                                            ctx->buf_offset, commit_count,
-                                            ctx->slot_size, handle);
+                       offset_end, commit_count, handle);
 }
 
 /**

diff --git a/libringbuffer/frontend_internal.h b/libringbuffer/frontend_internal.h
index 8a0f78f32ee774e19e853b3c7c511957ed7adb85..51b652d2a3d2ed5dbb9eb41d0942a522bd4c86a7 100644 (file)
--- a/libringbuffer/frontend_internal.h
+++ b/libringbuffer/frontend_internal.h
@@ -499,23 +499,20 @@ void lib_ring_buffer_write_commit_counter(const struct lttng_ust_lib_ring_buffer
                                          unsigned long idx,
                                          unsigned long buf_offset,
                                          unsigned long commit_count,
-                                         size_t slot_size,
                                          struct lttng_ust_shm_handle *handle)
 {
-       unsigned long offset, commit_seq_old;
+       unsigned long commit_seq_old;
 
        if (config->oops != RING_BUFFER_OOPS_CONSISTENCY)
                return;
 
-       offset = buf_offset + slot_size;
-
        /*
         * subbuf_offset includes commit_count_mask. We can simply
         * compare the offsets within the subbuffer without caring about
         * buffer full/empty mismatch because offset is never zero here
         * (subbuffer header and record headers have non-zero length).
         */
-       if (caa_unlikely(subbuf_offset(offset - commit_count, chan)))
+       if (caa_unlikely(subbuf_offset(buf_offset - commit_count, chan)))
                return;
 
        commit_seq_old = v_read(config, &shmp_index(handle, buf->commit_hot, idx)->seq);

diff --git a/libringbuffer/ring_buffer_frontend.c b/libringbuffer/ring_buffer_frontend.c
index 80a0c5f438c371aeca4e3d5fee0bf0853081f066..eb4e48629ca3d26a08b3c015b069a0e3c29f7d68 100644 (file)
--- a/libringbuffer/ring_buffer_frontend.c
+++ b/libringbuffer/ring_buffer_frontend.c
@@ -1335,9 +1335,8 @@ void lib_ring_buffer_switch_old_start(struct lttng_ust_lib_ring_buffer *buf,
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->old,
                                      commit_count, oldidx, handle, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, oldidx,
-                                            offsets->old, commit_count,
-                                            config->cb.subbuffer_header_size(),
-                                            handle);
+                       offsets->old + config->cb.subbuffer_header_size(),
+                       commit_count, handle);
 }
 
 /*
@@ -1374,8 +1373,7 @@ void lib_ring_buffer_switch_old_end(struct lttng_ust_lib_ring_buffer *buf,
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->old - 1,
                                      commit_count, oldidx, handle, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, oldidx,
-                                            offsets->old, commit_count,
-                                            padding_size, handle);
+                       offsets->old + padding_size, commit_count, handle);
 }
 
 /*
@@ -1410,9 +1408,8 @@ void lib_ring_buffer_switch_new_start(struct lttng_ust_lib_ring_buffer *buf,
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->begin,
                                      commit_count, beginidx, handle, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, beginidx,
-                                            offsets->begin, commit_count,
-                                            config->cb.subbuffer_header_size(),
-                                            handle);
+                       offsets->begin + config->cb.subbuffer_header_size(),
+                       commit_count, handle);
 }
 
 /*