Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 11 Feb 2019 08:08:13 -0600
From:   Josh Poimboeuf <jpoimboe@redhat.com>
To:     Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Cc:     live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
        Miroslav Benes <mbenes@suse.cz>,
        Petr Mladek <pmladek@suse.com>, Jiri Kosina <jikos@kernel.org>
Subject: Re: [PATCH] livepatch: Enforce reliable stack trace as config
 dependency
Message-ID: <20190211140813.z7kap634gz3gp6a4@treble>
References: <20190209091728.23046-1-kamalesh@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20190209091728.23046-1-kamalesh@linux.vnet.ibm.com>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Feb 09, 2019 at 02:47:28PM +0530, Kamalesh Babulal wrote:
> While the consistency model was introduced, architectures without the
> reliable stack trace implementation could use the immediate flag for
> livepatching but with its own limitations.
> 
> After removal of the immediate flag by commit d0807da78e11
> ("livepatch: Remove immediate feature"), reliable stack trace became
> enforcing dependency for livepatch support on any architecture. In
> the current code, we ensure that the dependency is met when
> enabling the patch during the module load.
> 
> This dependency check can be improved by moving it to the Kconfig,
> which disables the support for livepatching in the kernel for unmet
> dependencies. This patch moves both HAVE_RELIABLE_STACKTRACE and
> STACKTRACE under config LIVEPATCH, it also removes the
> klp_have_reliable_stack() function.
> 
> Loading a livepatching module on an architecture where reliable
> stack trace is yet to be implemented, the user should see:
> 
> insmod: ERROR: could not insert module ./livepatch-sample.ko: Invalid module format
> 
> ...
> [  286.453463] livepatch_sample: module is marked as livepatch module, but livepatch support is disabled

Wouldn't the module fail to build in the first place, since
CONFIG_LIVEPATCH is disabled?

Anyway, I'm not sure about this approach.  This patch makes the s390
livepatch code no longer compilable, turning it into completely dead
code.  So if something changes in the s390 code which causes it to stop
compiling, nobody will notice.

I think I'd rather go in the opposite direction: allow the patches to be
loaded.  Then they can be forced, if needed.  That enables both compile
and runtime testing.  That way we don't make any backward progress,
until such arches get reliable stacktraces.

-- 
Josh