preemptirqsoff tracer

preemptirqsoff tracer

[Joel Fernandes]

Hi Steven,

Just looking at code of the preemptirqsoff tracer, I wonder if it is
really working as expected.

With the tracer enabled, start_critical_timing will be called only if
_both_ preemption and IRQs are turned off. Instead shouldn't this be
an OR condition (either preemption or IRQs are turned off)?

>From the example in ftrace.txt documentation:
    local_irq_disable();
    call_function_with_irqs_off();
    preempt_disable();
    call_function_with_irqs_and_preemption_off();
    local_irq_enable();
    call_function_with_preemption_off();
    preempt_enable();

Looks to me like the preemptirqsoff tracer will starting tracing from
preempt_disable and end at the preempt_enable. Instead, shouldn't we
be tracing starting from the local_irq_disable and end at the
preempt_enable?

Regards,
Joel

Re: preemptirqsoff tracer

[Steven Rostedt]

On Tue, 4 Apr 2017 01:27:25 -0700
Joel Fernandes <joelaf@google.com> wrote:

> Hi Steven,
> 
> Just looking at code of the preemptirqsoff tracer, I wonder if it is
> really working as expected.
> 
> With the tracer enabled, start_critical_timing will be called only if
> _both_ preemption and IRQs are turned off. Instead shouldn't this be
> an OR condition (either preemption or IRQs are turned off)?

What code are you looking at. From what I have:

void start_critical_timings(void)
{
	if (preempt_trace() || irq_trace())
		start_critical_timing(CALLER_ADDR0, CALLER_ADDR1);
}

There's an OR statement.

-- Steve

> 
> >From the example in ftrace.txt documentation:  
>     local_irq_disable();
>     call_function_with_irqs_off();
>     preempt_disable();
>     call_function_with_irqs_and_preemption_off();
>     local_irq_enable();
>     call_function_with_preemption_off();
>     preempt_enable();
> 
> Looks to me like the preemptirqsoff tracer will starting tracing from
> preempt_disable and end at the preempt_enable. Instead, shouldn't we
> be tracing starting from the local_irq_disable and end at the
> preempt_enable?
> 
> Regards,
> Joel

Re: preemptirqsoff tracer

[Joel Fernandes]

On Tue, Apr 4, 2017 at 6:27 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 4 Apr 2017 01:27:25 -0700
> Joel Fernandes <joelaf@google.com> wrote:
>
>> Hi Steven,
>>
>> Just looking at code of the preemptirqsoff tracer, I wonder if it is
>> really working as expected.
>>
>> With the tracer enabled, start_critical_timing will be called only if
>> _both_ preemption and IRQs are turned off. Instead shouldn't this be
>> an OR condition (either preemption or IRQs are turned off)?
>
> What code are you looking at. From what I have:

Mainline.

>
> void start_critical_timings(void)
> {
>         if (preempt_trace() || irq_trace())
>                 start_critical_timing(CALLER_ADDR0, CALLER_ADDR1);
> }

This is called from the idle path though.

>
> There's an OR statement.

Actually in trace_preempt_off, its an AND condition but the
irq_trace() is negated so it works correctly (too many double
negatives so I missed that, sorry!).

if (preempt_trace() && !irq_trace) ...

and likewise in trace_hardirqs_on, its:

if (!preempt_trace() && irq_trace())

So its all good :-)
Regards,
Joel