From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752682AbdJTI7U (ORCPT ); Fri, 20 Oct 2017 04:59:20 -0400 Received: from mx2.suse.de ([195.135.220.15]:50070 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752302AbdJTI7S (ORCPT ); Fri, 20 Oct 2017 04:59:18 -0400 Date: Fri, 20 Oct 2017 10:59:16 +0200 (CEST) From: Miroslav Benes To: Petr Mladek cc: Jason Baron , Josh Poimboeuf , linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, jeyu@kernel.org, jikos@kernel.org Subject: Re: [PATCH v3 2/2] livepatch: add atomic replace In-Reply-To: <20171018112510.GU2795@pathway.suse.cz> Message-ID: References: <20171006223247.fkkrrtxydq33c7dw@treble> <3b0c1c00-e4ae-13ab-434d-73d8a5e5a60d@akamai.com> <20171018112510.GU2795@pathway.suse.cz> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 18 Oct 2017, Petr Mladek wrote: > On Wed 2017-10-18 11:10:09, Miroslav Benes wrote: > > On Tue, 17 Oct 2017, Jason Baron wrote: > > > If the atomic replace patch does > > > not contain any immediates, then we can drop the reference on the > > > immediately preceding patch only. That is because there may have been > > > previous transitions to immediate functions in the func stack, and the > > > transition to the atomic replace patch only checks immediately preceding > > > transition. It would be possible to check all of the previous immediate > > > function transitions, but this adds complexity and seems like not a > > > common pattern. So I would suggest that we just drop the reference on > > > the previous patch if the atomic replace patch does not contain any > > > immediate functions. > > > > It is even more complicated and it is not connected only to atomic replace > > patch (I realized this while reading the first part of your email and > > then you confirmed it with this paragraph). The consistency model is > > broken with respect to immediate patches. > > > > func a > > patches 1i > > 2i > > 3 > > > > Now, when you're applying 3, only 2i function is checked. But there might > > be a task sleeping in 1i. Such task would be migrated to 3, because we do > > not check 1 in klp_check_stack_func() at all. > > > > I see three solutions. > > > > 1. Say it is an user's fault. Since it is not obvious and it is > > easy-to-make mistake, I would not go this way. > > > > 2. We can fix klp_check_stack_func() in an exact way you're proposing. > > We'd go back in func stack as long as there are immediate patches there. > > This adds complexity and I'm not sure if all the problems would be solved > > because scenarios how patches are stacked and applied to different > > functions may be quite complex. > > > > 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. > > To make it clear. We currently rely on the immediate handling on > architectures without a reliable stack checking. The question > is if anyone uses it for another purpose in practice. > > A solution would be to remove the per-func immediate flag > and invert the logic of the per-patch one. We could rename > it to something like "consistency_required" or "semantic_changes". > A patch with this flag set then might be refused on systems > without reliable stacks. Otherwise, the consistency model > would be used for all patches. I have a problem with this. I'd like to see the consistency model as a default and not something to ask for. It should be used always unless explicitly forbidden. Just to be sure, we agreed to remove immediate, didn't we? Miroslav