Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758289AbcDANjt (ORCPT ); Fri, 1 Apr 2016 09:39:49 -0400 Received: from mx2.suse.de ([195.135.220.15]:60875 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755275AbcDANjq (ORCPT ); Fri, 1 Apr 2016 09:39:46 -0400 Date: Fri, 1 Apr 2016 15:39:44 +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: <20160401133944.GQ5522@pathway.suse.cz> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 Content-Length: 2791 Lines: 65 On Fri 2016-03-25 14:34:47, Josh Poimboeuf wrote: > These patches are still a work in progress, but Jiri asked that I share > them before I go on vacation next week. Based on origin/master because > it has CONFIG_STACK_VALIDATION. I have to follow Mirek and say that it is a great work. > 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. I like the possibility to immediately patch some functions or objects. Just note that this is not yet completely implemented and it is not on the TODO list. 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. Also we should ignore immediate functions and objects when the stack is checked. > Still have a lot of TODOs, some of them are listed here. If you see > something objectionable, it might be a good idea to make sure it's not > already on the TODO list :-) > > TODO: > - come up with a better name than universe? KLP_STATE_PREV/NEXT? > KLP_UNPATCHED/PATCHED? there were some objections to the name in v1. The name "universe" has an advantage if we later allow to enable/disable more patches in parallel. The integer might hold an identifier of the last applied patch. I have been playing with this for kGraft one year ago and it was really challenging. We should avoid it if possible. It is not really needed if we are able to complete any transition in a reasonable time. 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. Note that similar information is provided by TIF_KLP_NEED_UPDATE flag. We use only this bit in kGraft. It saves some space in task_struct but it brings other challenges. We need to prevent migration using a global "kgr_immutable" flag until ftrace handlers for all patched functions are in place. We need to set the flag back when the ftrace handler is called and the global "kgr_immutable" flag is set. > - update documentation for sysfs, proc, livepatch Also we should publish somewhere the information about TIF_KLP_NEED_UPDATE flag, e.g. /proc//klp_need_update. It is handy to see what process blocks the transition. We have something similar in kGraft, see in_progress_show() at https://git.kernel.org/cgit/linux/kernel/git/jirislaby/kgraft.git/commit/?h=kgraft-4.4&id=1c82fbd7b1fe240f4ed178a6506a93033f6a4bed I am still shaking my head around the patches. Best Regards, Petr