Patch "ring-buffer: Fix full_waiters_pending in poll" has been added to the 6.6-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 full_waiters_pending in poll

to the 6.6-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-full_waiters_pending-in-poll.patch
and it can be found in the queue-6.6 subdirectory.

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



commit cda92181ef7e257c1cbaa196723dae5414aa59fa
Author: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
Date:   Tue Mar 12 09:19:20 2024 -0400

    ring-buffer: Fix full_waiters_pending in poll
    
    [ Upstream commit 8145f1c35fa648da662078efab299c4467b85ad5 ]
    
    If a reader of the ring buffer is doing a poll, and waiting for the ring
    buffer to hit a specific watermark, there could be a case where it gets
    into an infinite ping-pong loop.
    
    The poll code has:
    
      rbwork->full_waiters_pending = true;
      if (!cpu_buffer->shortest_full ||
          cpu_buffer->shortest_full > full)
             cpu_buffer->shortest_full = full;
    
    The writer will see full_waiters_pending and check if the ring buffer is
    filled over the percentage of the shortest_full value. If it is, it calls
    an irq_work to wake up all the waiters.
    
    But the code could get into a circular loop:
    
            CPU 0                                   CPU 1
            -----                                   -----
     [ Poll ]
       [ shortest_full = 0 ]
       rbwork->full_waiters_pending = true;
                                              if (rbwork->full_waiters_pending &&
                                                  [ buffer percent ] > shortest_full) {
                                                     rbwork->wakeup_full = true;
                                                     [ queue_irqwork ]
    
       cpu_buffer->shortest_full = full;
    
                                              [ IRQ work ]
                                              if (rbwork->wakeup_full) {
                                                    cpu_buffer->shortest_full = 0;
                                                    wakeup poll waiters;
      [woken]
       if ([ buffer percent ] > full)
          break;
       rbwork->full_waiters_pending = true;
                                              if (rbwork->full_waiters_pending &&
                                                  [ buffer percent ] > shortest_full) {
                                                     rbwork->wakeup_full = true;
                                                     [ queue_irqwork ]
    
       cpu_buffer->shortest_full = full;
    
                                              [ IRQ work ]
                                              if (rbwork->wakeup_full) {
                                                    cpu_buffer->shortest_full = 0;
                                                    wakeup poll waiters;
      [woken]
    
     [ Wash, rinse, repeat! ]
    
    In the poll, the shortest_full needs to be set before the
    full_pending_waiters, as once that is set, the writer will compare the
    current shortest_full (which is incorrect) to decide to call the irq_work,
    which will reset the shortest_full (expecting the readers to update it).
    
    Also move the setting of full_waiters_pending after the check if the ring
    buffer has the required percentage filled. There's no reason to tell the
    writer to wake up waiters if there are no waiters.
    
    Link: https://lore.kernel.org/linux-trace-kernel/20240312131952.630922155@xxxxxxxxxxx
    
    Cc: stable@xxxxxxxxxxxxxxx
    Cc: Mark Rutland <mark.rutland@xxxxxxx>
    Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
    Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
    Fixes: 42fb0a1e84ff5 ("tracing/ring-buffer: Have polling block on watermark")
    Reviewed-by: Masami Hiramatsu (Google) <mhiramat@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 a8ad5141d7ba3..b4979f6a449d7 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1112,16 +1112,32 @@ __poll_t ring_buffer_poll_wait(struct trace_buffer *buffer, int cpu,
 		poll_wait(filp, &rbwork->full_waiters, poll_table);
 
 		raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
-		rbwork->full_waiters_pending = true;
 		if (!cpu_buffer->shortest_full ||
 		    cpu_buffer->shortest_full > full)
 			cpu_buffer->shortest_full = full;
 		raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
-	} else {
-		poll_wait(filp, &rbwork->waiters, poll_table);
-		rbwork->waiters_pending = true;
+		if (full_hit(buffer, cpu, full))
+			return EPOLLIN | EPOLLRDNORM;
+		/*
+		 * Only allow full_waiters_pending update to be seen after
+		 * the shortest_full is set. If the writer sees the
+		 * full_waiters_pending flag set, it will compare the
+		 * amount in the ring buffer to shortest_full. If the amount
+		 * in the ring buffer is greater than the shortest_full
+		 * percent, it will call the irq_work handler to wake up
+		 * this list. The irq_handler will reset shortest_full
+		 * back to zero. That's done under the reader_lock, but
+		 * the below smp_mb() makes sure that the update to
+		 * full_waiters_pending doesn't leak up into the above.
+		 */
+		smp_mb();
+		rbwork->full_waiters_pending = true;
+		return 0;
 	}
 
+	poll_wait(filp, &rbwork->waiters, poll_table);
+	rbwork->waiters_pending = true;
+
 	/*
 	 * There's a tight race between setting the waiters_pending and
 	 * checking if the ring buffer is empty.  Once the waiters_pending bit
@@ -1137,9 +1153,6 @@ __poll_t ring_buffer_poll_wait(struct trace_buffer *buffer, int cpu,
 	 */
 	smp_mb();
 
-	if (full)
-		return full_hit(buffer, cpu, full) ? EPOLLIN | EPOLLRDNORM : 0;
-
 	if ((cpu == RING_BUFFER_ALL_CPUS && !ring_buffer_empty(buffer)) ||
 	    (cpu != RING_BUFFER_ALL_CPUS && !ring_buffer_empty_cpu(buffer, cpu)))
 		return EPOLLIN | EPOLLRDNORM;




[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