linux-kernel - Re: [RFC v4 3/4] irqflags: Avoid unnecessary calls to trace

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]

Message-ID: <CAJWu+orGDvo77vG9k9AZBBQ7WCNPyVj0yAfr5BNqw7=ASaJd3g@mail.gmail.com>
Date:   Mon, 23 Apr 2018 10:24:11 -0700
From:   Joel Fernandes <joelaf@...gle.com>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     rostedt <rostedt@...dmis.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-rt-users <linux-rt-users@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Tom Zanussi <tom.zanussi@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Boqun Feng <boqun.feng@...il.com>,
        fweisbec <fweisbec@...il.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        kbuild test robot <fengguang.wu@...el.com>,
        baohong liu <baohong.liu@...el.com>,
        vedang patel <vedang.patel@...el.com>,
        kernel-team <kernel-team@....com>
Subject: Re: [RFC v4 3/4] irqflags: Avoid unnecessary calls to trace_ if you can

Hi Mathieu,

On Mon, Apr 23, 2018 at 10:12 AM, Mathieu Desnoyers
<mathieu.desnoyers@...icios.com> wrote:
> ----- On Apr 23, 2018, at 12:18 PM, rostedt rostedt@...dmis.org wrote:
>
>> On Mon, 23 Apr 2018 10:59:43 -0400 (EDT)
>> Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:
>>
>>> The main open question here is whether we want one SRCU grace period
>>> domain per SRCU tracepoint definition, or just one SRCU domain for all
>>> SRCU tracepoints would be fine.
>>>
>>> I'm not sure what we would gain by having the extra granularity provided
>>> by one SRCU grace period domain per tracepoint, and having a single SRCU
>>> domain for all SRCU tracepoints makes it easy to batch grace period after
>>> bulk tracepoint modifications.
>>>
>>> Thoughts ?
>>
>> I didn't think too much depth in this. It was more of just a brain
>> storming idea. Yeah, one singe RCU domain may be good enough. I was
>> thinking more of how to know when a tracepoint required the SRCU domain
>> as supposed to a preempt disabled domain, and wanted to just suggest
>> the linker script approach.
>>
>> This is how I detect if trace_printk() is used anywhere in the kernel
>> (and do that big warning if it is). That way the trace events don't
>> need to be created any special way. You just use the trace_##event##_X
>> flavor and it automatically detects what to do. But we need to make
>> sure the same event isn't used for multiple flavors (SRCU vs schedule),
>> or maybe we can, and any change would have to do both synchronizations.
>
> The approach I used for synchronize rcu a few years ago when I did a srcu
> tracepoint prototype [1] was simply this:
>
>  static inline void tracepoint_synchronize_unregister(void)
>  {
> +       synchronize_srcu(&tracepoint_srcu);
>         synchronize_sched();
>  }
>
> So whenever we synchronize after tracepoint unregistration, the tracepoint
> code always issue both synchronize_sched() and SRCU synchronize. This way,
> tracepoint API users don't have to care about the kind of tracepoint they
> are updating.
>
> I'm inclined to explicitly declare the tracepoints with their given
> synchronization method. Tracepoint probe callback functions for currently
> existing tracepoints expect to have preemption disabled when invoked.
> This assumption will not be true anymore for srcu-tracepoints.

Nice thing about your patch is it defines at declaration time that its
an srcu-tracepoint. The API users don't need care about which
synchronization method to use which will probably prevent bugs like,
if someone forget to use the _rcuidle variable for a tracepoint or
something.

I carved out some of my time to test this patch today :-)

thanks,

- Joel