linux-toolchains.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: linux-toolchains@vger.kernel.org
Subject: Function pointer attributes
Date: Wed, 25 Nov 2020 11:52:14 +0100	[thread overview]
Message-ID: <20201125105214.GQ2414@hirez.programming.kicks-ass.net> (raw)

Hi all!

Would it be possible to declare a function pointer that only takes
functions from a specific section? In particular I'm trying to convert
idle to noinstr and chasing through the maze of function pointers is
painful.

It would be immensely helpful if we could declare a function pointer
itself as noinstr such that an assignment by a non-noinstr function
would generate a compiler warn.

Something like: void (* noinstr x86_idle)(void), fails horribly
(understandable).

Now, part of noinstr is a section attribute, so just being able to limit
a function pointer to that specific section would be sufficient.

The alternative is playing dodgy games with the function signature, but
that is quite aweful indeed, see below for an example. But that's gross
and I really don't want to sprinkle that throughout the tree (there's a
_LOT_ of idle code).


---
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 145a7ac0c19a..d11c13be2278 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -652,7 +652,10 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p)
 unsigned long boot_option_idle_override = IDLE_NO_OVERRIDE;
 EXPORT_SYMBOL(boot_option_idle_override);
 
-static void (*x86_idle)(void);
+typedef struct noinstr_struct { } noinstr_t;
+#define noinstr_return return (noinstr_t){}
+
+static noinstr_t (*x86_idle)(void);
 
 #ifndef CONFIG_SMP
 static inline void play_dead(void)
@@ -675,7 +678,7 @@ void arch_cpu_idle_dead(void)
 /*
  * Called from the generic idle code.
  */
-void arch_cpu_idle(void)
+noinstr void arch_cpu_idle(void)
 {
 	x86_idle();
 }
@@ -691,12 +694,18 @@ void __cpuidle default_idle(void)
 EXPORT_SYMBOL(default_idle);
 #endif
 
+static noinstr noinstr_t halt_idle(void)
+{
+	raw_safe_halt();
+	noinstr_return;
+}
+
 #ifdef CONFIG_XEN
 bool xen_set_default_idle(void)
 {
 	bool ret = !!x86_idle;
 
-	x86_idle = default_idle;
+	x86_idle = halt_idle;
 
 	return ret;
 }
@@ -739,21 +748,19 @@ void stop_this_cpu(void *dummy)
  *
  * XXX this function is completely buggered vs RCU and tracing.
  */
-static void amd_e400_idle(void)
+static noinstr noinstr_t amd_e400_idle(void)
 {
 	/*
 	 * We cannot use static_cpu_has_bug() here because X86_BUG_AMD_APIC_C1E
 	 * gets set after static_cpu_has() places have been converted via
 	 * alternatives.
 	 */
-	if (!boot_cpu_has_bug(X86_BUG_AMD_APIC_C1E)) {
-		default_idle();
-		return;
-	}
+	if (!boot_cpu_has_bug(X86_BUG_AMD_APIC_C1E))
+		return halt_idle();
 
 	tick_broadcast_enter();
 
-	default_idle();
+	halt_idle();
 
 	/*
 	 * The switch back from broadcast mode needs to be called with
@@ -762,6 +769,7 @@ static void amd_e400_idle(void)
 	raw_local_irq_disable();
 	tick_broadcast_exit();
 	raw_local_irq_enable();
+	noinstr_return;
 }
 
 /*
@@ -790,7 +798,7 @@ static int prefer_mwait_c1_over_halt(const struct cpuinfo_x86 *c)
  * with interrupts enabled and no flags, which is backwards compatible with the
  * original MWAIT implementation.
  */
-static __cpuidle void mwait_idle(void)
+static noinstr noinstr_t mwait_idle(void)
 {
 	if (!current_set_polling_and_test()) {
 		if (this_cpu_has(X86_BUG_CLFLUSH_MONITOR)) {
@@ -808,6 +816,7 @@ static __cpuidle void mwait_idle(void)
 		raw_local_irq_enable();
 	}
 	__current_clr_polling();
+	noinstr_return;
 }
 
 void select_idle_routine(const struct cpuinfo_x86 *c)
@@ -826,7 +835,7 @@ void select_idle_routine(const struct cpuinfo_x86 *c)
 		pr_info("using mwait in idle threads\n");
 		x86_idle = mwait_idle;
 	} else
-		x86_idle = default_idle;
+		x86_idle = halt_idle;
 }
 
 void amd_e400_c1e_apic_setup(void)
@@ -879,7 +888,7 @@ static int __init idle_setup(char *str)
 		 * To continue to load the CPU idle driver, don't touch
 		 * the boot_option_idle_override.
 		 */
-		x86_idle = default_idle;
+		x86_idle = halt_idle;
 		boot_option_idle_override = IDLE_HALT;
 	} else if (!strcmp(str, "nomwait")) {
 		/*

                 reply	other threads:[~2020-11-25 10:52 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201125105214.GQ2414@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=linux-toolchains@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).