Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1303722

Re: [tip:perf/core] perf: Fix race in perf_event_exec()

Show all headers | View raw

On Thu, Jan 07, 2016 at 02:40:08PM +0100, Peter Zijlstra wrote:
> On Wed, Jan 06, 2016 at 01:56:56PM -0500, Eric Dumazet wrote:
> > On Wed, Jan 6, 2016 at 1:46 PM, tip-bot for Peter Zijlstra
> > <tipbot@zytor.com> wrote:
> > 
> > >
> > > This is because context switches can swap the task_struct::perf_event_ctxp[]
> > > pointer around. Therefore you have to either disable preemption when looking
> > > at current, or hold ctx->lock.
> > >
> > 
> > 
> > >
> > >  void perf_event_exec(void)
> > >  {
> > > -       struct perf_event_context *ctx;
> > >         int ctxn;
> > >
> > >         rcu_read_lock();
> > 
> > Do we still need this rcu_read_lock(), if perf_event_enable_on_exec()
> > uses  local_irq_save( ?
> 
> Strictly speaking we should not rely on the fact that RCU grace periods
> do not progress with IRQs disabled, so yes.

What Peter said!

The current implementation would let you get away with IRQs disabled
(aside from lockdep splats), but it is easy to imagine implementations
that do not, hence the restriction.

Is the extra rcu_read_lock() and rcu_read_unlock()  causing a performance
problem?  If so, one option would be use of synchronize_sched() and
call_rcu_sched(), which explicitly recognize disabled IRQs as read-side
critical sections.  Of course, this might well put you in a position
where you would like to use IRQ disabling in some cases, BH disabling
(or softirq context) in others, and rcu_read_lock() in yet other cases.

If so, the good news is that RCU actually now supports this efficiently.
There is the shiny new synchronize_rcu_mult() macro that can be used
as follows to wait for all three types of grace periods:

	synchronize_rcu_mult(call_rcu, call_rcu_bh, call_rcu_sched);

This would wait concurrently for all three grace periods to elapse.

							Thanx, Paul

--
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/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[tip:perf/core] perf: Fix race in perf_event_exec() tip-bot for Peter Zijlstra <tipbot@zytor.com> - 2016-01-06 19:50 +0100
  Re: [tip:perf/core] perf: Fix race in perf_event_exec() Eric Dumazet <edumazet@google.com> - 2016-01-06 20:00 +0100
    Re: [tip:perf/core] perf: Fix race in perf_event_exec() Peter Zijlstra <peterz@infradead.org> - 2016-01-07 14:50 +0100
      Re: [tip:perf/core] perf: Fix race in perf_event_exec() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-01-07 17:30 +0100
        Re: [tip:perf/core] perf: Fix race in perf_event_exec() Eric Dumazet <edumazet@google.com> - 2016-01-07 17:40 +0100
          Re: [tip:perf/core] perf: Fix race in perf_event_exec() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-01-07 17:50 +0100

csiph-web