(2014/04/23 10:56), Steven Rostedt wrote: > On Wed, 23 Apr 2014 10:26:00 +0900 > Masami Hiramatsu <masami.hiramatsu...@hitachi.com> wrote: > > >> Agreed. That should be done in a protected (critical) region, >> and the region must be protected by correct lock. It seems that >> the ftrace_lock is not a correct one. > > The setting of RO to RW done by ftrace before doing the normal > modification is under the ftrace_lock mutex. Why wouldn't that be the > correct lock?
Hmm, Ok. I checked that currently ftrace is the only user of set_all_modules_text_rw(), so until another user appears, ftrace_lock mutex can work. (and also, we need a comment on the top of such functions, about by what it is protected. ) > The issue today is with the loading of a module and ftrace > expecting its code to be RW. Here's the current race: > > > CPU 1 CPU 2 > ----- ----- > load_module() > module->state = MODULE_STATE_COMING > > register_ftrace_function() > mutex_lock(&ftrace_lock); > ftrace_startup() > update_ftrace_function(); > ftrace_arch_code_modify_prepare() > set_all_module_text_rw(); > <enables-ftrace> > ftrace_arch_code_modify_post_process() > set_all_module_text_ro(); > > [ here all module text is set to RO, > including the module that is > loading!! ] > > blocking_notifier_call_chain(MODULE_STATE_COMING); > ftrace_init_module() > > > [ tries to modify code, but it's RO, and fails! ] > > One solution is to add a way to set a single module text to ro and rw, > and then we can encapsulate ftrace_init_module() under ftrace_lock > mutex and have the ftrace_init_module() set the text to RW and then > back to RO, and this will keep ftrace from having issues with the > loaded module. It sounds nicer solution, less side-effect. > Now, if text poke does something similar, we need to make another mutex > that covers modifying text. Don't we have one already? We have the text_mutex already :). > The worry I have here, and why I still prefer the simple split state of > MODULE_STATE_COMING, is that once you add another mutex, we now have to > fight mutex ordering. Not to mention where else things might do this :-p I see, however, we should take care of it, at least comment level. Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
