On Mon, Nov 20, 2017 at 04:57:19PM +0100, Miroslav Benes wrote:
> On Wed, 15 Nov 2017, Miroslav Benes wrote:
>
> > If a task sleeps in a set of patched functions uninterruptedly, it could
> > block the whole transition indefinitely. Thus it may be useful to clear
> > its TIF_PATCH_PENDING to allow the process to finish.
> >
> > Admin can do that now by writing to force sysfs attribute in livepatch
> > sysfs directory. TIF_PATCH_PENDING is then cleared for all tasks and the
> > transition can finish successfully.
> >
> > Important note! Administrator should not use this feature without a
> > clearance from a patch distributor. It must be checked that by doing so
> > the consistency model guarantees are not violated.
> >
> > Signed-off-by: Miroslav Benes <mbenes@xxxxxxx>
>
> While working on "immediate" removal, I realized we had the similar
> problem here with modules removal. There is no way out of the rabbit hole.
>
> If a patch is forced, we obviously cannot say there is no task sleeping in
> the old code. This could be disastrous if such old module is then removed
> (either we disabled it and we want to rmmod it, or there is a new "atomic
> replace" patch and we want to remove the old one).
>
> We need something like the following (at least as a starting point)
>
> diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
> index 566ab210853f..df4f2bbd9731 100644
> --- a/kernel/livepatch/transition.c
> +++ b/kernel/livepatch/transition.c
> @@ -33,6 +33,8 @@ struct klp_patch *klp_transition_patch;
>
> static int klp_target_state = KLP_UNDEFINED;
>
> +static bool klp_forced = false;
> +
> /*
> * This work can be performed periodically to finish patching or unpatching any
> * "straggler" tasks which failed to transition in the first attempt.
> @@ -109,7 +111,7 @@ static void klp_complete_transition(void)
> }
> }
>
> - if (klp_target_state == KLP_UNPATCHED && !immediate_func)
> + if (klp_target_state == KLP_UNPATCHED && !klp_forced && !immediate_func)
> module_put(klp_transition_patch->mod);
>
> /* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
> @@ -642,4 +644,6 @@ void klp_force_transition(void)
>
> for_each_possible_cpu(cpu)
> klp_update_patch_state(idle_task(cpu));
> +
> + klp_forced = true;
> }
>
>
> It is still better than immediate, because it is a "ex post" action.
Looks good to me.
> We can also try to improve later. We could remember all forced tasks
> and reenable rmmod once those tasks are really migrated ("shadow
> migration").
NACK :-) Forcing should hopefully be a rare event, not worth the
trouble to try to keep track of that IMO.
--
Josh
--
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
