From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752629AbdJSK5N (ORCPT ); Thu, 19 Oct 2017 06:57:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46266 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752188AbdJSK5L (ORCPT ); Thu, 19 Oct 2017 06:57:11 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BFC26C04AC40 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jpoimboe@redhat.com Date: Thu, 19 Oct 2017 05:57:07 -0500 From: Josh Poimboeuf To: Miroslav Benes Cc: Jiri Kosina , Jason Baron , linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, jeyu@kernel.org, pmladek@suse.com Subject: Re: [PATCH v3 2/2] livepatch: add atomic replace Message-ID: <20171019105707.dlwxsq6ckhxqc3jf@treble> References: <20171006223247.fkkrrtxydq33c7dw@treble> <3b0c1c00-e4ae-13ab-434d-73d8a5e5a60d@akamai.com> <20171018161409.b46tuwtk4kvd6thm@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 19 Oct 2017 10:57:10 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 19, 2017 at 10:30:24AM +0200, Miroslav Benes wrote: > On Wed, 18 Oct 2017, Josh Poimboeuf wrote: > > > On Wed, Oct 18, 2017 at 03:36:42PM +0200, Jiri Kosina wrote: > > > On Wed, 18 Oct 2017, Miroslav Benes wrote: > > > > > > > 3. Drop immediate. It causes problems only and its advantages on x86_64 > > > > are theoretical. You would still need to solve the interaction with atomic > > > > replace on other architecture with immediate preserved, but that may be > > > > easier. Or we can be aggressive and drop immediate completely. The force > > > > transition I proposed earlier could achieve the same. > > > > > > After brief off-thread discussion, I've been thinking about this a bit > > > more and I also think that we should claim immediate "an experiment that > > > failed", especially as the force functionality (which provides equal > > > functionality from the userspace POV) will likely be there sonnish. > > > > Agreed. > > > > To clarify, we'll need the force patch before removing > > klp_patch.immediate, so we don't break non-x86 arches in the meantime. > > Yes, that is correct. I've just started to work on the force patch. Good! > > On the other hand I think we could remove klp_func.immediate > > immediately. > > > > > > While we're on the subject of removing code... :-) > > > > > > I've mentioned this several times before, but the more features we add, > > the more obvious this point becomes: if we could figure out how to get > > rid of the "patching unloaded modules" feature, the code would be so > > much better, and I'd actually be able to fit the code in my brain. Then > > we could get rid of all these sneaky bugs that Miroslav and Petr keep > > finding, and I wouldn't get an uneasy feeling everytime somebody posts a > > new feature. > > > > Here's one vague idea for how to achieve that. More ideas welcome. > > > > 1) Make the consistency model synchronous with respect to modules: don't > > allow any modules to load or unload until the patch transition is > > complete. > > > > 2) Instead of one big uber patch module which patches vmlinux and > > modules at the same time, make each patch module specific to a single > > object. Then bundle the patch modules together somehow into a "patch > > bundle" so they're treated as a single atomic unit. > > > > 3) The patch bundle, when loaded, would load some of its patch modules > > immediately (for those objects which are already loaded). For > > unloaded objects, the corresponding patch modules will be copied into > > memory but not loaded. > > > > 4) Then, when a to-be-patched module is loaded, the module loader loads > > it into memory and relocates it, but doesn't make it live. Then it > > loads the patch module from the memory blob, makes the patch module > > live, and then makes the to-be-patched module live. > > > > (A variation would be to create a way for user space to load a module in > > the paused state. Then user space can handle the dependencies and do > > the patch juggling. I think that would mean depmod/modprobe would need > > to be involved.) > > It might be easier to do at least part of it in userspace. > > > It could be a terrible idea, though it might be worth looking into. > > I'm worried that we would only move the complexity elsewhere, but maybe > not. We'd remove the code from kernel/livepatch/, but some non-trivial > changes would go to the module loader. Yes, that's a very real possibility. In order for it to be an improvement, it would have to be self-contained. We don't want to make the module code worse either. > Jessica, would that be possible? > > We need to discuss it with packaging if that'd be possible to deliver it. > > Anyway, interesting idea. Yeah, just an idea to consider... I'd like to play around with it someday. -- Josh