trace: Fix epoll hang when we race with new entries

Epoll on trace_pipe can sometimes hang in a weird case.  If the ring buffer is
empty when we set waiters_pending but an event shows up exactly at that moment
we can miss being woken up by the ring buffers irq work.  Since
ring_buffer_empty() is inherently racey we will sometimes think that the buffer
is not empty.  So we don't get woken up and we don't think there are any events
even though there were some ready when we added the watch, which makes us hang.
This patch fixes this by making sure that we are actually on the wait list
before we set waiters_pending, and add a memory barrier to make sure
ring_buffer_empty() is going to be correct.

Link: http://lkml.kernel.org/p/1408989581-23727-1-git-send-email-jbacik@fb.com
Cc: stable@vger.kernel.org # 3.10+
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index afb04b9b818a473f8b38e0aa94ee12ad84c44d1a..b38fb2b9e23772d3f3360ef556cf0714d4a44803 100644 (file)
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -626,8 +626,22 @@ int ring_buffer_poll_wait(struct ring_buffer *buffer, int cpu,
                work = &cpu_buffer->irq_work;
        }
 
-       work->waiters_pending = true;
        poll_wait(filp, &work->waiters, poll_table);
+       work->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
+        * is set, the next event will wake the task up, but we can get stuck
+        * if there's only a single event in.
+        *
+        * FIXME: Ideally, we need a memory barrier on the writer side as well,
+        * but adding a memory barrier to all events will cause too much of a
+        * performance hit in the fast path.  We only need a memory barrier when
+        * the buffer goes from empty to having content.  But as this race is
+        * extremely small, and it's not a problem if another event comes in, we
+        * will fix it later.
+        */
+       smp_mb();
 
        if ((cpu == RING_BUFFER_ALL_CPUS && !ring_buffer_empty(buffer)) ||
            (cpu != RING_BUFFER_ALL_CPUS && !ring_buffer_empty_cpu(buffer, cpu)))