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:   Thu, 10 Oct 2019 10:50:35 +0200
From:   Petr Mladek <pmladek@suse.com>
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     jikos@kernel.org, Joe Lawrence <joe.lawrence@redhat.com>,
        jpoimboe@redhat.com, mingo@redhat.com,
        Miroslav Benes <mbenes@suse.cz>, linux-kernel@vger.kernel.org,
        live-patching@vger.kernel.org
Subject: Re: [PATCH 0/3] ftrace: Introduce PERMANENT ftrace_ops flag
Message-ID: <20191010085035.emsdks6xecazqc6k@pathway.suse.cz>
References: <20191007081714.20259-1-mbenes@suse.cz>
 <20191008193534.GA16675@redhat.com>
 <20191009112234.bi7lvp4pvmna26vz@pathway.suse.cz>
 <20191009102654.501ad7c3@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191009102654.501ad7c3@gandalf.local.home>
User-Agent: NeoMutt/20170912 (1.9.0)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed 2019-10-09 10:26:54, Steven Rostedt wrote:
> Petr Mladek <pmladek@suse.com> wrote:
> I think Joe's approach is much easier to understand and implement. The
> "ftrace_enabled" is a global flag, and affects all things ftrace (the
> function bindings). What this patch was doing, was what you said. Make
> ftrace_enabled only affect the ftrace_ops without the "PERMANENT" flag
> set. But that is complex and requires a bit more accounting in the
> ftrace system. Something I think we should try to avoid.

From my POV, the fact that livepatches make use of ftrace handlers
is just an implementation detail. Ideally, users should not know
about it. The livepatch handlers should be handled special way
and should not be affected by the ftrace sysfs interface.
And ideally, the ftrace sysfs interface should still be available
for other ftrace users.

I would understand if this would be too complicated and we would
need to find a compromise.

> What we are now proposing, is that if "ftrace_enabled" is not set, the
> register_ftrace_function() will fail if the ftrace_ops passed to it has
> the PERMANENT flag set (which would cause live patching to fail to
> load).

From the livepatch point of view it would be more reliable if
ftrace_ops with PERMANENT flag would force "ftrace_enabled"
to be always true.

It will make the flag unusable for other ftrace users. But it
will be already be the case when it can't be disabled.

Best Regards,
Petr