MIME-Version: 1.0
In-Reply-To: <20170904075614.bjkkrgyv2dpz7x5v@hirez.programming.kicks-ass.net>
References: <20170903085051.6348-1-joelaf@google.com> <20170903085051.6348-3-joelaf@google.com>
 <20170904075614.bjkkrgyv2dpz7x5v@hirez.programming.kicks-ass.net>
From: Joel Fernandes <joelaf@google.com>
Date: Mon, 4 Sep 2017 12:04:44 -0700
Message-ID: <CAJWu+opjZaN4d2-+0jD3QwO+jEXx16WdY0Y_CrP4xihyoemgPQ@mail.gmail.com>
Subject: Re: [PATCH 2/2] tracing: Add support for critical section events
To: Peter Zijlstra <peterz@infradead.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>, kernel-team@android.com,
        Ingo Molnar <mingo@redhat.com>,
        Byungchul Park <byungchul.park@lge.com>, Tejun Heo <tj@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2252
Lines: 51

Hi Peter,

On Mon, Sep 4, 2017 at 12:56 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sun, Sep 03, 2017 at 01:50:51AM -0700, Joel Fernandes wrote:
>> Critical section trace events can be used for tracing the start and
>> end of a critical section which can be used by a trace viewer like
>> systrace to graphically view the start and end of a critical section and
>> correlate them with latencies and scheduling issues.
>>
>> Reason for starting critical section:
>> Either preemption or interrupts were turned off.
>> Reason for ending critical section:
>> Both preemption and interrupts are now turned back on.
>
> Please don't use the name critical section for this.

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.

> Also IRQ and preempt already have a gazillion trace hooks, why do we need more?

I think I didn't CC you on the coverletter (that's my bad). That's Ok
I'll point to links below.
The goal of the patch is not to add more hooks, but to make it
possible for userspace to use the trace-events mechanism to see these
events, so that these events can be seen along with other trace events
when using event tracing, using a convenient trace event interface
which can be enabled by userspace. This makes it possible to visually
see these events as a timeline along with other kernel and userspace
events. See more description in my coverletter [1] and a screenshot of
how this is used [2] by the systrace trace viewer. These patches make
it possible for this.

Also, this work is along the same lines of what we discussed in a
recent conference about adding irqsoff and preemptoff as real trace
events, and then using synthetic events (which can combine start and
end event into a single event) to redo the irqsoff tracer, so in that
sense it is in the right direction. Synthetic events is some time away
though from being merged AFAIK.

thanks,

- Joel

[1] https://lkml.org/lkml/2017/9/3/15
[2] http://imgur.com/download/TZplEVp
[3] all patches:
    https://patchwork.kernel.org/patch/9936047/
    https://patchwork.kernel.org/patch/9936045/