Re: [PATCH 1/3] perf: Optimize perf_install_in_event()

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Oct 23, 2019 at 03:30:27PM +0300, Alexander Shishkin wrote:
> Peter Zijlstra <peterz@infradead.org> writes:
> 
> > +	/*
> > +	 * perf_event_attr::disabled events will not run and can be initialized
> > +	 * without IPI. Except when this is the first event for the context, in
> > +	 * that case we need the magic of the IPI to set ctx->is_active.
> > +	 *
> > +	 * The IOC_ENABLE that is sure to follow the creation of a disabled
> > +	 * event will issue the IPI and reprogram the hardware.
> > +	 */
> > +	if (__perf_effective_state(event) == PERF_EVENT_STATE_OFF && ctx->nr_events) {
> > +		raw_spin_lock_irq(&ctx->lock);
> > +		if (task && ctx->task == TASK_TOMBSTONE) {
> 
> Confused: isn't that redundant? If ctx->task reads TASK_TOMBSTONE, task
> is always !NULL,

The test is only relevant for task contexts, that's what the first
'task' clause tests for, then we need to check the ctx isn't dying,
which is the second clause 'ctx->task == TASK_TOMBSTONE'.

> afaict. And in any case, if a task context is going
> away, we shouldn't probably be adding events there. Or am I missing
> something?

Right, so in that case we exit early.