[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <566E3076.5080708@intel.com>
Date: Mon, 14 Dec 2015 10:59:02 +0800
From: "Qiu, PeiyangX" <peiyangx.qiu@...el.com>
To: linux-kernel@...r.kernel.org
Cc: yanmin_zhang@...ux.intel.com,
"rostedt@...dmis.org; mingo"@redhat.com
Subject: [PATCH] ftrace: fix race between ftrace and insmod
We hit ftrace_bug report when booting Android on a 64bit ATOM SOC chip.
Basically, there is a race between insmod and ftrace_run_update_code.
After load_module=>ftrace_module_init, another thread jumps in to call
ftrace_run_update_code=>ftrace_arch_code_modify_prepare
=>set_all_modules_text_rw, to change all modules
as RW. Since the new module is at MODULE_STATE_UNFORMED, the text attribute
is not changed. Then, the 2nd thread goes ahead to change codes.
However, load_module continues to call
complete_formation=>set_section_ro_nx,
then 2nd thread would fail when probing the module's TEXT.
Below patch tries to resolve it.
commit a949ae560a511fe4e3adf48fa44fefded93e5c2b
Author: Steven Rostedt (Red Hat) <rostedt@...dmis.org>
Date: Thu Apr 24 10:40:12 2014 -0400
ftrace/module: Hardcode ftrace_module_init() call into load_module()
But it can't fully resolve the issue.
THis patch holds ftrace_mutex across ftrace_module_init and
complete_formation.
Signed-off-by: Qiu Peiyang <peiyangx.qiu@...el.com>
Signed-off-by: Zhang Yanmin <yanmin.zhang@...el.com>
---
include/linux/ftrace.h | 6 ++++--
kernel/module.c | 3 ++-
kernel/trace/ftrace.c | 33 ++++++++++++++++++++++-----------
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index eae6548..4adc279 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -585,7 +585,8 @@ static inline int ftrace_modify_call(struct
dyn_ftrace *rec, unsigned long old_a
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_init_start(struct module *mod);
+extern void ftrace_module_init_end(struct module *mod);
extern void ftrace_disable_daemon(void);
extern void ftrace_enable_daemon(void);
@@ -595,7 +596,8 @@ 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_release_mod(struct module *mod) {}
-static inline void ftrace_module_init(struct module *mod) {}
+static inline void ftrace_module_init_start(struct module *mod) {}
+static inline void ftrace_module_init_end(struct module *mod) {}
static inline __init int register_ftrace_command(struct
ftrace_func_command *cmd)
{
return -EINVAL;
diff --git a/kernel/module.c b/kernel/module.c
index 8f051a1..324c5c6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3506,10 +3506,11 @@ static int load_module(struct load_info *info,
const char __user *uargs,
dynamic_debug_setup(info->debug, info->num_debug);
/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
- ftrace_module_init(mod);
+ ftrace_module_init_start(mod);
/* Finally it's fully formed, ready to start executing. */
err = complete_formation(mod, info);
+ ftrace_module_init_end(mod);
if (err)
goto ddebug_cleanup;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3f743b1..436c199 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4795,7 +4795,7 @@ static int ftrace_cmp_ips(const void *a, const
void *b)
return 0;
}
-static int ftrace_process_locs(struct module *mod,
+static int ftrace_process_locs_nolock(struct module *mod,
unsigned long *start,
unsigned long *end)
{
@@ -4820,8 +4820,6 @@ static int ftrace_process_locs(struct module *mod,
if (!start_pg)
return -ENOMEM;
- mutex_lock(&ftrace_lock);
-
/*
* Core and each module needs their own pages, as
* modules will free them when they are removed.
@@ -4889,8 +4887,18 @@ static int ftrace_process_locs(struct module *mod,
local_irq_restore(flags);
ret = 0;
out:
- mutex_unlock(&ftrace_lock);
+ return ret;
+}
+static int ftrace_process_locs(struct module *mod,
+ unsigned long *start,
+ unsigned long *end)
+{
+ int ret;
+
+ mutex_lock(&ftrace_lock);
+ ret = ftrace_process_locs_nolock(mod, start, end);
+ mutex_unlock(&ftrace_lock);
return ret;
}
@@ -4940,19 +4948,22 @@ void ftrace_release_mod(struct module *mod)
mutex_unlock(&ftrace_lock);
}
-static void ftrace_init_module(struct module *mod,
- unsigned long *start, unsigned long *end)
+void ftrace_module_init_start(struct module *mod)
{
+ unsigned long *start = mod->ftrace_callsites;
+ unsigned long *end = mod->ftrace_callsites + mod->num_ftrace_callsites;
+
+ mutex_lock(&ftrace_lock);
+
if (ftrace_disabled || start == end)
return;
- ftrace_process_locs(mod, start, end);
+
+ ftrace_process_locs_nolock(mod, start, end);
}
-void ftrace_module_init(struct module *mod)
+void ftrace_module_init_end(struct module *mod)
{
- ftrace_init_module(mod, mod->ftrace_callsites,
- mod->ftrace_callsites +
- mod->num_ftrace_callsites);
+ mutex_unlock(&ftrace_lock);
}
static int ftrace_module_notify_exit(struct notifier_block *self,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists