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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 4 Sep 2017 17:54:02 -0700
From:   Joel Fernandes <joelaf@...gle.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>, kernel-team@...roid.com,
        Ingo Molnar <mingo@...hat.com>,
        Byungchul Park <byungchul.park@....com>,
        Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH 2/2] tracing: Add support for critical section events

On Mon, Sep 4, 2017 at 4:34 PM, Steven Rostedt <rostedt@...dmis.org> wrote:
> On Mon, 4 Sep 2017 21:44:26 +0200
> Peter Zijlstra <peterz@...radead.org> 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