On Mon, May 09, 2016 at 11:41:37AM +0200, Miroslav Benes wrote:
> > +void klp_init_transition(struct klp_patch *patch, int state)
> > +{
> > + struct task_struct *g, *task;
> > + unsigned int cpu;
> > + struct klp_object *obj;
> > + struct klp_func *func;
> > + int initial_state = !state;
> > +
> > + klp_transition_patch = patch;
> > +
> > + /*
> > + * If the patch can be applied or reverted immediately, skip the
> > + * per-task transitions.
> > + */
> > + if (patch->immediate)
> > + return;
> > +
> > + /*
> > + * Initialize all tasks to the initial patch state to prepare them for
> > + * switching to the target state.
> > + */
> > + read_lock(&tasklist_lock);
> > + for_each_process_thread(g, task)
> > + task->patch_state = initial_state;
> > + read_unlock(&tasklist_lock);
> > +
> > + /*
> > + * Ditto for the idle "swapper" tasks.
> > + */
> > + get_online_cpus();
> > + for_each_online_cpu(cpu)
> > + idle_task(cpu)->patch_state = initial_state;
> > + put_online_cpus();
> > +
> > + /*
> > + * Ensure klp_ftrace_handler() sees the task->patch_state updates
> > + * before the func->transition updates. Otherwise it could read an
> > + * out-of-date task state and pick the wrong function.
> > + */
> > + smp_wmb();
> > +
> > + /*
> > + * Set the func transition states so klp_ftrace_handler() will know to
> > + * switch to the transition logic.
> > + *
> > + * When patching, the funcs aren't yet in the func_stack and will be
> > + * made visible to the ftrace handler shortly by the calls to
> > + * klp_patch_object().
> > + *
> > + * When unpatching, the funcs are already in the func_stack and so are
> > + * already visible to the ftrace handler.
> > + */
> > + klp_for_each_object(patch, obj)
> > + klp_for_each_func(obj, func)
> > + func->transition = true;
> > +
> > + /*
> > + * Set the global target patch state which tasks will switch to. This
> > + * has no effect until the TIF_PATCH_PENDING flags get set later.
> > + */
> > + klp_target_state = state;
>
> I am afraid there is a problem for (patch->immediate == true) patches.
> klp_target_state is not set for those and the comment is not entirely
> true, because klp_target_state has an effect in several places.
Ah, you're right. I moved this assignment here for v2. It was
originally done before the patch->immediate check. If I remember
correctly, I moved it closer to the barrier for better readability (but
I created a bug in the process).
> I guess we need to set klp_target_state even for immediate patches. Should
> we also initialize it with KLP_UNDEFINED and set it to KLP_UNDEFINED in
> klp_complete_transition()?
Yes, to both.
--
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
