On Wed 2017-08-16 15:20:32, Josh Poimboeuf wrote: > On Wed, Aug 16, 2017 at 03:17:03PM -0400, Joe Lawrence wrote: > > I also wrote a quick test script (see below) to exercise some of the > > load/unload/enable/disable/error status combinations. I'm not sure > > about some of the behaviors, most notably test6 with regard to > > post-unpatch-callbacks as executed on a cancelled transition. (See > > results and comments further below.) > > Yeah, that doesn't seem right. Maybe in case of a pre-patch callback > error, we should only call post-unpatch callbacks for those objects > whose pre-patch callbacks were successfully called (and returned zero). > That would mean tracking on a per-object basis which objects had their > pre-patch callbacks called (successfully). > > That would give the patch module a post-unpatch chance to tear down > anything it had set up in the pre-patch callback. > > And the behavior should be documented. All this makes sense. > > Also, maybe it's just my reading of the log, but would it be clearer if > > the "(un)patching ... complete" messages indicated that they are > > referring to a transaction? It's a bit confusing to see "unpatching ... > > complete" before the pre-unpatch-callbacks ever execute. Not a big > > deal, but I can send a follow up patch if others agree. > > Hm. I'm thinking this highlights the fact that the pre-unpatch callback > is being called in the wrong place. It should actually be called before > the unpatching transition starts. When called from > klp_unpatch_objects(), the new code is no longer running, so it's > effectively post-patch instead of pre-patch. > > Another random thought: maybe we should show the "patching complete" > message *after* the post-patch callback is run. That would be more > honest with the user, as technically, the post-patch callback is part of > the patching process. > > And a similar comment for the "unpatching complete" message. Makes sense as well. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
