From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932691AbcA2RaO (ORCPT ); Fri, 29 Jan 2016 12:30:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54428 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752748AbcA2RaM (ORCPT ); Fri, 29 Jan 2016 12:30:12 -0500 Date: Fri, 29 Jan 2016 11:30:10 -0600 From: Josh Poimboeuf To: Miroslav Benes Cc: Jessica Yu , Seth Jennings , Jiri Kosina , Vojtech Pavlik , Steven Rostedt , Ingo Molnar , live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] livepatch: Implement separate coming and going module notifiers Message-ID: <20160129173010.GA19101@treble.redhat.com> References: <1454049827-3726-1-git-send-email-jeyu@redhat.com> <1454049827-3726-2-git-send-email-jeyu@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 29, 2016 at 05:30:46PM +0100, Miroslav Benes wrote: > Otherwise than that it looks good. I agree there are advantages to split > the notifiers. For example we can replace the coming one with the function > call somewhere in load_module() to improve error handling if the patching > fails while loading a module. This would be handy with a consistency model > in the future. Yeah, we'll need something like that eventually. Though we'll need to make sure that ftrace_module_enable() is still called beforehand, after setting MODULE_STATE_COMING state, due to the race described in 5156dca. Something like: [note: klp_module_notify_coming() is replaced with klp_module_enable()] diff --git a/kernel/module.c b/kernel/module.c index 8358f46..aeabd81 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -3371,6 +3371,13 @@ static int complete_formation(struct module *mod, struct load_info *info) mod->state = MODULE_STATE_COMING; mutex_unlock(&module_mutex); + ftrace_module_enable(mod); + err = klp_module_enable(mod); + if (err) { + ftrace_release_mod(mod); + return err; + } + blocking_notifier_call_chain(&module_notify_list, MODULE_STATE_COMING, mod); return 0; diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index eca592f..c42cf37 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -5045,9 +5045,6 @@ static int ftrace_module_notify(struct notifier_block *self, struct module *mod = data; switch (val) { - case MODULE_STATE_COMING: - ftrace_module_enable(mod); - break; case MODULE_STATE_GOING: ftrace_release_mod(mod); break; -- Josh