From: Petr Mladek <pmladek@suse.com>
To: Miroslav Benes <mbenes@suse.cz>
Cc: Jessica Yu <jeyu@redhat.com>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Seth Jennings <sjenning@redhat.com>,
Jiri Kosina <jikos@kernel.org>, Vojtech Pavlik <vojtech@suse.com>,
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: livepatch/module: remove livepatch module notifier
Date: Fri, 5 Feb 2016 11:06:56 +0100 [thread overview]
Message-ID: <20160205100656.GC3305@pathway.suse.cz> (raw)
In-Reply-To: <alpine.LNX.2.00.1602051004000.25516@pobox.suse.cz>
On Fri 2016-02-05 10:15:56, Miroslav Benes wrote:
> On Thu, 4 Feb 2016, Jessica Yu wrote:
> > Argh, thank you for catching that. I think we could split up
> > complete_formation()
> > into two functions in order to make the error handling work.
> >
> > Does all this look ok?
>
> Hm, there is an another option. We can cover the needed error handling in
> complete_formation(). So for the first error there
> (verify_export_symbols() fails) we need to only release module_mutex. For
> our second error we need more. We would call module_bug_cleanup() under
> module_mutex, module_disable_{ro,nx} and going notifiers. Is this correct?
> It would be hidden in complete_formation() this way and cleaner in my
> opinion. There is some code duplication though.
This sounds better to me.
> > Also, one last thing, I noticed that module->state
> > isn't
> > set to MODULE_STATE_GOING anywhere before the going notifier chain is called
> > in
> > the bug_cleanup label (I think it is still COMING at that point), so the
> > klp_module_disable call right afterwards would have bailed out because of
> > that.
> > To be consistent, shouldn't it be set before the going notifiers are called?
>
> This could break something or introduce a race somewhere. Git grep says
> there are several checks for MODULE_STATE_GOING through out the kernel
> which need to be checked.
I think that we could relax the condition in the
klp_module_going/disable() function and allow to call it
also in MODULE_STATE_COMMING_STATE. It would deserve
a comment.
Best Regards,
Petr
next prev parent reply other threads:[~2016-02-05 10:07 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-02 1:17 [PATCH v2 0/2] Fix ordering of ftrace/livepatch calls on module load and unload Jessica Yu
2016-02-02 1:17 ` [PATCH v2 1/2] ftrace/module: remove ftrace module notifier Jessica Yu
2016-02-04 13:27 ` Petr Mladek
2016-02-04 14:18 ` Steven Rostedt
2016-02-04 15:21 ` Petr Mladek
2016-02-04 15:33 ` Steven Rostedt
2016-02-02 1:17 ` [PATCH v2 2/2] livepatch/module: remove livepatch " Jessica Yu
2016-02-04 14:39 ` Petr Mladek
2016-02-04 14:56 ` Steven Rostedt
2016-02-04 16:47 ` Miroslav Benes
2016-02-05 4:11 ` Jessica Yu
2016-02-05 9:15 ` Miroslav Benes
2016-02-05 10:06 ` Petr Mladek [this message]
2016-02-08 0:34 ` Rusty Russell
2016-02-04 17:29 ` [PATCH v2 2/2] " Miroslav Benes
2016-02-04 20:55 ` Josh Poimboeuf
2016-02-05 8:59 ` Miroslav Benes
2016-02-04 10:43 ` [PATCH v2 0/2] Fix ordering of ftrace/livepatch calls on module load and unload Jiri Kosina
2016-02-04 13:29 ` Steven Rostedt
2016-02-05 1:17 ` Rusty Russell
-- strict thread matches above, loose matches on Subject: below --
2016-02-09 4:50 [PATCH v4 0/4] " Jessica Yu
2016-02-09 4:50 ` [PATCH v4 4/4] livepatch/module: remove livepatch module notifier Jessica Yu
2016-02-09 23:43 ` Rusty Russell
2016-02-10 10:18 ` Jiri Kosina
2016-02-14 22:59 ` Jiri Kosina
2016-02-15 23:27 ` Josh Poimboeuf
2016-02-15 23:42 ` Jiri Kosina
2016-02-16 0:48 ` Jessica Yu
2016-02-16 8:41 ` Miroslav Benes
2016-02-16 19:51 ` Jessica Yu
2016-02-29 0:30 ` [PATCH v4 4/4] " Rusty Russell
2016-03-01 3:00 ` Jessica Yu
2016-03-11 20:03 [PATCH v2 0/2] Livepatch module notifier cleanup Jessica Yu
2016-03-11 20:03 ` [PATCH v2 2/2] livepatch/module: remove livepatch module notifier Jessica Yu
2016-03-14 15:06 ` Petr Mladek
2016-03-14 17:50 ` Jessica Yu
2016-03-15 9:10 ` Petr Mladek
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=20160205100656.GC3305@pathway.suse.cz \
--to=pmladek@suse.com \
--cc=jeyu@redhat.com \
--cc=jikos@kernel.org \
--cc=jpoimboe@redhat.com \
--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.