This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC] GCC support for live-patching
On Mon, 22 Oct 2018, Qing Zhao wrote:
> Hi,
>
> thanks for the comments.
>
> >
> > thanks for the proposal. The others have already expressed some of my
> > worries and remarks, but I think it would be only right to write them
> > again. Especially since I am part of the team responsible for
> > implementation and maintenance of live patches here at SUSE, we use kGraft
> > and we prepare everything manually (compared to kpatch and ksplice).
>
> One question here, what’s the major benefit to prepare the patches manually?
I could almost quote what you wrote below. It is a C file, easy to review
and maintain. You have everything "under control". It allows to implement
tricky hacks easily by hand if needed.
> >> 1. A study of Kernel live patching schemes.
> >>
> >> Three major kernel live patching tools: https://lwn.net/Articles/734765/
> >>
> >> * ksplice: http://www.ksplice.com/doc/ksplice.pdf
> >> * kpatch: https://lwn.net/Articles/597123/
> >> https://github.com/dynup/kpatch
> >> * kGraft:
> >> https://pdfs.semanticscholar.org/presentation/af4c/895aa3fef0cc2b501317aaec9d91ba2d704c.pdf
> >>
> >> In the above, ksplice and kpatch can automatically generate binary patches
> >> as following:
> >>
> >> * a collection of tools which convert a source diff patch to a patch
> >> module. They work by compiling the kernel both with and without the source
> >> patch, comparing the binaries, and generating a binary patch module which
> >> includes new binary versions of the functions to be replaced.
> >>
> >> on the other hand, kGraft offers a way to create patches entirely by hand.
> >> The source of the patch is a single C file, easy to review, easy to
> >> maintain.
> >>
> >> In addition to kGraft, there are other live patching tools that prefer
> >> creating patches by hand for the similar reason.
> >>
> >> The compiler support is mainly for the above live patching tools that create
> >> patches entirely by hand. the major purpose is:
> >>
> >> * control patch code size and debug complexity;
> >> * keep good run time performance;
> >>
> >> 2. the major problems of compiler in live patching:
> >>
> >> For the live patching schemes that create patches by hand, when patching
> >> one function, there might a list of functions that will be impacted by
> >> this patched function due to compiler optimization/analyses (mainly IPA
> >> optimization/analyses), a complete patch will include the patched function
> >> and all impacted functions. Usually, there are two major factors to be
> >> considered in such live patching schemes:
> >>
> >> * patch code size, one major factor is the length of the list
> >> of impacted functions;
> >> * run time performance.
> >>
> >> If we want to control the patch code size, to make the list of impacted
> >> functions minimum, we have to disable corresponding compiler optimizations
> >> as much as possible.
> >
> > Andi already talked about it and I, too, do not understand your worry
> > about patch code size. First, it has never been so bad here. Yes,
> > sometimes the function closure gets bigger due to optimizations and
> > inlining. I've considered it as nothing else than a lack of better
> > tooling, because it is indeed something which could be improved a lot.
> > Nicolai (CCed) works on a potential solution. It is also one of the topics
> > at LPC miniconf in Vancouver.
> >
> > Second, the idea to disable inlining would not fly at SUSE. I can't
> > imagine to even propose it. The kernel heavily relies on the feature. The
> > optimizations are a different story and some of those certainly could be
> > disabled with no harm caused.
> >
> > So let me ask, what is your motivation behind this? Is there a real
> > problem you're trying to solve? It may have been mentioned somewhere and I
> > missed it.
>
> the major functionality we want is: to Only enable static inlining for live patching for one
> of our internal customers. the major purpose is to control the patch code size explosion and
> debugging complexity due to too much inlining of global functions for the specific application.
I hoped for more details, but ok.
> therefore, I proposed the multiple level of control for -flive-patching to satisfy multiple request from
> different users.
>
> So far, from the feedback, I see that among the 4 levels of control, none, only-inline-static, inline,
> and inline-clone, “none” and “inline” are NOT needed at all.
>
> however, -flive-patching = [only-inline-static | inline-clone] are necessary.
>
> >
> >> On the other hand, in order to keep good run time performance, we need to
> >> keep the compiler optimization as much as possible.
> >>
> >> So, there should be some tradeoff between these two factors.
> >>
> >> The following are two major categories of compiler optimizations
> >> we should considered:
> >>
> >> A. compiler optimizations/analyses that extract ipa info from
> >> a routine's body, and use such info to guide other optimization.
> >>
> >> Since the body of the routine might be changed for live patching,
> >> the ipa info extracted from the body of the routine also changes,
> >> as a result, all the routines that directly or indirectly utilize
> >> the ipa info from this routine are in the list of the impacted
> >> routines.
> >>
> >> Most of the IPA analyses and optimization belong to this category.
> >>
> >> Although theoretically the impacted routine list from such ipa
> >> phases could be computed, the list might be huge. Such huge list
> >> of impacted routine might explode the patch code size too much.
> >>
> >> Therefore, it might be more pratical to just completely disable such
> >> ipa optimizations/analyses.
> >>
> >> B. Inlining, and all optimizaitons that internally create clone.
> >> for example, cloning, ipa-sra, partial inlining, etc.
> >> We can track the effect and impacted routine of such optimization
> >> easily.
> >> Such kind of optimization could be kept, but the compiler
> >> should provide the list of impacted functions if a routine need to
> >> be patched.
> >>
> >> There is patch code size explosion potential even with only enabling
> >> inlining and cloning for live patching. Users need a way to control
> >> the inlining and cloning in order to control the code size explosion
> >> and complexity of debugging.
> >>
> >> 3. Details of the proposal:
> >
> > This sounds awfully complicated. Especially when there is a dumping option
> > in GCC thanks to Martin. What information do you miss there? We could
> > improve the analysis tool. So far, it has given us all the info we need.
>
> Yes, it’s TRUE that the tool Martin wrote should serve the same purpose. nothing new from this
> new GCC option -flive-patch-list compared to Martin’s tool.
>
> However, by simply adding this new GCC’s option, we can simplify the whole procedure for helping
> live-patching. by only running GCC with the new added options once, we can get the impacted function list
> at the same time. No need to run another tool anymore.
I probably do not understand completely. I thought that using the
option you would "preprocess" everything during the kernel build and then
you'd need a tool to get the impacted function list for a given function.
In that case, Martin's work is more than sufficient.
Now I think you meant to run GCC with a given function, build everything
and the list. Iteratively for every to-be-patched function. It does not
sound better to me.
> this is the major benefit from this new option.
>
> anyway, if most of the people think that this new option is not necessary, I am fine to delete it.
> >
> > In the end, I'd be more than happy with what has been proposed in this
> > thread by the others. To have a way to guarantee that GCC would not apply
> > an optimization that could potentially destroy our effort to livepatch a
> > running system.
>
> So, the major functionality you want from GCC is:
>
> -flive-patching=inline-clone
>
> Only enable inlining and all optimizations that internally create clone,
> for example, cloning, ipa-sra, partial inlining, etc; disable all
> other IPA optimizations/analyses.
>
> As a result, when patching a routine, all its callers and its clones’
> callers need to be patched as well.
>
> ?
I cannot decide that. Perhaps. Josh called this hypothetical option
-fpreserve-function-abi, which seems self-explaining to me.
Regards,
Miroslav