Messages in this thread | | | Date | Tue, 22 Nov 2011 14:15:30 +0100 | Subject | Re: [PATCH] perf_event: fix loss of notification with multi-event sampling | From | Stephane Eranian <> |
| |
On Tue, Nov 22, 2011 at 11:40 AM, Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, 2011-11-16 at 15:53 +0100, Stephane Eranian wrote: >> + /* ring_buffer waitq pointer */ >> + wait_queue_head_t *waitq; > > Not a big issue, but is there a reason to keep this pointer instead of > always having to do: > > rcu_read_lock(); > rb = rcu_dereference(event->rb); > if (rb) > wake_up_all(rb->waitq); > rcu_read_unlock(); > > Hmm, looking at that there must be a reason we go through all the RCU > trouble for event->rb, assuming there is, your lack of rcu in say > perf_poll() could go funny. > I think we go drop waitq and go through event->rb each time we need to get to the wait queue.
> /me ponders.. > > Ah, could it be a race of poll()/wakeup() vs perf_event_set_output() ? > Are you saying that by dropping event->waitq in favor of event->rb->waitq we make this problem disappear due to rcu protections?
Poll_wait() is a blocking call. It may wait on a stale waitq. But that problem was probably already there. I am not clear as to what to do about that. in perf_set_output() you would need to wakeup from poll_wait() and then go back in with the new waitq.
Similarly, I am not clear as to what happens when you close an event for which you have a waiter in poll_wait(). I assume you wakeup from it. But I don't see where that's implemented.
> Suppose you're a threaded proglet and either one cpu/thread has an > incoming event that does a wakeup, or one thread is stuck in poll() > whilst another thread does perf_event_set_output(), it could swizzle the > event->rb right out from under you. > > Now, this is of course a somewhat silly thing to do.. but still it > shouldn't make things go *bang*. > > Now the above wake_up_all() thing would work just fine, its just poll() > that I'm not sure how to fix. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |