• Steven Rostedt (Google)'s avatar
    ring-buffer: Fix full_waiters_pending in poll · 8145f1c3
    Steven Rostedt (Google) authored
    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@goodmis.org
    
    Cc: stable@vger.kernel.org
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Fixes: 42fb0a1e ("tracing/ring-buffer: Have polling block on watermark")
    Reviewed-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
    Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
    8145f1c3
ring_buffer.c 171 KB