Patch "ring-buffer: Fix wrong stat of cpu_buffer->read" has been added to the 5.4-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    ring-buffer: Fix wrong stat of cpu_buffer->read

to the 5.4-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     ring-buffer-fix-wrong-stat-of-cpu_buffer-read.patch
and it can be found in the queue-5.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit f405630a7f0d80304dbf43eaaec1a4fdc84a0a7a
Author: Zheng Yejian <zhengyejian1@xxxxxxxxxx>
Date:   Mon Jul 24 13:40:40 2023 +0800

    ring-buffer: Fix wrong stat of cpu_buffer->read
    
    [ Upstream commit 2d093282b0d4357373497f65db6a05eb0c28b7c8 ]
    
    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/20230724054040.3489499-1-zhengyejian1@xxxxxxxxxx
    
    Cc: <mhiramat@xxxxxxxxxx>
    Cc: <vnagarnaik@xxxxxxxxxx>
    Fixes: 83f40318dab0 ("ring-buffer: Make removal of ring buffer pages atomic")
    Signed-off-by: Zheng Yejian <zhengyejian1@xxxxxxxxxx>
    Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index afd7f3a51485e..445475c229b3a 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -485,6 +485,8 @@ struct ring_buffer_per_cpu {
 	unsigned long			read_bytes;
 	u64				write_stamp;
 	u64				read_stamp;
+	/* pages removed since last reset */
+	unsigned long			pages_removed;
 	/* ring buffer pages to update, > 0 to add, < 0 to remove */
 	long				nr_pages_to_update;
 	struct list_head		new_pages; /* new pages to add */
@@ -520,6 +522,7 @@ struct ring_buffer_iter {
 	struct buffer_page		*head_page;
 	struct buffer_page		*cache_reader_page;
 	unsigned long			cache_read;
+	unsigned long			cache_pages_removed;
 	u64				read_stamp;
 };
 
@@ -1581,6 +1584,8 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
 		to_remove = rb_list_head(to_remove)->next;
 		head_bit |= (unsigned long)to_remove & RB_PAGE_HEAD;
 	}
+	/* Read iterators need to reset themselves when some pages removed */
+	cpu_buffer->pages_removed += nr_removed;
 
 	next_page = rb_list_head(to_remove)->next;
 
@@ -1602,12 +1607,6 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
 		cpu_buffer->head_page = list_entry(next_page,
 						struct buffer_page, list);
 
-	/*
-	 * change read pointer to make sure any read iterators reset
-	 * themselves
-	 */
-	cpu_buffer->read = 0;
-
 	/* pages are removed, resume tracing and then free the pages */
 	atomic_dec(&cpu_buffer->record_disabled);
 	raw_spin_unlock_irq(&cpu_buffer->reader_lock);
@@ -3659,6 +3658,7 @@ static void rb_iter_reset(struct ring_buffer_iter *iter)
 
 	iter->cache_reader_page = iter->head_page;
 	iter->cache_read = cpu_buffer->read;
+	iter->cache_pages_removed = cpu_buffer->pages_removed;
 
 	if (iter->head)
 		iter->read_stamp = cpu_buffer->read_stamp;
@@ -4101,12 +4101,13 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
 	buffer = cpu_buffer->buffer;
 
 	/*
-	 * Check if someone performed a consuming read to
-	 * the buffer. A consuming read invalidates the iterator
-	 * and we need to reset the iterator in this case.
+	 * Check if someone performed a consuming read to the buffer
+	 * or removed some pages from the buffer. In these cases,
+	 * iterator was invalidated and we need to reset it.
 	 */
 	if (unlikely(iter->cache_read != cpu_buffer->read ||
-		     iter->cache_reader_page != cpu_buffer->reader_page))
+		     iter->cache_reader_page != cpu_buffer->reader_page ||
+		     iter->cache_pages_removed != cpu_buffer->pages_removed))
 		rb_iter_reset(iter);
 
  again:
@@ -4538,6 +4539,7 @@ rb_reset_cpu(struct ring_buffer_per_cpu *cpu_buffer)
 	cpu_buffer->last_overrun = 0;
 
 	rb_head_page_activate(cpu_buffer);
+	cpu_buffer->pages_removed = 0;
 }
 
 /**



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux