Skip to content

Commit 2d09328

Browse files
Zheng Yejianrostedt
authored andcommitted
ring-buffer: Fix wrong stat of cpu_buffer->read
When pages are removed in rb_remove_pages(), 'cpu_buffer->read' is set to 0 in order to make sure any read iterators reset themselves. However, this will mess 'entries' stating, see following steps: # cd /sys/kernel/tracing/ # 1. Enlarge ring buffer prepare for later reducing: # echo 20 > per_cpu/cpu0/buffer_size_kb # 2. Write a log into ring buffer of cpu0: # taskset -c 0 echo "hello1" > trace_marker # 3. Read the log: # cat per_cpu/cpu0/trace_pipe <...>-332 [000] ..... 62.406844: tracing_mark_write: hello1 # 4. Stop reading and see the stats, now 0 entries, and 1 event readed: # cat per_cpu/cpu0/stats entries: 0 [...] read events: 1 # 5. Reduce the ring buffer # echo 7 > per_cpu/cpu0/buffer_size_kb # 6. Now entries became unexpected 1 because actually no entries!!! # cat per_cpu/cpu0/stats entries: 1 [...] read events: 0 To fix it, introduce 'page_removed' field to count total removed pages since last reset, then use it to let read iterators reset themselves instead of changing the 'read' pointer. Link: https://lore.kernel.org/linux-trace-kernel/[email protected] Cc: <[email protected]> Cc: <[email protected]> Fixes: 83f4031 ("ring-buffer: Make removal of ring buffer pages atomic") Signed-off-by: Zheng Yejian <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 4b8b390 commit 2d09328

File tree

1 file changed

+12
-10
lines changed

1 file changed

+12
-10
lines changed

kernel/trace/ring_buffer.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,8 @@ struct ring_buffer_per_cpu {
523523
rb_time_t before_stamp;
524524
u64 event_stamp[MAX_NEST];
525525
u64 read_stamp;
526+
/* pages removed since last reset */
527+
unsigned long pages_removed;
526528
/* ring buffer pages to update, > 0 to add, < 0 to remove */
527529
long nr_pages_to_update;
528530
struct list_head new_pages; /* new pages to add */
@@ -559,6 +561,7 @@ struct ring_buffer_iter {
559561
struct buffer_page *head_page;
560562
struct buffer_page *cache_reader_page;
561563
unsigned long cache_read;
564+
unsigned long cache_pages_removed;
562565
u64 read_stamp;
563566
u64 page_stamp;
564567
struct ring_buffer_event *event;
@@ -1957,6 +1960,8 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
19571960
to_remove = rb_list_head(to_remove)->next;
19581961
head_bit |= (unsigned long)to_remove & RB_PAGE_HEAD;
19591962
}
1963+
/* Read iterators need to reset themselves when some pages removed */
1964+
cpu_buffer->pages_removed += nr_removed;
19601965

19611966
next_page = rb_list_head(to_remove)->next;
19621967

@@ -1978,12 +1983,6 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
19781983
cpu_buffer->head_page = list_entry(next_page,
19791984
struct buffer_page, list);
19801985

1981-
/*
1982-
* change read pointer to make sure any read iterators reset
1983-
* themselves
1984-
*/
1985-
cpu_buffer->read = 0;
1986-
19871986
/* pages are removed, resume tracing and then free the pages */
19881987
atomic_dec(&cpu_buffer->record_disabled);
19891988
raw_spin_unlock_irq(&cpu_buffer->reader_lock);
@@ -4395,6 +4394,7 @@ static void rb_iter_reset(struct ring_buffer_iter *iter)
43954394

43964395
iter->cache_reader_page = iter->head_page;
43974396
iter->cache_read = cpu_buffer->read;
4397+
iter->cache_pages_removed = cpu_buffer->pages_removed;
43984398

43994399
if (iter->head) {
44004400
iter->read_stamp = cpu_buffer->read_stamp;
@@ -4849,12 +4849,13 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
48494849
buffer = cpu_buffer->buffer;
48504850

48514851
/*
4852-
* Check if someone performed a consuming read to
4853-
* the buffer. A consuming read invalidates the iterator
4854-
* and we need to reset the iterator in this case.
4852+
* Check if someone performed a consuming read to the buffer
4853+
* or removed some pages from the buffer. In these cases,
4854+
* iterator was invalidated and we need to reset it.
48554855
*/
48564856
if (unlikely(iter->cache_read != cpu_buffer->read ||
4857-
iter->cache_reader_page != cpu_buffer->reader_page))
4857+
iter->cache_reader_page != cpu_buffer->reader_page ||
4858+
iter->cache_pages_removed != cpu_buffer->pages_removed))
48584859
rb_iter_reset(iter);
48594860

48604861
again:
@@ -5298,6 +5299,7 @@ rb_reset_cpu(struct ring_buffer_per_cpu *cpu_buffer)
52985299
cpu_buffer->last_overrun = 0;
52995300

53005301
rb_head_page_activate(cpu_buffer);
5302+
cpu_buffer->pages_removed = 0;
53015303
}
53025304

53035305
/* Must have disabled the cpu buffer then done a synchronize_rcu */

0 commit comments

Comments
 (0)