[PATCH] entry: fix compile error in dynamic_irqentry_exit_cond_resched()

[PATCH] entry: fix compile error in dynamic_irqentry_exit_cond_resched()

[Sven Schnelle]

kernel/entry/common.c: In function ‘dynamic_irqentry_exit_cond_resched’:
kernel/entry/common.c:409:14: error: implicit declaration of function ‘static_key_unlikely’; did you mean ‘static_key_enable’? [-Werror=implicit-function-declaration]
  409 |         if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
      |              ^~~~~~~~~~~~~~~~~~~
      |              static_key_enable

static_key_unlikely() should be static_branch_unlikely().

Fixes: 99cf983cc8bca ("sched/preempt: Add PREEMPT_DYNAMIC using static keys")
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
 kernel/entry/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index ef8d94a98b7e..371ee8914af1 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -406,7 +406,7 @@ DEFINE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);
 DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
 void dynamic_irqentry_exit_cond_resched(void)
 {
-	if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
+	if (!static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
 		return;
 	raw_irqentry_exit_cond_resched();
 }
-- 
2.32.0

Re: [PATCH] entry: fix compile error in dynamic_irqentry_exit_cond_resched()

[Mark Rutland]

On Wed, Mar 30, 2022 at 10:43:28AM +0200, Sven Schnelle wrote:
> kernel/entry/common.c: In function ‘dynamic_irqentry_exit_cond_resched’:
> kernel/entry/common.c:409:14: error: implicit declaration of function ‘static_key_unlikely’; did you mean ‘static_key_enable’? [-Werror=implicit-function-declaration]
>   409 |         if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
>       |              ^~~~~~~~~~~~~~~~~~~
>       |              static_key_enable
> 
> static_key_unlikely() should be static_branch_unlikely().
> 
> Fixes: 99cf983cc8bca ("sched/preempt: Add PREEMPT_DYNAMIC using static keys")
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>

Sorry about this. FWIW:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

For context for others, this'll only show up on architectures which both use
the generic entry code and select CONFIG_HAVE_PREEMPT_DYNAMIC_KEY. Today, only
arm64 selects CONFIG_HAVE_PREEMPT_DYNAMIC_KEY, and it doesn't use the generic
entry code.

Sven, I assume you're looking at wiring this up on s390 or parisc?

Thanks,
Mark.

> ---
>  kernel/entry/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index ef8d94a98b7e..371ee8914af1 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -406,7 +406,7 @@ DEFINE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);
>  DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
>  void dynamic_irqentry_exit_cond_resched(void)
>  {
> -	if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
> +	if (!static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
>  		return;
>  	raw_irqentry_exit_cond_resched();
>  }
> -- 
> 2.32.0
> 

Re: [PATCH] entry: fix compile error in dynamic_irqentry_exit_cond_resched()

[Sven Schnelle]

Mark Rutland <mark.rutland@arm.com> writes:

> On Wed, Mar 30, 2022 at 10:43:28AM +0200, Sven Schnelle wrote:
>> kernel/entry/common.c: In function ‘dynamic_irqentry_exit_cond_resched’:
>> kernel/entry/common.c:409:14: error: implicit declaration of
>> function ‘static_key_unlikely’; did you mean ‘static_key_enable’?
>> [-Werror=implicit-function-declaration]
>>   409 |         if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
>>       |              ^~~~~~~~~~~~~~~~~~~
>>       |              static_key_enable
>> 
>> static_key_unlikely() should be static_branch_unlikely().
>> 
>> Fixes: 99cf983cc8bca ("sched/preempt: Add PREEMPT_DYNAMIC using static keys")
>> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
>
> Sorry about this. FWIW:
>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>
> For context for others, this'll only show up on architectures which both use
> the generic entry code and select CONFIG_HAVE_PREEMPT_DYNAMIC_KEY. Today, only
> arm64 selects CONFIG_HAVE_PREEMPT_DYNAMIC_KEY, and it doesn't use the generic
> entry code.
>
> Sven, I assume you're looking at wiring this up on s390 or parisc?

Yes, i'm looking whether we can use the same implementation on s390. :)

I reported it already on 03/18, but looks like that Mail was lost
somehow:

https://www.spinics.net/lists/kernel/msg4283802.html

I was wondering whether we can make dynamic_irqentry_exit_cond_resched()
static, so it gets inlined. On s390 the compiler generates a branch to
that function just to return immediately if the static key isn't enabled.
With static it would get inlined, and therefore save one function call.
What do you think?

Thanks
Sven

Re: [PATCH] entry: fix compile error in dynamic_irqentry_exit_cond_resched()

[Mark Rutland]

On Wed, Mar 30, 2022 at 11:17:15AM +0200, Sven Schnelle wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> > On Wed, Mar 30, 2022 at 10:43:28AM +0200, Sven Schnelle wrote:
> >> kernel/entry/common.c: In function ‘dynamic_irqentry_exit_cond_resched’:
> >> kernel/entry/common.c:409:14: error: implicit declaration of
> >> function ‘static_key_unlikely’; did you mean ‘static_key_enable’?
> >> [-Werror=implicit-function-declaration]
> >>   409 |         if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
> >>       |              ^~~~~~~~~~~~~~~~~~~
> >>       |              static_key_enable
> >> 
> >> static_key_unlikely() should be static_branch_unlikely().
> >> 
> >> Fixes: 99cf983cc8bca ("sched/preempt: Add PREEMPT_DYNAMIC using static keys")
> >> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> >
> > Sorry about this. FWIW:
> >
> > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> >
> > For context for others, this'll only show up on architectures which both use
> > the generic entry code and select CONFIG_HAVE_PREEMPT_DYNAMIC_KEY. Today, only
> > arm64 selects CONFIG_HAVE_PREEMPT_DYNAMIC_KEY, and it doesn't use the generic
> > entry code.
> >
> > Sven, I assume you're looking at wiring this up on s390 or parisc?
> 
> Yes, i'm looking whether we can use the same implementation on s390. :)
> 
> I reported it already on 03/18, but looks like that Mail was lost
> somehow:
> 
> https://www.spinics.net/lists/kernel/msg4283802.html

Ah; sorry, I didn't spot that somehow.

> I was wondering whether we can make dynamic_irqentry_exit_cond_resched()
> static, so it gets inlined. On s390 the compiler generates a branch to
> that function just to return immediately if the static key isn't enabled.
> With static it would get inlined, and therefore save one function call.
> What do you think?

I appreciate that it saves one call, but does that actually matter in practice,
given this is called only once per interrupt?

I'm not fundamentally opposed to changing it, but doing so would make it
different from all the other dynamic_*() cases which need the check to be
out-of-line to avoid bloating the callers, and it's not clear to me that we'd
gain much by doing so.

FWIW, on arm64 we had some additional conditions we have to check, so we roll
our own (out-of-line) implementation anyway. So doing something arch-specific
is also an option.

Thanks,
Mark.

Re: [PATCH] entry: fix compile error in dynamic_irqentry_exit_cond_resched()

[Sven Schnelle]

Mark Rutland <mark.rutland@arm.com> writes:

>> I was wondering whether we can make dynamic_irqentry_exit_cond_resched()
>> static, so it gets inlined. On s390 the compiler generates a branch to
>> that function just to return immediately if the static key isn't enabled.
>> With static it would get inlined, and therefore save one function call.
>> What do you think?
>
> I appreciate that it saves one call, but does that actually matter in practice,
> given this is called only once per interrupt?
>
> I'm not fundamentally opposed to changing it, but doing so would make it
> different from all the other dynamic_*() cases which need the check to be
> out-of-line to avoid bloating the callers, and it's not clear to me that we'd
> gain much by doing so.
>
> FWIW, on arm64 we had some additional conditions we have to check, so we roll
> our own (out-of-line) implementation anyway. So doing something arch-specific
> is also an option.

I didn't measure it, i just noticed it by looking at the generated code.
Given your argument about consistency with the other functions i think
we leave it that way.

Thanks!
Sven

[tip: sched/urgent] entry: Fix compile error in dynamic_irqentry_exit_cond_resched()

[tip-bot2 for Sven Schnelle]

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     0a70045ed8516dfcff4b5728557e1ef3fd017c53
Gitweb:        https://git.kernel.org/tip/0a70045ed8516dfcff4b5728557e1ef3fd017c53
Author:        Sven Schnelle <svens@linux.ibm.com>
AuthorDate:    Wed, 30 Mar 2022 10:43:28 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Apr 2022 09:59:36 +02:00

entry: Fix compile error in dynamic_irqentry_exit_cond_resched()

kernel/entry/common.c: In function ‘dynamic_irqentry_exit_cond_resched’:
kernel/entry/common.c:409:14: error: implicit declaration of function ‘static_key_unlikely’; did you mean ‘static_key_enable’? [-Werror=implicit-function-declaration]
  409 |         if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
      |              ^~~~~~~~~~~~~~~~~~~
      |              static_key_enable

static_key_unlikely() should be static_branch_unlikely().

Fixes: 99cf983cc8bca ("sched/preempt: Add PREEMPT_DYNAMIC using static keys")
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Link: https://lore.kernel.org/r/20220330084328.1805665-1-svens@linux.ibm.com
---
 kernel/entry/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index e57a224..93c3b86 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -392,7 +392,7 @@ DEFINE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);
 DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
 void dynamic_irqentry_exit_cond_resched(void)
 {
-	if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
+	if (!static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
 		return;
 	raw_irqentry_exit_cond_resched();
 }