Re: [RFC][PATCH v8 2/3] trace,x86: add x86 irq vector tracepoints

[Steven Rostedt]

On Mon, 2013-01-21 at 22:14 +0000, Seiji Aguchi wrote:

> +void trace_irq_vector_regfunc(void)
> +{
> +	if (!trace_irq_vector_refcount) {
> +		smp_call_function(switch_to_trace_idt, NULL, 0);
> +		local_irq_disable();
> +		switch_to_trace_idt(NULL);
> +		local_irq_enable();
> +	}
> +	trace_irq_vector_refcount++;
> +}
> +
> +void trace_irq_vector_unregfunc(void)
> +{
> +	trace_irq_vector_refcount--;
> +	if (!trace_irq_vector_refcount) {
> +		smp_call_function(restore_original_idt, NULL, 0);
> +		local_irq_disable();
> +		restore_original_idt(NULL);
> +		local_irq_enable();
> +	}
> +}
> +

What protection do these functions have? I mean, the reg functions of a
TRACE_EVENT() can be initiated by perf and ftrace at he same time, as
well as LTTng (if someone adds it). The tracepoint itself may have
protection, but all the tracepoints you created use the same reg
functions, and they are not protected from each other.

You need to add a mutex around these, like:

static DEFINE_MUTEX(irq_reg_mutex);

void trace_irq_vector_regfunc(void)
{
	mutex_lock(&irq_reg_mutex);
	if (!trace_irq_vector_refcount) {
		smp_call_function(switch_to_trace_idt, NULL, 0);
		local_irq_disable();
		switch_to_trace_idt(NULL);
		local_irq_enable();
	}
	trace_irq_vector_refcount++;
	mutex_unlock(&irq_reg_mutex);
}

and the same for the trace_irq_vector_unregfunc().

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/