All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Jessica Yu <jeyu@redhat.com>
Cc: Seth Jennings <sjenning@redhat.com>,
	Jiri Kosina <jikos@kernel.org>, Vojtech Pavlik <vojtech@suse.com>,
	Miroslav Benes <mbenes@suse.cz>,
	Rusty Russell <rusty@rustcorp.com.au>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 0/2] Fix ordering of ftrace/livepatch calls on module load and unload
Date: Mon, 8 Feb 2016 11:48:46 -0600	[thread overview]
Message-ID: <20160208174846.GB23106@treble.redhat.com> (raw)
In-Reply-To: <1454728097-7106-1-git-send-email-jeyu@redhat.com>

On Fri, Feb 05, 2016 at 10:08:15PM -0500, Jessica Yu wrote:
> As explained here [1], livepatch modules are failing to initialize properly
> because the ftrace coming module notifier (which calls
> ftrace_module_enable()) runs *after* the livepatch module notifier (which
> enables the patch(es)). Thus livepatch attempts to apply patches to
> modules before ftrace_module_enable() is even called for the corresponding
> module(s). As a result, patch modules break. Ftrace code must run before
> livepatch on module load, and the reverse is true on module unload.
> 
> For ftrace and livepatch, order of initialization (plus exit/cleanup code) is
> important for loading and unloading modules, and using module notifiers to
> perform this work is not ideal since it is not always clear what gets called
> when. In this patchset, dependence on the module notifier call chain is removed
> in favor of hard coding the corresponding function calls in the module loader.
> This promotes better code visibility and ensures that ftrace and livepatch code
> get called in the correct order on patch module load and unload.
> 
> Tested the changes with a test livepatch module that patches 9p and nilfs2,
> and verified that the issue described in [1] is fixed.

Since Rusty agreed to your suggested changes for splitting up
complete_formation() and for setting mod->state = MODULE_STATE_GOING
before calling the going notifiers in the error path, can you do a v4
with those changes?  They should probably be split up like:

1. split up complete_formation()
2. set MODULE_STATE_GOING before calling going notifiers in error path
3. remove ftrace module notifier
4. remove livepatch module notifier

-- 
Josh

  parent reply	other threads:[~2016-02-08 17:48 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-06  3:08 [PATCH v3 0/2] Fix ordering of ftrace/livepatch calls on module load and unload Jessica Yu
2016-02-06  3:08 ` [PATCH v3 1/2] ftrace/module: remove ftrace module notifier Jessica Yu
2016-02-08 14:18   ` Petr Mladek
2016-02-08 17:31   ` Josh Poimboeuf
2016-02-06  3:08 ` [PATCH v3 2/2] livepatch/module: remove livepatch " Jessica Yu
2016-02-08 14:30   ` Petr Mladek
2016-02-08 17:48 ` Josh Poimboeuf [this message]
2016-02-08 17:58   ` Fix ordering of ftrace/livepatch calls on module load and unload Jessica Yu
2016-02-08 18:00     ` Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160208174846.GB23106@treble.redhat.com \
    --to=jpoimboe@redhat.com \
    --cc=jeyu@redhat.com \
    --cc=jikos@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=rusty@rustcorp.com.au \
    --cc=sjenning@redhat.com \
    --cc=vojtech@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.