Re: [PATCH v4 16/18] static_call: Allow early init

Josh Poimboeuf
2020-05-06T21:16:07Z

On Fri, May 01, 2020 at 10:29:05PM +0200, Peter Zijlstra wrote:
> 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@infradead.org>
> ---
> arch/x86/kernel/setup.c | 2 +
> arch/x86/kernel/static_call.c | 8 +++++-
> include/linux/static_call.h | 15 ++++++++++--
> kernel/static_call.c | 52 +++++++++++++++++++++++++++++++++++++++---
> 4 files changed, 71 insertions(+), 6 deletions(-)

This doesn't work when the key is defined in a module. In
__static_call_update(), first.site_mod->mod is NULL, but
static_call_key_sites() points to the module's call sites.

This seems to fix it (sorry, also has the 'next' -> 'mods' rename).
The actual fix is in static_call_key_sites() and static_call_key_mods().

diff --git a/include/linux/static_call.h b/include/linux/static_call.h
index e112553cfce6..ee55075707f1 100644
--- a/include/linux/static_call.h
+++ b/include/linux/static_call.h
@@ -110,9 +110,10 @@ struct static_call_mod {
struct static_call_key {
void *func;
union {
- /* bit0 => 0 - next, 1 - sites */
+ /* bit 0: 0 = mods; 1 = sites */
unsigned long type;
- struct static_call_mod *next;
+
+ struct static_call_mod *mods;
struct static_call_site *sites;
};
};
diff --git a/kernel/static_call.c b/kernel/static_call.c
index 783953db28c4..d78e50c5b8a3 100644
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -94,23 +94,23 @@ static inline void static_call_sort_entries(struct static_call_site *start,
static_call_site_cmp, static_call_site_swap);
}

-static inline bool static_call_key_has_next(struct static_call_key *key)
+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)
+static inline struct static_call_mod *static_call_key_mods(struct static_call_key *key)
{
- if (static_call_key_has_next(key))
- return key->next->next;
+ if (!static_call_key_has_mods(key))
+ return NULL;

- 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_next(key))
- return key->next->sites;
+ if (static_call_key_has_mods(key))
+ return NULL;

return (struct static_call_site *)(key->type & ~1);
}
@@ -118,7 +118,12 @@ static inline struct static_call_site *static_call_key_sites(struct static_call_
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, first;
+ struct static_call_mod *site_mod;
+ struct static_call_mod first = {
+ .next = static_call_key_mods(key),
+ .mod = NULL,
+ .sites = static_call_key_sites(key),
+ };

cpus_read_lock();
static_call_lock();
@@ -137,17 +142,14 @@ void __static_call_update(struct static_call_key *key, void *tramp, void *func)
if (WARN_ON_ONCE(!static_call_initialized))
goto done;

- 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) {
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;
}
@@ -228,12 +230,12 @@ static int __static_call_init(struct module *mod,
if (!site_mod)
return -ENOMEM;

- if (!static_call_key_has_next(key)) {
+ if (!static_call_key_has_mods(key)) {
site_mod->mod = NULL;
site_mod->next = NULL;
site_mod->sites = static_call_key_sites(key);

- key->next = site_mod;
+ key->mods = site_mod;

site_mod = kzalloc(sizeof(*site_mod), GFP_KERNEL);
if (!site_mod)
@@ -242,8 +244,8 @@ static int __static_call_init(struct module *mod,

site_mod->mod = mod;
site_mod->sites = site;
- site_mod->next = key->next;
- key->next = site_mod;
+ site_mod->next = key->mods;
+ key->mods = site_mod;
}

do_transform:
@@ -328,7 +330,7 @@ static void static_call_del_module(struct module *mod)

prev_key = key;

- for (prev = &key->next, site_mod = key->next;
+ for (prev = &key->mods, site_mod = key->mods;
site_mod && site_mod->mod != mod;
prev = &site_mod->next, site_mod = site_mod->next)
;