Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752107AbbLNC7I (ORCPT ); Sun, 13 Dec 2015 21:59:08 -0500 Received: from mga09.intel.com ([134.134.136.24]:38121 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751737AbbLNC7H (ORCPT ); Sun, 13 Dec 2015 21:59:07 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,425,1444719600"; d="scan'208";a="840458291" Subject: [PATCH] ftrace: fix race between ftrace and insmod To: linux-kernel@vger.kernel.org References: Cc: yanmin_zhang@linux.intel.com, "rostedt@goodmis.org; mingo"@redhat.com From: "Qiu, PeiyangX" Message-ID: <566E3076.5080708@intel.com> Date: Mon, 14 Dec 2015 10:59:02 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5463 Lines: 159 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) 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 Signed-off-by: Zhang Yanmin --- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/