From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752582AbcFIOUn (ORCPT ); Thu, 9 Jun 2016 10:20:43 -0400 Received: from mx2.suse.de ([195.135.220.15]:58238 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752520AbcFIOUl (ORCPT ); Thu, 9 Jun 2016 10:20:41 -0400 Date: Thu, 9 Jun 2016 16:20:38 +0200 From: Petr Mladek To: Josh Poimboeuf Cc: Jiri Kosina , Jessica Yu , Miroslav Benes , linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, Vojtech Pavlik Subject: Re: [RFC PATCH v1.9 00/14] livepatch: hybrid consistency model Message-ID: <20160609142038.GA31195@pathway.suse.cz> References: <20160401133944.GQ5522@pathway.suse.cz> <20160405134430.q4pqm2oalivjka4s@treble.redhat.com> <20160428185353.dljx4aykcr4hdznm@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160428185353.dljx4aykcr4hdznm@treble> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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