Received-SPF: pass (google.com: domain of linux-kernel+bounces-200851-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 4 Jun 2024 10:37:30 -0400
From: Joe Lawrence <joe.lawrence@redhat.com>
To: zhang warden <zhangwarden@gmail.com>
Cc: Miroslav Benes <mbenes@suse.cz>, Josh Poimboeuf <jpoimboe@kernel.org>,
	Jiri Kosina <jikos@kernel.org>, Petr Mladek <pmladek@suse.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] livepatch: introduce klp_func called interface
Message-ID: <Zl8mqq6nFlZL+6sb@redhat.com>
References: <20240520005826.17281-1-zhangwarden@gmail.com>
 <alpine.LSU.2.21.2405200845130.11413@pobox.suse.cz>
 <BBD2A553-6D44-4CD5-94DD-D8B2D5536F94@gmail.com>
 <alpine.LSU.2.21.2405210823590.4805@pobox.suse.cz>
 <Zloh/TbRFIX6UtA+@redhat.com>
 <4DE98E35-2D1F-4A4E-8689-35FD246606EF@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4DE98E35-2D1F-4A4E-8689-35FD246606EF@gmail.com>

On Tue, Jun 04, 2024 at 04:14:51PM +0800, zhang warden wrote:
> 
> 
> > On Jun 1, 2024, at 03:16, Joe Lawrence <joe.lawrence@redhat.com> wrote:
> > 
> > Adding these attributes to livepatch sysfs would be expedient and
> > probably easier for us to use, but imposes a recurring burden on us to
> > maintain and test (where is the documentation and kselftest for this new
> > interface?).  Or, we could let the other tools handle all of that for
> > us.
> How this attribute imposes a recurring burden to maintain and test?
> 

Perhaps "responsibility" is a better description.  This would introduce
an attribute that someone's userspace utility is relying on.  It should
at least have a kselftest to ensure a random patch in 2027 doesn't break
it.

> > Perhaps if someone already has an off-the-shelf script that is using
> > ftrace to monitor livepatched code, it could be donated to
> > Documentation/livepatch/?  I can ask our QE folks if they have something
> > like this.
> 
> My intention to introduce this attitude to sysfs is that user who what to see if this function is called can just need to show this function attribute in the livepatch sysfs interface.
> 
> User who have no experience of using ftrace will have problems to get the calling state of the patched function. After all, ftrace is a professional kernel tracing tools.
> 
> Adding this attribute will be more easier for us to show if this patched function is called. Actually, I have never try to use ftrace to trace a patched function. Is it OK of using ftrace for a livepatched function?
> 

If you build with CONFIG_SAMPLE_LIVEPATCH=m, you can try it out (or with
one of your own livepatches):

# Convenience variable
  $ SYSFS=/sys/kernel/debug/tracing

# Install the livepatch sample demo module
  $ insmod samples/livepatch/livepatch-sample.ko

# Verify that ftrace can filter on our functions
  $ grep cmdline_proc_show $SYSFS/available_filter_functions
  cmdline_proc_show
  livepatch_cmdline_proc_show [livepatch_sample]

# Turn off any existing tracing and filter functions
  $ echo 0 > $SYSFS/tracing_on
  $ echo > $SYSFS/set_ftrace_filter

# Set up the function tracer and add the kernel's cmdline_proc_show()
# and livepatch-sample's livepatch_cmdline_proc_show()
  $ echo function > $SYSFS/current_tracer
  $ echo cmdline_proc_show >> $SYSFS/set_ftrace_filter
  $ echo livepatch_cmdline_proc_show >> $SYSFS/set_ftrace_filter
  $ cat $SYSFS/set_ftrace_filter
  cmdline_proc_show
  livepatch_cmdline_proc_show [livepatch_sample]

# Turn on the ftracing and force execution of the original and
# livepatched functions
  $ echo 1 > $SYSFS/tracing_on
  $ cat /proc/cmdline 
  this has been live patched

# Checkout out the trace file results
  $ cat $SYSFS/trace
  # tracer: function
  #
  # entries-in-buffer/entries-written: 2/2   #P:8
  #
  #                                _-----=> irqs-off/BH-disabled
  #                               / _----=> need-resched
  #                              | / _---=> hardirq/softirq
  #                              || / _--=> preempt-depth
  #                              ||| / _-=> migrate-disable
  #                              |||| /     delay
  #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
  #              | |         |   |||||     |         |
               cat-254     [002] ...2.   363.043498: cmdline_proc_show <-seq_read_iter
               cat-254     [002] ...1.   363.043501: livepatch_cmdline_proc_show <-seq_read_iter


The kernel docs provide a lot of explanation of the complete ftracing
interface.  It's pretty power stuff, though you may also go the other
direction and look into using the trace-cmd front end to simplify all of
the sysfs manipulation.  Julia Evans wrote a blog [1] a while back that
provides a some more examples.

[1] https://jvns.ca/blog/2017/03/19/getting-started-with-ftrace/

--
Joe