Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CALAqxLV4rM74wuzuZ+BkUi+keccxkAxv30N4vrFO7CVQ5vnT1A@mail.gmail.com>
 <20200415085348.5511a5fe@gandalf.local.home> <CALAqxLV1A6sOC1GWpFYXeBoDff0+AJgoOYK7NktcTdvX3kvAeg@mail.gmail.com>
 <20200415161424.584d07d3@gandalf.local.home>
In-Reply-To: <20200415161424.584d07d3@gandalf.local.home>
From:   John Stultz <john.stultz@linaro.org>
Date:   Wed, 15 Apr 2020 13:17:53 -0700
Message-ID: <CALAqxLU26PVFPSza5GceSF6gTVdzo_2D3G0dBp0KZXvAWFUktA@mail.gmail.com>
Subject: Re: On trace_*_rcuidle functions in modules
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     paulmck@kernel.org, Josh Triplett <josh@joshtriplett.org>,
        lkml <linux-kernel@vger.kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Saravana Kannan <saravanak@google.com>,
        Todd Kjos <tkjos@google.com>, Stephen Boyd <sboyd@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 15, 2020 at 1:14 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 15 Apr 2020 12:56:52 -0700
> John Stultz <john.stultz@linaro.org> wrote:
>
> > I'm trying to enable the qcom rpmh driver
> > (drivers/soc/qcom/rpmh-rsc.c) to be a module.  As I mentioned to Paul,
> > it registers a cpu_pm notifier callback, which calls its
> > __tcs_buffer_write() function. The trace in the __tcs_buffer_write()
> > function was just converted to using rcuidle to address bugs seen when
> > it was being called from idle.
> >
> > > Currently, Thomas and Peter are working on removing trace events from
> > > places that don't have RCU enabled, or at least cleaning up the context
> > > switches from user to kernel to interrupt.
> >
> > So does that mean folks would most likely lean to trying to remove the
> > tracepoint rather than reevaluating allowing the rcuidle call to be
> > made from the module?
> >
>
> No. The clean up is to try to make the switch from each context small, fast
> and safe. But what you are describing is the switch to idle, which is a
> different story and something that there's some talk about cleaning up, but
> not at the same level. Especially if there's more complex code that is
> happening with RCU watching.
>
> Looking at the commit that keeps trace_*_rcuidle() code out:
>
>   7ece55a4a3a04a ("trace: Don't declare trace_*_rcuidle functions in modules")
>
> Which was added because the rcuidle variant called RCU code that was not
> exported either. Which would have the same issue now as
> rcu_irq_exit_irqson() is also not exported. Which would be needed.

Right, reverting that change and adding the exports seems like the
most direct solution, but I suspect that wasn't done back in the day
for a good reason. So I'm just trying to better understand that
reason.

> Hmm, isn't module code itself synchronized via RCU. Then having module code
> being called without RCU "watching" could be dangerous?

I'm not sure I'm following you here. Could you explain more?

thanks
-john