[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YFoN7nCl8OfGtpeh@hirez.programming.kicks-ass.net>
Date: Tue, 23 Mar 2021 16:49:02 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: linux-kernel@...r.kernel.org
Cc: linux-tip-commits@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
Ingo Molnar <mingo@...nel.org>, x86@...nel.org,
Josh Poimboeuf <jpoimboe@...hat.com>,
Steven Rostedt <rostedt@...dmis.org>, jbaron@...mai.com,
ardb@...nel.org, frederic@...nel.org
Subject: Re: [tip: locking/core] static_call: Fix function type mismatch
On Tue, Mar 23, 2021 at 08:37:33AM +0100, Peter Zijlstra wrote:
> On Mon, Mar 22, 2021 at 11:30:07PM -0000, tip-bot2 for Arnd Bergmann wrote:
> > The following commit has been merged into the locking/core branch of tip:
> >
> > Commit-ID: 335c73e7c8f7deb23537afbbbe4f8ab48bd5de52
> > Gitweb: https://git.kernel.org/tip/335c73e7c8f7deb23537afbbbe4f8ab48bd5de52
> > Author: Arnd Bergmann <arnd@...db.de>
> > AuthorDate: Mon, 22 Mar 2021 22:42:24 +01:00
> > Committer: Ingo Molnar <mingo@...nel.org>
> > CommitterDate: Tue, 23 Mar 2021 00:08:53 +01:00
> >
> > static_call: Fix function type mismatch
> >
> > The __static_call_return0() function is declared to return a 'long',
> > while it aliases a couple of functions that all return 'int'. When
> > building with 'make W=1', gcc warns about this:
> >
> > kernel/sched/core.c:5420:37: error: cast between incompatible function types from 'long int (*)(void)' to 'int (*)(void)' [-Werror=cast-function-type]
> > 5420 | static_call_update(might_resched, (typeof(&__cond_resched)) __static_call_return0);
> >
> > Change all these function to return 'long' as well, but remove the cast to
> > ensure we get a warning if any of the types ever change.
> >
> > Signed-off-by: Arnd Bergmann <arnd@...db.de>
> > Signed-off-by: Ingo Molnar <mingo@...nel.org>
> > Link: https://lore.kernel.org/r/20210322214309.730556-1-arnd@kernel.org
>
> So I strongly disagree and think the warning is bad and should be
> disabled. I'll go uncommit this patch.
How's this then? I haven't yet build GCC11 myself, but seeing how adding
the (void *) cast fixed the ftrace case, it should work here too.
And note that if you remove the (void *) cast, you get a nice warning:
../include/linux/static_call.h:121:41: error: initialization of ‘int (*)(void)’ from incompatible pointer type ‘long int (*)(void)’ [-Werror=incompatible-pointer-types]
No fancy new compiler fail^Wfeatures needed.
Also note the irony, how a warning that was supposed to strengthen types
leads to weakening them.
---
Subject: static_call: Relax static_call_update() function argument type
static_call_update() had stronger type requirements than regular C,
relax them to match. Instead of requiring the @func argument has the
exact matching type, allow any type which C is willing to promote to the
right (function) pointer type. Specifically this allows (void *)
arguments.
This cleans up a bunch of static_call_update() callers for
PREEMPT_DYNAMIC and should get around silly GCC11 warnings for free.
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
include/linux/static_call.h | 4 ++--
kernel/sched/core.c | 18 +++++++++---------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/linux/static_call.h b/include/linux/static_call.h
index e01b61ab86b1..fc94faa53b5b 100644
--- a/include/linux/static_call.h
+++ b/include/linux/static_call.h
@@ -118,9 +118,9 @@ extern void arch_static_call_transform(void *site, void *tramp, void *func, bool
#define static_call_update(name, func) \
({ \
- BUILD_BUG_ON(!__same_type(*(func), STATIC_CALL_TRAMP(name))); \
+ typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
__static_call_update(&STATIC_CALL_KEY(name), \
- STATIC_CALL_TRAMP_ADDR(name), func); \
+ STATIC_CALL_TRAMP_ADDR(name), __F); \
})
#define static_call_query(name) (READ_ONCE(STATIC_CALL_KEY(name).func))
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3384ea74cad4..42f9bedc666c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5402,25 +5402,25 @@ static void sched_dynamic_update(int mode)
switch (mode) {
case preempt_dynamic_none:
static_call_update(cond_resched, __cond_resched);
- static_call_update(might_resched, (typeof(&__cond_resched)) __static_call_return0);
- static_call_update(preempt_schedule, (typeof(&preempt_schedule)) NULL);
- static_call_update(preempt_schedule_notrace, (typeof(&preempt_schedule_notrace)) NULL);
- static_call_update(irqentry_exit_cond_resched, (typeof(&irqentry_exit_cond_resched)) NULL);
+ static_call_update(might_resched, (void *)&__static_call_return0);
+ static_call_update(preempt_schedule, NULL);
+ static_call_update(preempt_schedule_notrace, NULL);
+ static_call_update(irqentry_exit_cond_resched, NULL);
pr_info("Dynamic Preempt: none\n");
break;
case preempt_dynamic_voluntary:
static_call_update(cond_resched, __cond_resched);
static_call_update(might_resched, __cond_resched);
- static_call_update(preempt_schedule, (typeof(&preempt_schedule)) NULL);
- static_call_update(preempt_schedule_notrace, (typeof(&preempt_schedule_notrace)) NULL);
- static_call_update(irqentry_exit_cond_resched, (typeof(&irqentry_exit_cond_resched)) NULL);
+ static_call_update(preempt_schedule, NULL);
+ static_call_update(preempt_schedule_notrace, NULL);
+ static_call_update(irqentry_exit_cond_resched, NULL);
pr_info("Dynamic Preempt: voluntary\n");
break;
case preempt_dynamic_full:
- static_call_update(cond_resched, (typeof(&__cond_resched)) __static_call_return0);
- static_call_update(might_resched, (typeof(&__cond_resched)) __static_call_return0);
+ static_call_update(cond_resched, (void *)&__static_call_return0);
+ static_call_update(might_resched, (void *)&__static_call_return0);
static_call_update(preempt_schedule, __preempt_schedule_func);
static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
Powered by blists - more mailing lists