Re: [PATCH v3 5/6] x86/ftrace: Use text_poke()
From: Petr Mladek
Date: Thu Jan 30 2020 - 04:53:52 EST
On Wed 2020-01-29 09:59:51, Josh Poimboeuf wrote:
> In retrospect, the prerequisites for merging it should have been:
OK, let me do one more move in this game.
> 1) Document how source-based patches can be safely generated;
I agree that the information are really scattered over many files
in Documentation/livepatch/. Anyway, there is a lot of useful
hints:
+ structure and behavior of the livepatch module, link
to a sample, limitations, are described in livepatch.rst
+ many other catches are described in the other files:
callbacks, module-elf-fomat, cumulative-patches,
system-state.
Yes, it would be great to have a better structure, more information.
But do not get me wrong. Anyone, Joe definitely, is able to create
livepatch from sources by this information.
Anyone could play with it, ask questions, and improve the
documentation. Better documentation would help but it is
not a blocker, definitely.
> 2) Fix the scheduler performance regression;
The optimizations are disabled only when livepatching is enabled.
I would consider this as a prize for the feature. There are
many things like this.
As it was said. It was 1-3 percent in scheduler microbenchmark.
It would make sense to fix it only when it causes such a regression
in real workloads. Do you have any?
> 3) Figure out if there are any other regressions by detecting which
> function interfaces are affected by the flag and seeing if they're
> hot path;
IMHO, benchmarks are much more effective and we spent non-trivial
resources when running them.
> 4) Provide a way for the N-1 users to opt-out
AFAIK, the only prize is the 1-3 percent scheduler performance degradation.
If you really do not want to pay this prize, let's make it configurable.
But the option is definitely needed when source livepatches are used.
There is no other reasonable way to detect and workaround these
problems. For this, it has to be in upstream kernel. It is in line
with the effort to make livepatching less and less error prone.
And please, let's stop playing this multi-user games. There is at least
one known user of source based livepatches. By coincidence, it is also
a big contributor to this subsystem. Adding an extra option into
CFLAGS is quite error prone. You can imagine how complicated is
a kernel rpm spec file for more kernel flavors. The only safe way
is to have the optimization tight with the CONFIG option in
kernel sources.
> 5) Fix the objtool warnings (or is it a GCC bug)
Nobody was aware of them. I wonder if they even existed at that time.
We have a simple fix now. Let's continue in the thread started by
Jikos if we could get a better solution.
> 6) Make -flive-patching compatible with LTO (or at least acknowledge
> that it should and will be done soon)
Is LTO officially supported upstream?
Are all patches, features tested for LTO compactibility?
Is there any simple way to build and run LTO kernel?
> 7) At least make it build- or runtime-incompatible with Clang-built
> kernels to prevent people from assuming it's safe.
Same questions as for LTO.
> If you don't want to revert the patch, then address my concerns instead
> of minimizing and deflecting at every opportunity.
I would really like to keep focusing on realistic problems and
realistic solutions:
+ make the optimization configurable if you resist on it
+ fix the objtool warnings
Anything else is out of scope of this thread from my POV.
Best Regards,
Petr