From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1948478AbdEZRiB (ORCPT ); Fri, 26 May 2017 13:38:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51914 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753169AbdEZRh6 (ORCPT ); Fri, 26 May 2017 13:37:58 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3DBF96549 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jpoimboe@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 3DBF96549 Date: Fri, 26 May 2017 12:37:56 -0500 From: Josh Poimboeuf To: Petr Mladek Cc: Miroslav Benes , jeyu@redhat.com, jikos@kernel.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] livepatch: force transition process to finish Message-ID: <20170526173756.khuimh6eh3xcvvyh@treble> References: <20170518120043.7205-1-mbenes@suse.cz> <20170518120043.7205-4-mbenes@suse.cz> <20170524130610.GK7297@pathway.suse.cz> <20170524150950.GC26699@pathway.suse.cz> <20170525160307.GI26699@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170525160307.GI26699@pathway.suse.cz> 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.39]); Fri, 26 May 2017 17:37:57 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 25, 2017 at 06:03:07PM +0200, Petr Mladek wrote: > On Thu 2017-05-25 14:59:55, Miroslav Benes wrote: > > > > > > > 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. > > Sounds good to me. I'm thinking that WARN_ON_ONCE() seems too severe. If the patch didn't need a consistency model in the first place then it wouldn't be worth warning about. We have to trust that the user knows what they're doing. And that's true for the entire live patching process, including patch analysis and patch creation. And anyway we already have a taint flag for that: TAINT_LIVEPATCH. > > > > 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. > > AFAIK, we did not have to do it, see > https://lkml.kernel.org/r/20161222143452.GK25166@pathway.suse.cz > and the comment starting with "It would still leave a small". > > Just for record, the idea of disabling the TIF flags came from Josh > in another mail. I have just repeated it. > > I think that the problem already is complex enough and the > serialization would reduce the space of potential races. > But it is possible that I see it just too complex here. IMO we can skip the mutex. The consistency model will be broken anyway, so all bets are off. -- Josh