[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKv+Gu9bMkFEx_GxH3uLm0gveQurp7ELDi6C5NvpugufPFMbRA@mail.gmail.com>
Date: Fri, 9 Nov 2018 14:39:17 +0100
From: Ard Biesheuvel <ard.biesheuvel@...aro.org>
To: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"the arch/x86 maintainers" <x86@...nel.org>,
Andy Lutomirski <luto@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Jason Baron <jbaron@...mai.com>, Jiri Kosina <jkosina@...e.cz>,
David Laight <David.Laight@...lab.com>,
Borislav Petkov <bp@...en8.de>
Subject: Re: [RFC PATCH 1/3] static_call: Add static call infrastructure
On 8 November 2018 at 22:15, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> Add a static call infrastructure. Static calls use code patching to
> hard-code function pointers into direct branch instructions. They give
> the flexibility of function pointers, but with improved performance.
> This is especially important for cases where retpolines would otherwise
> be used, as retpolines can significantly impact performance.
>
> This code is heavily inspired by the jump label code (aka "static
> jumps"), as some of the concepts are very similar.
>
> There are three implementations, depending on arch support:
>
> 1) optimized: patched call sites (CONFIG_HAVE_STATIC_CALL_OPTIMIZED)
> 2) unoptimized: patched trampolines (CONFIG_HAVE_STATIC_CALL_UNOPTIMIZED)
> 3) basic function pointers
>
> For more details, see the comments in include/linux/static_call.h.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
> arch/Kconfig | 6 +
> include/asm-generic/vmlinux.lds.h | 11 ++
> include/linux/module.h | 10 +
> include/linux/static_call.h | 186 +++++++++++++++++++
> include/linux/static_call_types.h | 19 ++
> kernel/Makefile | 1 +
> kernel/module.c | 5 +
> kernel/static_call.c | 297 ++++++++++++++++++++++++++++++
> 8 files changed, 535 insertions(+)
> create mode 100644 include/linux/static_call.h
> create mode 100644 include/linux/static_call_types.h
> create mode 100644 kernel/static_call.c
>
> diff --git a/kernel/static_call.c b/kernel/static_call.c
> new file mode 100644
> index 000000000000..599ebc6fc4f1
> --- /dev/null
> +++ b/kernel/static_call.c
...
> +static void __init static_call_init(void)
> +{
> + struct static_call_site *start = __start_static_call_sites;
> + struct static_call_site *stop = __stop_static_call_sites;
> + struct static_call_site *site;
> +
> + if (start == stop) {
> + pr_warn("WARNING: empty static call table\n");
> + return;
> + }
> +
> + cpus_read_lock();
> + static_call_lock();
> +
> + static_call_sort_entries(start, stop);
> +
> + for (site = start; site < stop; site++) {
> + struct static_call_key *key = static_call_key(site);
> + unsigned long addr = static_call_addr(site);
> +
> + if (list_empty(&key->site_mods)) {
> + struct static_call_mod *mod;
> +
> + mod = kzalloc(sizeof(*mod), GFP_KERNEL);
> + if (!mod) {
> + WARN(1, "Failed to allocate memory for static calls");
> + return;
> + }
> +
> + mod->sites = site;
> + list_add_tail(&mod->list, &key->site_mods);
> +
> + /*
> + * The trampoline should no longer be used. Poison it
> + * it with a BUG() to catch any stray callers.
> + */
> + arch_static_call_poison_tramp(addr);
This patches the wrong thing: the trampoline is at key->func not addr.
However, patching it here means we poison it before all users are
patched. I added this on top
diff --git a/kernel/static_call.c b/kernel/static_call.c
index 599ebc6fc4f1..d9562329bec6 100644
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -248,6 +248,7 @@ static void __init static_call_init(void)
struct static_call_site *start = __start_static_call_sites;
struct static_call_site *stop = __stop_static_call_sites;
struct static_call_site *site;
+ struct static_call_key *prev_key = NULL;
if (start == stop) {
pr_warn("WARNING: empty static call table\n");
@@ -279,7 +280,9 @@ static void __init static_call_init(void)
* The trampoline should no longer be used. Poison it
* it with a BUG() to catch any stray callers.
*/
- arch_static_call_poison_tramp(addr);
+ if (prev_key)
+
arch_static_call_poison_tramp((unsigned long)prev_key->func);
+ prev_key = key;
}
arch_static_call_transform(addr, key->func);
> + }
> +
> + arch_static_call_transform(addr, key->func);
> + }
> +
> + static_call_initialized = true;
> +
> + static_call_unlock();
> + cpus_read_unlock();
> +
> +#ifdef CONFIG_MODULES
> + register_module_notifier(&static_call_module_nb);
> +#endif
> +}
> +early_initcall(static_call_init);
> --
> 2.17.2
>
Powered by blists - more mailing lists