| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200714155417.GZ10769@hirez.programming.kicks-ass.net>
Date: Tue, 14 Jul 2020 17:54:17 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, mhiramat@...nel.org,
bristot@...hat.com, jbaron@...mai.com,
torvalds@...ux-foundation.org, tglx@...utronix.de,
mingo@...nel.org, namit@...are.com, hpa@...or.com, luto@...nel.org,
ard.biesheuvel@...aro.org, jpoimboe@...hat.com,
pbonzini@...hat.com, mathieu.desnoyers@...icios.com,
linux@...musvillemoes.dk
Subject: Re: [PATCH v6 15/17] static_call: Allow early init
On Tue, Jul 14, 2020 at 10:16:36AM -0400, Steven Rostedt wrote:
> > That's just how the code was; it treated vmlinux as the NULL module, and
> > as such needed a static_call_mod allocated to host the static_call_sites
> > pointer.
> >
> > That is, the static_call_key has a single linked list pointer to
> > static_call_mod, and every module has an entry on that list with a
> > pointer to their sites. Very simple and straight forward.
> >
> > Except it requires an allocation to set up, which is a pain is you want
> > it initialized very early.
>
> OK, I'm still missing something, but having a hard time explaining
> exactly what that is. ;-)
>
> I guess that is, why did moving the initialization early require an
> allocation where initializing it later did not? What allocation are we
> avoiding?
The other way around. Before this patch initialization required an
allocation, with this patch init no longer requires one.
> I'm not seeing why this trick is needed when we moved the init early as
> compared to doing the same thing later on.
We use the trick to avoid the alloc, which is what enables early init.
So, before:
- init required alloc
- alloc prohibits early use
after:
- init no longer require alloc
- because horrible pointer reuse
- early use possible
> > @@ -192,16 +222,35 @@ static int __static_call_init(struct mod
> > if (key != prev_key) {
> > prev_key = key;
> >
> > + if (!mod) {
> > + key->sites = site;
> > + key->type |= 1;
> > + goto do_transform;
> > + }
> > +
>
> We want to avoid calling kzalloc() early?
>
> If so, this should have a comment here stating so and why.
Fair enough; and I think I even see a further optimiation.
How's this:
---
Subject: static_call: Allow early init
From: Peter Zijlstra <peterz@...radead.org>
Date: Fri, 4 Oct 17:21:10 CEST 2019
In order to use static_call() to wire up x86_pmu, we need to
initialize earlier; copy some of the tricks from jump_label to enable
this.
Primarily we overload key->next to store a sites pointer when there
are no modules, this avoids having to use kmalloc() to initialize the
sites and allows us to run much earlier.
(arguably, this is much much earlier than needed for perf, but it
might allow other uses.)
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
arch/x86/kernel/setup.c | 2 +
arch/x86/kernel/static_call.c | 8 ++++-
include/linux/static_call.h | 15 ++++++++-
kernel/static_call.c | 67 +++++++++++++++++++++++++++++++++++++++---
4 files changed, 85 insertions(+), 7 deletions(-)
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -19,6 +19,7 @@
#include <linux/hugetlb.h>
#include <linux/tboot.h>
#include <linux/usb/xhci-dbgp.h>
+#include <linux/static_call.h>
#include <uapi/linux/mount.h>
@@ -848,6 +849,7 @@ void __init setup_arch(char **cmdline_p)
early_cpu_init();
arch_init_ideal_nops();
jump_label_init();
+ static_call_init();
early_ioremap_init();
setup_olpc_ofw_pgd();
--- a/arch/x86/kernel/static_call.c
+++ b/arch/x86/kernel/static_call.c
@@ -11,7 +11,7 @@ enum insn_type {
RET = 3, /* tramp / site cond-tail-call */
};
-static void __static_call_transform(void *insn, enum insn_type type, void *func)
+static void __ref __static_call_transform(void *insn, enum insn_type type, void *func)
{
int size = CALL_INSN_SIZE;
const void *code;
@@ -33,11 +33,17 @@ static void __static_call_transform(void
code = text_gen_insn(RET_INSN_OPCODE, insn, func);
size = RET_INSN_SIZE;
break;
+
+ default: /* GCC is a moron -- it figures @code can be uninitialized below */
+ BUG();
}
if (memcmp(insn, code, size) == 0)
return;
+ if (unlikely(system_state == SYSTEM_BOOTING))
+ return text_poke_early(insn, code, size);
+
text_poke_bp(insn, code, size, NULL);
}
--- a/include/linux/static_call.h
+++ b/include/linux/static_call.h
@@ -136,6 +136,8 @@ extern void arch_static_call_transform(v
#ifdef CONFIG_HAVE_STATIC_CALL_INLINE
+extern void __init static_call_init(void);
+
struct static_call_mod {
struct static_call_mod *next;
struct module *mod; /* for vmlinux, mod == NULL */
@@ -144,7 +146,12 @@ struct static_call_mod {
struct static_call_key {
void *func;
- struct static_call_mod *mods;
+ union {
+ /* bit 0: 0 = mods, 1 = sites */
+ unsigned long type;
+ struct static_call_mod *mods;
+ struct static_call_site *sites;
+ };
};
extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
@@ -155,7 +162,7 @@ extern int static_call_text_reserved(voi
DECLARE_STATIC_CALL(name, _func); \
struct static_call_key STATIC_CALL_KEY(name) = { \
.func = _func, \
- .mods = NULL, \
+ .type = 1, \
}; \
ARCH_DEFINE_STATIC_CALL_TRAMP(name, _func)
@@ -180,6 +187,8 @@ extern int static_call_text_reserved(voi
#elif defined(CONFIG_HAVE_STATIC_CALL)
+static inline void static_call_init(void) { }
+
struct static_call_key {
void *func;
};
@@ -225,6 +234,8 @@ static inline int static_call_text_reser
#else /* Generic implementation */
+static inline void static_call_init(void) { }
+
struct static_call_key {
void *func;
};
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -94,10 +94,31 @@ static inline void static_call_sort_entr
static_call_site_cmp, static_call_site_swap);
}
+static inline bool static_call_key_has_mods(struct static_call_key *key)
+{
+ return !(key->type & 1);
+}
+
+static inline struct static_call_mod *static_call_key_next(struct static_call_key *key)
+{
+ if (!static_call_key_has_mods(key))
+ return NULL;
+
+ return key->mods;
+}
+
+static inline struct static_call_site *static_call_key_sites(struct static_call_key *key)
+{
+ if (static_call_key_has_mods(key))
+ return NULL;
+
+ return (struct static_call_site *)(key->type & ~1);
+}
+
void __static_call_update(struct static_call_key *key, void *tramp, void *func)
{
struct static_call_site *site, *stop;
- struct static_call_mod *site_mod;
+ struct static_call_mod *site_mod, first;
cpus_read_lock();
static_call_lock();
@@ -116,13 +137,22 @@ void __static_call_update(struct static_
if (WARN_ON_ONCE(!static_call_initialized))
goto done;
- for (site_mod = key->mods; site_mod; site_mod = site_mod->next) {
+ first = (struct static_call_mod){
+ .next = static_call_key_next(key),
+ .mod = NULL,
+ .sites = static_call_key_sites(key),
+ };
+
+ for (site_mod = &first; site_mod; site_mod = site_mod->next) {
struct module *mod = site_mod->mod;
if (!site_mod->sites) {
/*
* This can happen if the static call key is defined in
* a module which doesn't use it.
+ *
+ * It also happens in the has_mods case, where the
+ * 'first' entry has no sites associated with it.
*/
continue;
}
@@ -192,16 +222,45 @@ static int __static_call_init(struct mod
if (key != prev_key) {
prev_key = key;
+ /*
+ * For vmlinux (!mod) avoid the allocation by storing
+ * the sites pointer in the key itself. Also see
+ * __static_call_update()'s @first.
+ */
+ if (!mod) {
+ key->sites = site;
+ key->type |= 1;
+ goto do_transform;
+ }
+
site_mod = kzalloc(sizeof(*site_mod), GFP_KERNEL);
if (!site_mod)
return -ENOMEM;
+ /*
+ * When the key has a direct sites pointer, extract
+ * that into an explicit struct static_call_mod, so we
+ * can have a list of modules.
+ */
+ if (static_call_key_sites(key)) {
+ site_mod->mod = NULL;
+ site_mod->next = NULL;
+ site_mod->sites = static_call_key_sites(key);
+
+ key->mods = site_mod;
+
+ site_mod = kzalloc(sizeof(*site_mod), GFP_KERNEL);
+ if (!site_mod)
+ return -ENOMEM;
+ }
+
site_mod->mod = mod;
site_mod->sites = site;
- site_mod->next = key->mods;
+ site_mod->next = static_call_key_next(key);
key->mods = site_mod;
}
+do_transform:
arch_static_call_transform(site_addr, NULL, key->func,
static_call_is_tail(site));
}
@@ -348,7 +407,7 @@ int static_call_text_reserved(void *star
return __static_call_mod_text_reserved(start, end);
}
-static void __init static_call_init(void)
+void __init static_call_init(void)
{
int ret;
Powered by blists - more mailing lists