Re: [PATCH 2/2] tracing: Add support for critical section events

From: Joel Fernandes
Date: Mon Sep 04 2017 - 20:58:15 EST


On Mon, Sep 4, 2017 at 4:34 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Mon, 4 Sep 2017 21:44:26 +0200
> Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
>> > I can change the name to something else, but at the moment I can't
>> > think of anything better. Could you suggest a better name? Also btw,
>> > 'critical timings' is the terminology used within the irqsoff tracer
>> > so this is in line with that.
>>
>> So 'critical section' is what some mis-guided people call the locked
>> region of a lock :-) Using it for something else is prone to cause more
>> confusion...
>>
>> I would simply call them what they are: irq_disable,irq_enable
>> preempt_disable,preempt_enable.
>
> Yes please. The "critical section" naming came from the code that was
> from the latency tracer days of the real time patch (pre-ftrace). The
> irqsoff tracer has the least modification from the original code, and
> probably should be rewritten one of these days.

Sounds good to me. For the subsystem, could you guys suggest a name? I
was thinking "atomic_section"?

Something like:

subsystem: atomic_section
events:
irqsoff_disable
irqsoff_enable
preemptoff_disable
preemptoff_enable

and additionally (to do what my patch does):
preemptirqsoff_enable
preemptirqsoff_disable

Does this sound good to you?

I did bulk of the work today for moving code from the irqsoff tracer
into a common area that's irqsoff tracer specific, and now I'm getting
close to the point of creating the above tracepoints and posting
another series, but I wanted to get your thoughts on the above.

I like this new approach better because the next logically step ontop
of this work can be to rewrite irqsoff tracer by using probes on the
above tracepoints (which I'll do once we can get these in).

thanks,

- Joel