On Thu 2016-04-28 13:53:53, Josh Poimboeuf wrote: > On Tue, Apr 05, 2016 at 08:44:30AM -0500, Josh Poimboeuf wrote: > > On Fri, Apr 01, 2016 at 03:39:44PM +0200, Petr Mladek wrote: > > > > There's also a func->immediate flag which allows users to specify that > > > > certain functions in the patch can be applied without per-task > > > > consistency. This might be useful if you want to patch a common > > > > function like schedule(), and the function change doesn't need > > > > consistency but the rest of the patch does. > > > > > > We probably should not set func->transition flag when func->immediate > > > is set or when the related func->object is set. It currently happens > > > only when patch->immediate is set. > > > > Agreed, I'll skip setting func->transition if func->immediate is set. > > So I'm getting ready to post v2, and I think I changed my mind on this > one, for a couple of reasons: > > 1) It's conceptually simpler if func->transition gets set for all > functions, so there are less edge cases to consider. > > 2) For unpatching, if func->transition is set, func->immediate results > in the ftrace handler picking the old function immediately, which is > more expected and in line with the name 'immediate'. If 'transition' > is not set then it doesn't switch to the old function until the > klp_func gets removed from the func stack. Fair enough. > > > If we support only one transition at a time, a simple boolean > > > or even bit should be enough. The most descriptive name would > > > be klp_transition_patch_applied but it is quite long. > > > > Yeah, I'll change it to a bool. > > I could probably go either way on this one, but I'm leaving it as a > non-bool for now, because I think: > > klp_patch_target == KLP_PATCHED > klp_patch_target == KLP_UNPATCHED > > reads better and does a better job describing its purpose than: > > klp_target_patched > !klp_target_patched > > And also, the corresponding task_struct.patch_state variable is now a > tri-state variable, with KLP_UNDEFINED (-1) being the third option to > indicate there's currently no patch operation in progress. And I think > it's easier to understand and implement if we use the same > KLP_PATCHED/UNPATCHED defines for both variables. I am fine with the tristate. The boolean was mentioned as a minimalistic solution in the generic discussion about the name "universe". Just an idea. What about using klp_transition_target and task_struct.klp_transtion_state? It will be more clear that these two variables are used only during the patch transition. 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
