Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753742AbbLOD0o (ORCPT ); Mon, 14 Dec 2015 22:26:44 -0500 Received: from mga04.intel.com ([192.55.52.120]:44099 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752552AbbLOD0n (ORCPT ); Mon, 14 Dec 2015 22:26:43 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,430,1444719600"; d="scan'208";a="13259812" Message-ID: <566F8871.7070408@linux.intel.com> Date: Tue, 15 Dec 2015 11:26:41 +0800 From: "Zhang, Yanmin" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Steven Rostedt , "Qiu, PeiyangX" CC: linux-kernel@vger.kernel.org, mingo@redhat.com, Rusty Russell Subject: Re: [PATCH] ftrace: fix race between ftrace and insmod References: <566E3076.5080708@intel.com> <566E3482.1090008@intel.com> <20151214105128.780b8bac@gandalf.local.home> <566F675C.9000904@linux.intel.com> In-Reply-To: <566F675C.9000904@linux.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2858 Lines: 63 On 2015/12/15 9:05, Zhang, Yanmin wrote: > On 2015/12/14 23:51, Steven Rostedt wrote: >> On Mon, 14 Dec 2015 11:16:18 +0800 >> "Qiu, PeiyangX" wrote: >> >>> 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 >> First, this patch has major whitespace damage. All tabs are now spaces! > This seems very hackish, although I can't think of a better way at the > moment. But I would like not to add more code into module.c if > possible, and just use a notifier unless there's a real reason we can't > (as there was when we added the hook in the first place). > We would double-check it again. It's not a good idea to change common > module codes. I double-checked it. If using notifier, we have to add 2 new module notification events in enum module_state. For example, MODULE_STATE_INITING, MODULE_STATE_INITING_FAILED. At MODULE_STATE_INITING, call ftrace_module_init(mod); At MODULE_STATE_INITING_FAILED, call ftrace_release_mod (or similar function); At MODULE_STATE_COMING, call a new function which links the new start_pg to ftrace_pages. Such design can also fix another bug in current kernel that ftrace start_pg of the module is not cleaned up if complete_formation fails. However, we change module common codes. The new events only serve ftrace. If not holding ftrace_mutex across start_pg setup and complete_formation, some other variables are not protected well, such like ftrace_pages, ftrace_start_up, and so on. Yanmin -- 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/