Re: [PATCH v3 2/3] soc: qcom: rpmh: Allow RPMH driver to be loaded as a module

From: John Stultz <john.stultz@linaro.org>
To: Matthias Kaehlcke <mka@chromium.org>
Cc: lkml <linux-kernel@vger.kernel.org>, Todd Kjos <tkjos@google.com>,
	Saravana Kannan <saravanak@google.com>,
	Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Stephen Boyd <swboyd@chromium.org>,
	Douglas Anderson <dianders@chromium.org>
Subject: Re: [PATCH v3 2/3] soc: qcom: rpmh: Allow RPMH driver to be loaded as a module
Date: Wed, 15 Apr 2020 12:47:06 -0700	[thread overview]
Message-ID: <CALAqxLUvQRS0iFz5fXReXvY08oij1BtP6vpnL1qUY-Bs6OncnQ@mail.gmail.com> (raw)
In-Reply-To: <20200415182536.GX199755@google.com>

On Wed, Apr 15, 2020 at 11:25 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Hi John,
>
> with commit efde2659b0fe ("drivers: qcom: rpmh-rsc: Use rcuidle
> tracepoints for rpmh") the rpmh-rsc driver fails to build as a
> module:
>
> drivers/soc/qcom/rpmh-rsc.c:281:3: error: implicit declaration of function 'trace_rpmh_send_msg_rcuidle' [-Werror,-Wimplicit-function-decr]
>                 trace_rpmh_send_msg_rcuidle(drv, tcs_id, j, msgid, cmd);
>
>
> The problem is that the _rcuidle() functions are not generated for modules:
>
> #ifndef MODULE
> #define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
>         static inline void trace_##name##_rcuidle(proto)                \
>         {                                                               \
>                 if (static_key_false(&__tracepoint_##name.key))         \
>                         __DO_TRACE(&__tracepoint_##name,                \
>                                 TP_PROTO(data_proto),                   \
>                                 TP_ARGS(data_args),                     \
>                                 TP_CONDITION(cond), 1);                 \
>         }
> #else
> #define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)
> #endif
>
> Not sure what the best solution would be in this case. Having the macro
> define a dummy function for modules would fix the build error, however it
> would be confusing that the event is traced when the driver is built-in,
> but not when it is built as a module.
>
> I imagine the goal behind making this driver a module is to have a single
> kernel image for multiple SoC platforms, without too much platform
> specific code in the kernel image itself.
>
> I guess the question is whether there any options for keeping the driver
> modular and having consistent tracing behavior, short of removing the
> tracepoint.

Yea.  Stephen found that issue in -next last night once Bjorn added
the patches to his tree yesterday.

I've reached out to see if the restrictions on the trace_*_rcuidle
calls on modules is still necessary in this thread:
  https://lore.kernel.org/lkml/CALAqxLV4rM74wuzuZ+BkUi+keccxkAxv30N4vrFO7CVQ5vnT1A@mail.gmail.com/

For now, I suggested Bjorn revert the patch in his tree, and I'll try
to figure out an alternative solution to the trace call.

thanks
-john