live-patching.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Question about inline, notrace, and livepatch
@ 2023-05-02 17:40 Song Liu
  2023-05-02 17:56 ` Steven Rostedt
  0 siblings, 1 reply; 3+ messages in thread
From: Song Liu @ 2023-05-02 17:40 UTC (permalink / raw)
  To: live-patching, Steven Rostedt

We hit the following hiccup a couple times in the past few months:

A function is marked as "inline", but the compiler decided not to inline it.
Since "inline" implies "notrace" since [1], this function doesn't have
fentry/mcount. When we built livepatch for this function, kpatch-build
failed with:

   xxx.o: function yyy has no fentry/mcount call, unable to patch

This is not a deal breaker, as we can usually modify the patch to work
around it. But I wonder whether we still need "inline" to imply "notrace".
Can we remove this to make livepatch a little easier?

Thanks,
Song

[1] 93b3cca1ccd3 ("ftrace: Make all inline tags also include notrace")

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Question about inline, notrace, and livepatch
  2023-05-02 17:40 Question about inline, notrace, and livepatch Song Liu
@ 2023-05-02 17:56 ` Steven Rostedt
  2023-05-02 18:26   ` Song Liu
  0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2023-05-02 17:56 UTC (permalink / raw)
  To: Song Liu; +Cc: live-patching

On Tue, 2 May 2023 10:40:28 -0700
Song Liu <song@kernel.org> wrote:

> We hit the following hiccup a couple times in the past few months:
> 
> A function is marked as "inline", but the compiler decided not to inline it.
> Since "inline" implies "notrace" since [1], this function doesn't have
> fentry/mcount. When we built livepatch for this function, kpatch-build
> failed with:
> 
>    xxx.o: function yyy has no fentry/mcount call, unable to patch
> 
> This is not a deal breaker, as we can usually modify the patch to work
> around it. But I wonder whether we still need "inline" to imply "notrace".
> Can we remove this to make livepatch a little easier?

The history behind this is that there were cases that functions that were
inlined, were in critical paths that could cause crashes if they were
traced. In testing they never triggered because the developer's compiler
inlined them. Then on someone else's machine, the compiler decided not to
inline the function and the system crashed. It was hell to debug because I
was not able to reproduce the issue, as my compiler would always keep the
function inlined!

But a lot has changed since then. I've implemented the
"ftrace_test_recursion_trylock()" that catches pretty much all recursive
bugs (and can ever report when they happen). So I may be open to removing
the "notrace" from "inline".

-- Steve

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Question about inline, notrace, and livepatch
  2023-05-02 17:56 ` Steven Rostedt
@ 2023-05-02 18:26   ` Song Liu
  0 siblings, 0 replies; 3+ messages in thread
From: Song Liu @ 2023-05-02 18:26 UTC (permalink / raw)
  To: Steven Rostedt, Yonghong Song; +Cc: live-patching

+ Yonghong,

On Tue, May 2, 2023 at 10:56 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 2 May 2023 10:40:28 -0700
> Song Liu <song@kernel.org> wrote:
>
> > We hit the following hiccup a couple times in the past few months:
> >
> > A function is marked as "inline", but the compiler decided not to inline it.
> > Since "inline" implies "notrace" since [1], this function doesn't have
> > fentry/mcount. When we built livepatch for this function, kpatch-build
> > failed with:
> >
> >    xxx.o: function yyy has no fentry/mcount call, unable to patch
> >
> > This is not a deal breaker, as we can usually modify the patch to work
> > around it. But I wonder whether we still need "inline" to imply "notrace".
> > Can we remove this to make livepatch a little easier?
>
> The history behind this is that there were cases that functions that were
> inlined, were in critical paths that could cause crashes if they were
> traced. In testing they never triggered because the developer's compiler
> inlined them. Then on someone else's machine, the compiler decided not to
> inline the function and the system crashed. It was hell to debug because I
> was not able to reproduce the issue, as my compiler would always keep the
> function inlined!
>
> But a lot has changed since then. I've implemented the
> "ftrace_test_recursion_trylock()" that catches pretty much all recursive
> bugs (and can ever report when they happen). So I may be open to removing
> the "notrace" from "inline".

Thanks for the history! Let's try to remove this constraint.

Song

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-05-02 18:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-02 17:40 Question about inline, notrace, and livepatch Song Liu
2023-05-02 17:56 ` Steven Rostedt
2023-05-02 18:26   ` Song Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).