> > > 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 -- 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
