Message-ID: <ZuGav4txYowDpxqj@pathway.suse.cz>
Date: Wed, 11 Sep 2024 15:27:27 +0200
From: Petr Mladek <pmladek@...e.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
	x86@...nel.org, Miroslav Benes <mbenes@...e.cz>,
	Joe Lawrence <joe.lawrence@...hat.com>,
	Jiri Kosina <jikos@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Marcos Paulo de Souza <mpdesouza@...e.com>,
	Song Liu <song@...nel.org>
Subject: Re: [RFC 00/31] objtool, livepatch: Livepatch module generation

On Mon 2024-09-02 20:59:43, Josh Poimboeuf wrote:
> Hi,
> 
> Here's a new way to build livepatch modules called klp-build.
> 
> I started working on it when I realized that objtool already does 99% of
> the work needed for detecting function changes.
> 
> This is similar in concept to kpatch-build, but the implementation is
> much cleaner.
> 
> Personally I still have reservations about the "source-based" approach
> (klp-convert and friends), including the fragility and performance
> concerns of -flive-patching.  I would submit that klp-build might be
> considered the "official" way to make livepatch modules.

For me, it is not easy to compare the two approaches because I do not
have any practical experience with binary-diff bases approach.
I believe that it has its catches as well.

Anyway, I do not want to open a fight about the two approaches.
I would like to better understand the approach implemented by this
patchset.

First, I have been able to go it working by the extra fixes suggested
in the replies to Song Liu. And it worked well.

Second, I was surprised that the linking of vmlinux.o took much longer
than usual. It seems to be caused by -ffunction-sections
-fdata-sections compiler flags.

The difference is really huge. It might complicate kernel development.
I have done the following test:

	# finish build
	$> make -j8
	# touch a source file to simulate a change
	$> touch include/linux/livepatch.h
	# measure time need for the incremental build
	$> time make -j8

Without -ffunction-sections -fdata-sections:

	$> time make -j8
	real    0m58.719s
	user    3m25.925s
	sys     0m21.895s

With -ffunction-sections -fdata-sections:

	$> time make -j8
	real    13m28.126s
	user    15m43.944s
	sys     0m29.142s

=> Incremental build slowed down from 1min to 13.5min.

Is this expected?

Are you used to this when developing the kernel or do you use
a workaround?


> The concept is similar to kpatch-build which has been a successful
> out-of-tree project for over a decade.  It takes a source .patch as an
> input, builds kernels before and after, does a binary diff, and copies
> any changed functions into a new object file which is then linked into a
> livepatch module.
> 
> By making use of existing objtool functionality, and taking from lessons
> learned over the last decade of maintaining kpatch-build, the overall
> design is much simpler.  In fact, it's a complete redesign and has been
> written from scratch (no copied code).
> 
> Advantages over kpatch-build:
> 
>   - Much simpler design: ~3k fewer LOC

This brings the question how reliable and feature complete this code
is in compare with kPatch which is used in production.

One obvious area is the support of more architectures. I guess that
this  code supports only x86_64 at the moment. While kPatch supports
x86_64, ppc64, and s390. I wonder how complicated it would be to
support more architectures.

Also I tried to compare how kPatch and this code do the binary diff
and found the following:

  a) It seems that kPatch compares the assembly by "memcmp" while
     klp-build uses checksum. This looks good.


  b) Both tools have hacks for many special sections and details.
     I am not sure objtool handles all cases which are handled
     by kPatch.

     For example, it seems that kPatch ignores changes in line numbers
     generated by some macros, see kpatch_line_macro_change_only().
     I can't find a counter part in objtool.


  c) It seems that kPatch contains quite complicated code to correlate
     symbols.

     For example, it correlates local variables by comparing
     functions which reference them, see
     kpatch_correlate_static_local_variables().

     Or kPatch tries to correlate optimized .cold, and .part
     variants of the code via the parent code, see
     kpatch_detect_child_functions()

     While klp-build seems to correlate symbols just be comparing
     the demangled/stripped names, see correlate_symbols().
     This seems to be quite error prone.

     I actually do not understand how klp-build compares symbols
     with the same demangled/stripped names. I probably missed
     a trick somewhere.


Do not get me wrong. I do not expect that the upstream variant would
be feature complete from the beginning. I just want to get a picture
how far it is. The code will be maintained only when it would have
users. And it would have users only when it would be comparable or
better then kPatch.

Best Regards,
Petr

Powered by blists - more mailing lists