[PATCH v4 16/16] ftrace: Merge ftrace_module_{init,enable}()

From: Peter Zijlstra
Date: Fri Oct 18 2019 - 03:52:55 EST


Because of how some architectures used set_all_modules_text_*() there
was a dependency between the module state and its memory protection
state. This then required ftrace to be split into two functions, see
commit:

a949ae560a51 ("ftrace/module: Hardcode ftrace_module_init() call into load_module()")

Now that set_all_modules_text_*() is dead and burried, this is no
longer relevant and we can merge the ftrace_module hooks again.

Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
---
include/linux/ftrace.h | 2
kernel/module.c | 3 -
kernel/trace/ftrace.c | 124 +++++++++++++++++++------------------------------
3 files changed, 49 insertions(+), 80 deletions(-)

--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -579,7 +579,6 @@ static inline int ftrace_modify_call(str
extern int ftrace_arch_read_dyn_info(char *buf, int size);

extern int skip_trace(unsigned long ip);
-extern void ftrace_module_init(struct module *mod);
extern void ftrace_module_enable(struct module *mod);
extern void ftrace_release_mod(struct module *mod);

@@ -590,7 +589,6 @@ static inline int skip_trace(unsigned lo
static inline int ftrace_force_update(void) { return 0; }
static inline void ftrace_disable_daemon(void) { }
static inline void ftrace_enable_daemon(void) { }
-static inline void ftrace_module_init(struct module *mod) { }
static inline void ftrace_module_enable(struct module *mod) { }
static inline void ftrace_release_mod(struct module *mod) { }
static inline int ftrace_text_reserved(const void *start, const void *end)
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3836,9 +3836,6 @@ static int load_module(struct load_info

dynamic_debug_setup(mod, info->debug, info->num_debug);

- /* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
- ftrace_module_init(mod);
-
/* Finally it's fully formed, ready to start executing. */
err = complete_formation(mod, info);
if (err)
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5564,6 +5564,8 @@ static int ftrace_cmp_ips(const void *a,
return 0;
}

+static int referenced_filters(struct dyn_ftrace *rec);
+
static int ftrace_process_locs(struct module *mod,
unsigned long *start,
unsigned long *end)
@@ -5656,6 +5658,49 @@ static int ftrace_process_locs(struct mo
ftrace_update_code(mod, start_pg);
if (!mod)
local_irq_restore(flags);
+
+#ifdef CONFIG_MODULES
+ if (ftrace_disabled || !mod)
+ goto out_loop;
+
+ do_for_each_ftrace_rec(pg, rec) {
+ int cnt;
+ /*
+ * do_for_each_ftrace_rec() is a double loop.
+ * module text shares the pg. If a record is
+ * not part of this module, then skip this pg,
+ * which the "break" will do.
+ */
+ if (!within_module_core(rec->ip, mod) &&
+ !within_module_init(rec->ip, mod))
+ break;
+
+ cnt = 0;
+
+ /*
+ * When adding a module, we need to check if tracers are
+ * currently enabled and if they are, and can trace this record,
+ * we need to enable the module functions as well as update the
+ * reference counts for those function records.
+ */
+ if (ftrace_start_up)
+ cnt += referenced_filters(rec);
+
+ /* This clears FTRACE_FL_DISABLED */
+ rec->flags = cnt;
+
+ if (ftrace_start_up && cnt) {
+ int failed = __ftrace_replace_code(rec, 1);
+ if (failed) {
+ ftrace_bug(failed, rec);
+ goto out_loop;
+ }
+ }
+
+ } while_for_each_ftrace_rec();
+
+ out_loop:
+#endif
ret = 0;
out:
mutex_unlock(&ftrace_lock);
@@ -5823,85 +5868,14 @@ void ftrace_release_mod(struct module *m

void ftrace_module_enable(struct module *mod)
{
- struct dyn_ftrace *rec;
- struct ftrace_page *pg;
-
- mutex_lock(&ftrace_lock);
-
- if (ftrace_disabled)
- goto out_unlock;
-
- /*
- * If the tracing is enabled, go ahead and enable the record.
- *
- * The reason not to enable the record immediately is the
- * inherent check of ftrace_make_nop/ftrace_make_call for
- * correct previous instructions. Making first the NOP
- * conversion puts the module to the correct state, thus
- * passing the ftrace_make_call check.
- *
- * We also delay this to after the module code already set the
- * text to read-only, as we now need to set it back to read-write
- * so that we can modify the text.
- */
- if (ftrace_start_up)
- ftrace_arch_code_modify_prepare();
-
- do_for_each_ftrace_rec(pg, rec) {
- int cnt;
- /*
- * do_for_each_ftrace_rec() is a double loop.
- * module text shares the pg. If a record is
- * not part of this module, then skip this pg,
- * which the "break" will do.
- */
- if (!within_module_core(rec->ip, mod) &&
- !within_module_init(rec->ip, mod))
- break;
-
- cnt = 0;
-
- /*
- * When adding a module, we need to check if tracers are
- * currently enabled and if they are, and can trace this record,
- * we need to enable the module functions as well as update the
- * reference counts for those function records.
- */
- if (ftrace_start_up)
- cnt += referenced_filters(rec);
-
- /* This clears FTRACE_FL_DISABLED */
- rec->flags = cnt;
-
- if (ftrace_start_up && cnt) {
- int failed = __ftrace_replace_code(rec, 1);
- if (failed) {
- ftrace_bug(failed, rec);
- goto out_loop;
- }
- }
-
- } while_for_each_ftrace_rec();
-
- out_loop:
- if (ftrace_start_up)
- ftrace_arch_code_modify_post_process();
-
- out_unlock:
- mutex_unlock(&ftrace_lock);
+ if (!(ftrace_disabled || !mod->num_ftrace_callsites)) {
+ ftrace_process_locs(mod, mod->ftrace_callsites,
+ mod->ftrace_callsites + mod->num_ftrace_callsites);
+ }

process_cached_mods(mod->name);
}

-void ftrace_module_init(struct module *mod)
-{
- if (ftrace_disabled || !mod->num_ftrace_callsites)
- return;
-
- ftrace_process_locs(mod, mod->ftrace_callsites,
- mod->ftrace_callsites + mod->num_ftrace_callsites);
-}
-
static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,
struct dyn_ftrace *rec)
{