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.

[ Changelog inspired from Nikita Yushchenko's original patch. ]

Fixes #784

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

diff --git a/lib/ringbuffer/frontend_api.h b/lib/ringbuffer/frontend_api.h
index 31ccbfad020c5a1afa1b8570f1e6be0dd28438fa..ff6abce93d684a86e48066ed7fc537a2516fe9b4 100644 (file)
--- a/lib/ringbuffer/frontend_api.h
+++ b/lib/ringbuffer/frontend_api.h
@@ -298,8 +298,7 @@ void lib_ring_buffer_commit(const struct lib_ring_buffer_config *config,
         * 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);
+                       offset_end, commit_count);
 }
 
 /**

diff --git a/lib/ringbuffer/frontend_internal.h b/lib/ringbuffer/frontend_internal.h
index eda800de4e1fbaeb2aac47a720ba665269689021..47c080297ebd45ffd723de7c78caa6130fa672d3 100644 (file)
--- a/lib/ringbuffer/frontend_internal.h
+++ b/lib/ringbuffer/frontend_internal.h
@@ -421,23 +421,20 @@ void lib_ring_buffer_write_commit_counter(const struct lib_ring_buffer_config *c
                                          struct channel *chan,
                                          unsigned long idx,
                                          unsigned long buf_offset,
-                                         unsigned long commit_count,
-                                         size_t slot_size)
+                                         unsigned long commit_count)
 {
-       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 (unlikely(subbuf_offset(offset - commit_count, chan)))
+       if (unlikely(subbuf_offset(buf_offset - commit_count, chan)))
                return;
 
        commit_seq_old = v_read(config, &buf->commit_hot[idx].seq);

diff --git a/lib/ringbuffer/ring_buffer_frontend.c b/lib/ringbuffer/ring_buffer_frontend.c
index fc8d54139c446e0906b9b0560af3a2a568fa7992..86d5398a1e29b85ac97e68e23a3d6004af1fd015 100644 (file)
--- a/lib/ringbuffer/ring_buffer_frontend.c
+++ b/lib/ringbuffer/ring_buffer_frontend.c
@@ -1261,8 +1261,8 @@ void lib_ring_buffer_switch_old_start(struct lib_ring_buffer *buf,
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->old,
                                      commit_count, oldidx, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, oldidx,
-                                            offsets->old, commit_count,
-                                            config->cb.subbuffer_header_size());
+                       offsets->old + config->cb.subbuffer_header_size(),
+                       commit_count);
 }
 
 /*
@@ -1305,8 +1305,7 @@ void lib_ring_buffer_switch_old_end(struct lib_ring_buffer *buf,
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->old - 1,
                                      commit_count, oldidx, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, oldidx,
-                                            offsets->old, commit_count,
-                                            padding_size);
+                       offsets->old + padding_size, commit_count);
 }
 
 /*
@@ -1348,8 +1347,8 @@ void lib_ring_buffer_switch_new_start(struct lib_ring_buffer *buf,
        lib_ring_buffer_check_deliver(config, buf, chan, offsets->begin,
                                      commit_count, beginidx, tsc);
        lib_ring_buffer_write_commit_counter(config, buf, chan, beginidx,
-                                            offsets->begin, commit_count,
-                                            config->cb.subbuffer_header_size());
+                       offsets->begin + config->cb.subbuffer_header_size(),
+                       commit_count);
 }
 
 /*