[tip:,locking/core] static_call: Fix function type mismatch
diff mbox series

Message ID 161645580767.398.731817901273202970.tip-bot2@tip-bot2
State In Next
Commit 335c73e7c8f7deb23537afbbbe4f8ab48bd5de52
Headers show
Series
  • [tip:,locking/core] static_call: Fix function type mismatch
Related show

Commit Message

tip-bot2 for Thomas Gleixner March 22, 2021, 11:30 p.m. UTC
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@arndb.de>
AuthorDate:    Mon, 22 Mar 2021 22:42:24 +01:00
Committer:     Ingo Molnar <mingo@kernel.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@arndb.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20210322214309.730556-1-arnd@kernel.org
---
 include/linux/kernel.h |  4 ++--
 include/linux/sched.h  | 14 +++++++-------
 kernel/sched/core.c    |  8 ++++----
 3 files changed, 13 insertions(+), 13 deletions(-)

Comments

Peter Zijlstra March 23, 2021, 7:37 a.m. UTC | #1
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@arndb.de>
> AuthorDate:    Mon, 22 Mar 2021 22:42:24 +01:00
> Committer:     Ingo Molnar <mingo@kernel.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@arndb.de>
> Signed-off-by: Ingo Molnar <mingo@kernel.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.
Peter Zijlstra March 23, 2021, 3:49 p.m. UTC | #2
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@arndb.de>
> > AuthorDate:    Mon, 22 Mar 2021 22:42:24 +01:00
> > Committer:     Ingo Molnar <mingo@kernel.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@arndb.de>
> > Signed-off-by: Ingo Molnar <mingo@kernel.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@infradead.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);

Patch
diff mbox series

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 5b7ed6d..db24f8c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -82,12 +82,12 @@  struct user;
 
 #ifdef CONFIG_PREEMPT_VOLUNTARY
 
-extern int __cond_resched(void);
+extern long __cond_resched(void);
 # define might_resched() __cond_resched()
 
 #elif defined(CONFIG_PREEMPT_DYNAMIC)
 
-extern int __cond_resched(void);
+extern long __cond_resched(void);
 
 DECLARE_STATIC_CALL(might_resched, __cond_resched);
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index ef00bb2..b08080d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1875,20 +1875,20 @@  static inline int test_tsk_need_resched(struct task_struct *tsk)
  * cond_resched_lock() will drop the spinlock before scheduling,
  */
 #if !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC)
-extern int __cond_resched(void);
+extern long __cond_resched(void);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
 
 DECLARE_STATIC_CALL(cond_resched, __cond_resched);
 
-static __always_inline int _cond_resched(void)
+static __always_inline long _cond_resched(void)
 {
 	return static_call_mod(cond_resched)();
 }
 
 #else
 
-static inline int _cond_resched(void)
+static inline long _cond_resched(void)
 {
 	return __cond_resched();
 }
@@ -1897,7 +1897,7 @@  static inline int _cond_resched(void)
 
 #else
 
-static inline int _cond_resched(void) { return 0; }
+static inline long _cond_resched(void) { return 0; }
 
 #endif /* !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC) */
 
@@ -1906,9 +1906,9 @@  static inline int _cond_resched(void) { return 0; }
 	_cond_resched();			\
 })
 
-extern int __cond_resched_lock(spinlock_t *lock);
-extern int __cond_resched_rwlock_read(rwlock_t *lock);
-extern int __cond_resched_rwlock_write(rwlock_t *lock);
+extern long __cond_resched_lock(spinlock_t *lock);
+extern long __cond_resched_rwlock_read(rwlock_t *lock);
+extern long __cond_resched_rwlock_write(rwlock_t *lock);
 
 #define cond_resched_lock(lock) ({				\
 	___might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET);\
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9819121..927fd82 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6976,7 +6976,7 @@  SYSCALL_DEFINE0(sched_yield)
 }
 
 #if !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC)
-int __sched __cond_resched(void)
+long __sched __cond_resched(void)
 {
 	if (should_resched(0)) {
 		preempt_schedule_common();
@@ -7006,7 +7006,7 @@  EXPORT_STATIC_CALL_TRAMP(might_resched);
  * operations here to prevent schedule() from being called twice (once via
  * spin_unlock(), once by hand).
  */
-int __cond_resched_lock(spinlock_t *lock)
+long __cond_resched_lock(spinlock_t *lock)
 {
 	int resched = should_resched(PREEMPT_LOCK_OFFSET);
 	int ret = 0;
@@ -7026,7 +7026,7 @@  int __cond_resched_lock(spinlock_t *lock)
 }
 EXPORT_SYMBOL(__cond_resched_lock);
 
-int __cond_resched_rwlock_read(rwlock_t *lock)
+long __cond_resched_rwlock_read(rwlock_t *lock)
 {
 	int resched = should_resched(PREEMPT_LOCK_OFFSET);
 	int ret = 0;
@@ -7046,7 +7046,7 @@  int __cond_resched_rwlock_read(rwlock_t *lock)
 }
 EXPORT_SYMBOL(__cond_resched_rwlock_read);
 
-int __cond_resched_rwlock_write(rwlock_t *lock)
+long __cond_resched_rwlock_write(rwlock_t *lock)
 {
 	int resched = should_resched(PREEMPT_LOCK_OFFSET);
 	int ret = 0;