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

From: Joel Fernandes
Date: Mon Sep 04 2017 - 23:26:23 EST


On Mon, Sep 4, 2017 at 5:54 PM, Joel Fernandes <joelaf@xxxxxxxxxx> wrote:
> 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

Apologies, I meant (without the "off"):

subsystem: atomic_section
events:
irqs_disable
irqs_enable
preempt_disable
preempt_enable

and additionally (similar to what my patch does):
preemptirq_enable
preemptirq_disable


Could you let me know if we are in agreement about this naming?

thanks,

-Joel