[PATCH] kcov: mark in_softirq_really() as __always_inline

[PATCH] kcov: mark in_softirq_really() as __always_inline

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

If gcc decides not to inline in_softirq_really(), objtool warns about
a function call with UACCESS enabled:

kernel/kcov.o: warning: objtool: __sanitizer_cov_trace_pc+0x1e: call to in_softirq_really() with UACCESS enabled
kernel/kcov.o: warning: objtool: check_kcov_mode+0x11: call to in_softirq_really() with UACCESS enabled

Mark this as __always_inline to avoid the problem.

Fixes: 7d4df2dad312 ("kcov: properly check for softirq context")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/kcov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kcov.c b/kernel/kcov.c
index 28a6be6e64fd..187ba1b80bda 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -166,7 +166,7 @@ static void kcov_remote_area_put(struct kcov_remote_area *area,
  * Unlike in_serving_softirq(), this function returns false when called during
  * a hardirq or an NMI that happened in the softirq context.
  */
-static inline bool in_softirq_really(void)
+static __always_inline bool in_softirq_really(void)
 {
 	return in_serving_softirq() && !in_hardirq() && !in_nmi();
 }
-- 
2.39.5

Re: [PATCH] kcov: mark in_softirq_really() as __always_inline

[Marco Elver]

On Tue, 17 Dec 2024 at 08:18, Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> If gcc decides not to inline in_softirq_really(), objtool warns about
> a function call with UACCESS enabled:
>
> kernel/kcov.o: warning: objtool: __sanitizer_cov_trace_pc+0x1e: call to in_softirq_really() with UACCESS enabled
> kernel/kcov.o: warning: objtool: check_kcov_mode+0x11: call to in_softirq_really() with UACCESS enabled
>
> Mark this as __always_inline to avoid the problem.
>
> Fixes: 7d4df2dad312 ("kcov: properly check for softirq context")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

__always_inline is the usual approach for code that can be
instrumented - but I thought we explicitly never instrument
kernel/kcov.c with anything. So I'm rather puzzled why gcc would not
inline this function. In any case "inline" guarantees nothing, so:

Reviewed-by: Marco Elver <elver@google.com>

> ---
>  kernel/kcov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 28a6be6e64fd..187ba1b80bda 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -166,7 +166,7 @@ static void kcov_remote_area_put(struct kcov_remote_area *area,
>   * Unlike in_serving_softirq(), this function returns false when called during
>   * a hardirq or an NMI that happened in the softirq context.
>   */
> -static inline bool in_softirq_really(void)
> +static __always_inline bool in_softirq_really(void)
>  {
>         return in_serving_softirq() && !in_hardirq() && !in_nmi();
>  }
> --
> 2.39.5
>

Re: [PATCH] kcov: mark in_softirq_really() as __always_inline

[Josh Poimboeuf]

On Tue, Dec 17, 2024 at 09:30:24AM +0100, Marco Elver wrote:
> On Tue, 17 Dec 2024 at 08:18, Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > If gcc decides not to inline in_softirq_really(), objtool warns about
> > a function call with UACCESS enabled:
> >
> > kernel/kcov.o: warning: objtool: __sanitizer_cov_trace_pc+0x1e: call to in_softirq_really() with UACCESS enabled
> > kernel/kcov.o: warning: objtool: check_kcov_mode+0x11: call to in_softirq_really() with UACCESS enabled
> >
> > Mark this as __always_inline to avoid the problem.
> >
> > Fixes: 7d4df2dad312 ("kcov: properly check for softirq context")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> __always_inline is the usual approach for code that can be
> instrumented - but I thought we explicitly never instrument
> kernel/kcov.c with anything. So I'm rather puzzled why gcc would not
> inline this function. In any case "inline" guarantees nothing, so:

I'm guessing CONFIG_DEBUG_SECTION_MISMATCH was enabled, which enables
-fno-inline-functions-called-once which ends up being the cause of a lot
of these __always_inline patches.

I had a patch to get rid of that at some point, guess it got lost...

-- 
Josh

Re: [PATCH] kcov: mark in_softirq_really() as __always_inline

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

On Wed, Dec 18, 2024, at 09:40, Josh Poimboeuf wrote:
> On Tue, Dec 17, 2024 at 09:30:24AM +0100, Marco Elver wrote:
>> On Tue, 17 Dec 2024 at 08:18, Arnd Bergmann <arnd@kernel.org> wrote:
>> >
>> > From: Arnd Bergmann <arnd@arndb.de>
>> >
>> > If gcc decides not to inline in_softirq_really(), objtool warns about
>> > a function call with UACCESS enabled:
>> >
>> > kernel/kcov.o: warning: objtool: __sanitizer_cov_trace_pc+0x1e: call to in_softirq_really() with UACCESS enabled
>> > kernel/kcov.o: warning: objtool: check_kcov_mode+0x11: call to in_softirq_really() with UACCESS enabled
>> >
>> > Mark this as __always_inline to avoid the problem.
>> >
>> > Fixes: 7d4df2dad312 ("kcov: properly check for softirq context")
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> 
>> __always_inline is the usual approach for code that can be
>> instrumented - but I thought we explicitly never instrument
>> kernel/kcov.c with anything. So I'm rather puzzled why gcc would not
>> inline this function. In any case "inline" guarantees nothing, so:
>
> I'm guessing CONFIG_DEBUG_SECTION_MISMATCH was enabled, which enables
> -fno-inline-functions-called-once which ends up being the cause of a lot
> of these __always_inline patches.
>
> I had a patch to get rid of that at some point, guess it got lost...

It doesn't seem to be the cause here, I get the warning both with
and without CONFIG_DEBUG_SECTION_MISMATCH in random configurations.
I've attached one .config that shows the problem without this
option in case you want to investigate further.

     Arnd

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38798 bytes --]

Re: [PATCH] kcov: mark in_softirq_really() as __always_inline

[Josh Poimboeuf]

On Wed, Dec 18, 2024 at 09:49:46AM +0100, Arnd Bergmann wrote:
> On Wed, Dec 18, 2024, at 09:40, Josh Poimboeuf wrote:
> > On Tue, Dec 17, 2024 at 09:30:24AM +0100, Marco Elver wrote:
> >> On Tue, 17 Dec 2024 at 08:18, Arnd Bergmann <arnd@kernel.org> wrote:
> >> >
> >> > From: Arnd Bergmann <arnd@arndb.de>
> >> >
> >> > If gcc decides not to inline in_softirq_really(), objtool warns about
> >> > a function call with UACCESS enabled:
> >> >
> >> > kernel/kcov.o: warning: objtool: __sanitizer_cov_trace_pc+0x1e: call to in_softirq_really() with UACCESS enabled
> >> > kernel/kcov.o: warning: objtool: check_kcov_mode+0x11: call to in_softirq_really() with UACCESS enabled
> >> >
> >> > Mark this as __always_inline to avoid the problem.
> >> >
> >> > Fixes: 7d4df2dad312 ("kcov: properly check for softirq context")
> >> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> 
> >> __always_inline is the usual approach for code that can be
> >> instrumented - but I thought we explicitly never instrument
> >> kernel/kcov.c with anything. So I'm rather puzzled why gcc would not
> >> inline this function. In any case "inline" guarantees nothing, so:
> >
> > I'm guessing CONFIG_DEBUG_SECTION_MISMATCH was enabled, which enables
> > -fno-inline-functions-called-once which ends up being the cause of a lot
> > of these __always_inline patches.
> >
> > I had a patch to get rid of that at some point, guess it got lost...
> 
> It doesn't seem to be the cause here, I get the warning both with
> and without CONFIG_DEBUG_SECTION_MISMATCH in random configurations.
> I've attached one .config that shows the problem without this
> option in case you want to investigate further.

Guess I should have looked closer, that function is called more than
once, never mind...

-- 
Josh