Re: [PATCH 3/3] livepatch: force transition process to finish

[Miroslav Benes]

> > > In fact, I would suggest to take klp_mutex in force_store()
> > > and do all actions synchronously, including the check
> > > of klp_transition_patch.
> > 
> > I still think it is better not do it. klp_unmark_tasks() does nothing else 
> > than tasks already do. They call klp_update_patch_state() by themselves 
> > and they do not grab klp_mutex lock for doing that. klp_unmark_tasks() 
> > only forces this action.
> 
> You have a point. But I am not convinced ;-) klp_update_patch_state()
> was called very carefully only when it was safe. The forcing
> intentionally breaks the consistency model. User should really know
> what they are doing when they use this feature.
> 
> I think that we should actually taint the kernel. Developers should
> know when users were pulling their legs.

We could do that. I can change pr_warn() to WARN_ON_ONCE(), which would of 
course taint the kernel.
 
> > On the other hand, I do not see a problem in doing that. We already have a 
> > relationship between klp_mutex and tasklist_lock defined elsewhere, so it 
> > is safe.
> 
> Yup.
> 
> > It would only serialize things needlessly.
> 
> I do not agree. The speed is not important here. Also look
> into klp_reverse_transition(). We explicitly clear all
> TIF_PATCH_PENDING flags and call synchronize_rcu() just
> to make the situation easier and reduce space for potential
> mistakes.

Yes, because we had to do that. We ran into problems otherwise. We do not 
have to do it here. It does not help anything in my opinion.

Miroslav