Re: [RFC] GCC support for live-patching
On Tue, 23 Oct 2018, Qing Zhao wrote: > > > On Oct 23, 2018, at 4:11 AM, Miroslav Benes wrote: > >> > >> 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. > > Okay, I see. > > another question here: > > From my understanding of the live patching creation from Nicolai’s email, the > patch includes: > > 1. initial patched functions; > 2. all the callers of any patched function if it’s been > inlined/cloned/optimized; > 3. recursively copy any needed cpp macro, type, or > functions definition and add references to data objects with static > storage duration. > > during review and maintain procedure, are all the above 3 need to be reviewed > and maintained? Either I do not understand, or this is mainly a process question. It is really up to you if you want to review it or not. Possibility is the imporant word here. Source based approach (which I originally replied to and confused it with manual preparation) allows you to do it. > >>> > >>> 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. > at this time, this is the details I have. I can ask more if more details are > needed. > > > > >> 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. > >> > >>> > > 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. > > there might be misunderstanding among us for this part. Let me explain my > understanding first: > > 1. with martin’s tool, there are two steps to get the impacted function list > for patched functions: > > Step1, build kernel with GCC with -fdump-ipa-clones + a bunch of options > to disable bunch of ipa optimizations. ; > Step2, using the tool kgraft-ipa-analysis.py to analyze the dumped file > from step1 to report the impacted function list. > > 2. with the new functionality of the GCC proposed in this proposal, > -flive-patching -flive-patch-list > > Step1, build kernel with GCC with -flive-patching -flive-patch-list > > then gcc will automatically disable the unsafe ipa optimizations and > report the impacted function list with the safe ipa optimizations. > > compare 1 and 2, I think that 2 is better and much more convenient than 1. > another benefit from 2 is: > > if later we want more ipa optimization to be On for live-patching for the > runtime performance purpose, we can expand it easily to include those > ipa optimization and at the same time report the additional impacted function > list with the new ipa optimizations. > > however, for 1, this will be not easy to be extended. > > do I miss anything here? No, thanks for clearing it up. > > > >> this is the major benefit from this new option. > >> > >> anyway, if most of the people think that this new option is not
Re: [RFC] GCC support for live-patching
Hi Qing, Qing Zhao writes: > thanks a lot for your detailed explanation of the source based live patch > creation procedure. > really interesting and helpful information. > > more questions and comments below: > >>> >>> One question here, what’s the major benefit to prepare the patches >>> manually? >> >> There is none. We here at SUSE prefer the source based approach (as >> opposed to binary diff) for a number of reasons and the manual live >> patch creation is simply a consequence of not having any tooling for >> this yet. >> >> >> For reference, source based live patch creation involves the following >> steps: >> >> 1. Determine the initial set of to be patched functions: >> a.) Inspect the upstream diff for the fix in question, add >> any touched functions to the initial set. >> b.) For each function in the initial set, check whether it has been >> inlined/cloned/optimized and if so, add all its callers to the >> initial set. Repeat until the initial set has stabilized. >> >> 2. Copy & paste the initial set over to the new live patch sources. >> >> 3. Make it compile, i.e. recursively copy any needed cpp macro, type, or >> functions definition and add references to data objects with static >> storage duration. >> The rules are: >> a.) For data objects with static storage duration, a reference to the >> original must always be made. (If the symbol is EXPORT()ed, then >> fine. Otherwise, for kGraft, this involves a kallsyms lookup at >> patch module load time, for upstream kernel live patching, this >> has been solved with those '.klp' relocations). >> b.) If a called function is available as a symbol from either vmlinux >> or some (usually the patched) module, do not copy the definition, >> but add a reference to it, just as in a.). >> c.) If a type, cpp macro or (usually inlined) function is provided by >> some "public" header in /include/, include that >> rather than copying the definition. Counterexample: Non-public >> header outside of include/ like >> e.g. /fs/btrfs/qgroup.h. >> d.) Otherwise copy the definition to the live patch module sources. >> >> Rule 3b is not strictly necessary, but it helps in reducing the live >> patch code size which is a factor with _manual_ live patch creation. >> >> For 1b.), we need help from GCC. Namely, we want to know when some >> functions has been optimized and we want it to disable any of those IPA >> optimization it (currently) isn't capable to report properly. > > Yes, this this is the place that GCC can help. and it’s also the motivation > for this current proposal. > >> >> Step 3.) is a bit tedious sometimes TBH and yes, w/o any tooling in >> place, patch size would be a valid point. However, I'm currently working >> on that and I'm optimistic that I'll have a working prototype soon. > > So, currently, step 3 is done manually? Currently yes. > If the initial set of patched functions is too big, this work is > really tedious and error-prone. For the "tedious" part: yes it sometimes is. But we haven't seen any problems or bugs so far. So it's reliable in practice. > > without a good tool for step3, controlling the initial set size of patched > function is still meaningful. > >> >> That tool would be given the GCC command line from the original or "live >> patch target" kernel compilation for the source file in question, the >> set of functions as determined in 1.) and a number of user provided >> filter scripts to make the decisions in 3.). As a result, it would >> output a self-contained, minimal subset of the original kernel sources. >> >> With that tooling in place, live patch code size would not be a real >> concern for us. > > that’s good to know. > > however, my question here is: > > can this tool be easily adopted by other applications than linux kernel? i.e, > if there is another application that tries to use GCC’s live patching > feature with manually created source patch, will your tool for step 3 be > readily used by this application? Or, this application have to develop > a similar but different tool for itself? This tool's scope is the C99 language, more specifically the GCC dialect and I'll try to keep the CLI agnostic to the application or build environment anyway. That said, my primary interest is the Linux kernel and I'm going to make sure that it works there. It might not work out of the box for random applications, but require some tweaking or even bug fixing. >> >> So in conclusion, what we need from GCC is the information on when we >> have to live patch callers due to optimizations. If that's not possible >> for a particular class of optimization, it needs to be disabled. >> >> OTOH, we definitely want to keep the set of these disabled optimizations >> as small as possible in order to limit the impact of live patching on >> kernel performance. In particular, disabling any of the "cloning" >>
Re: [RFC] GCC support for live-patching
> On Oct 23, 2018, at 4:11 AM, Miroslav Benes wrote: >> >> 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. Okay, I see. another question here: >From my understanding of the live patching creation from Nicolai’s email, the >patch includes: 1. initial patched functions; 2. all the callers of any patched function if it’s been inlined/cloned/optimized; 3. recursively copy any needed cpp macro, type, or functions definition and add references to data objects with static storage duration. during review and maintain procedure, are all the above 3 need to be reviewed and maintained? >>> >>> 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. at this time, this is the details I have. I can ask more if more details are needed. > >> 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. >> >>> 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. there might be misunderstanding among us for this part. Let me explain my understanding first: 1. with martin’s tool, there are two steps to get the impacted function list for patched functions: Step1, build kernel with GCC with -fdump-ipa-clones + a bunch of options to disable bunch of ipa optimizations. ; Step2, using the tool kgraft-ipa-analysis.py to analyze the dumped file from step1 to report the impacted function list. 2. with the new functionality of the GCC proposed in this proposal, -flive-patching -flive-patch-list Step1, build kernel with GCC with -flive-patching -flive-patch-list then gcc will automatically disable the unsafe ipa optimizations and report the impacted function list with the safe ipa optimizations. compare 1 and 2, I think that 2 is better and much more convenient than 1. another benefit from 2 is: if later we want more ipa optimization to be On for live-patching for the runtime performance purpose, we can expand it easily to include those ipa optimization and at the same time report the additional impacted function list with the new ipa optimizations. however, for 1, this will be not easy to be extended. do I miss anything here? > >> 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,
Re: [RFC] GCC support for live-patching
Hi, Qing Zhao writes: >> >> 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? There is none. We here at SUSE prefer the source based approach (as opposed to binary diff) for a number of reasons and the manual live patch creation is simply a consequence of not having any tooling for this yet. For reference, source based live patch creation involves the following steps: 1. Determine the initial set of to be patched functions: a.) Inspect the upstream diff for the fix in question, add any touched functions to the initial set. b.) For each function in the initial set, check whether it has been inlined/cloned/optimized and if so, add all its callers to the initial set. Repeat until the initial set has stabilized. 2. Copy & paste the initial set over to the new live patch sources. 3. Make it compile, i.e. recursively copy any needed cpp macro, type, or functions definition and add references to data objects with static storage duration. The rules are: a.) For data objects with static storage duration, a reference to the original must always be made. (If the symbol is EXPORT()ed, then fine. Otherwise, for kGraft, this involves a kallsyms lookup at patch module load time, for upstream kernel live patching, this has been solved with those '.klp' relocations). b.) If a called function is available as a symbol from either vmlinux or some (usually the patched) module, do not copy the definition, but add a reference to it, just as in a.). c.) If a type, cpp macro or (usually inlined) function is provided by some "public" header in /include/, include that rather than copying the definition. Counterexample: Non-public header outside of include/ like e.g. /fs/btrfs/qgroup.h. d.) Otherwise copy the definition to the live patch module sources. Rule 3b is not strictly necessary, but it helps in reducing the live patch code size which is a factor with _manual_ live patch creation. For 1b.), we need help from GCC. Namely, we want to know when some functions has been optimized and we want it to disable any of those IPA optimization it (currently) isn't capable to report properly. Step 3.) is a bit tedious sometimes TBH and yes, w/o any tooling in place, patch size would be a valid point. However, I'm currently working on that and I'm optimistic that I'll have a working prototype soon. That tool would be given the GCC command line from the original or "live patch target" kernel compilation for the source file in question, the set of functions as determined in 1.) and a number of user provided filter scripts to make the decisions in 3.). As a result, it would output a self-contained, minimal subset of the original kernel sources. With that tooling in place, live patch code size would not be a real concern for us. So in conclusion, what we need from GCC is the information on when we have to live patch callers due to optimizations. If that's not possible for a particular class of optimization, it needs to be disabled. OTOH, we definitely want to keep the set of these disabled optimizations as small as possible in order to limit the impact of live patching on kernel performance. In particular, disabling any of the "cloning" optimizations, which GCC is able to report properly, would be a no-no IMO. IIUC, our preferred selection of allowed IPA optimizations would be provided by what you are referring to as "-flive-patching=inline-clone". >> >>> 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
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
Re: [RFC] GCC support for live-patching
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? > >> 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. 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
Re: [RFC] GCC support for live-patching
On Thu, 18 Oct 2018, Qing Zhao wrote: > Hi, > > After more detailed study of GCC’s IPA optimizations, further study of the > current available kernel live patching schemes and > other live-patching user’s request, I came up with the following initial > proposal in GCC to mainly support live-patching users who > manually create patches. > > Please take a look at the writeup, and let me know your opinions and > suggestions. Hi, 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). [...] > 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. > 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
Re: [RFC] GCC support for live-patching
> > Is it because you generate something manually and want to limit that > > work, > > I think that this is one of the reasons. > and as mentioned in my writeup, the targeted users of this new functionality > is for live-patching users who generate > patches by hand. Ok just means they need better tooling. > in which, it explains that these new options are for helping live-patching > users who create patches entirely by hand, including kernel > live-patching scheme kGraft, and one of our internal customers. It sounds to me the problem is not gcc here, but an inefficient scheme to create patches. > the major reason is to control the code size explosion of manual patches. > It’s the request from our internal customer. So essentially you want to disable inlining. The Linux kernel code heavily relies on inlining to optimize constants and remove unnecessary code paths. For example I cannot even imagine how horrible the code for get/put/copy_*_user would be if you just disabled inlining on it. That's a fairly common coding pattern in core kernel code and it's not going away. I think the time that is spent here pessimizing code would be far better spent creating better tools to create patches. There's a reason why near all people stopped writing things manually in assembler and moved to compilers. Same reasons should apply to patches. > I think that the current option > -fease-live-patching=inline-clone -flive-patching-list > > should automatically ONLY enabling inline+clone optimization (disable all > other ipa optimization/analyses at the same time) and generate the impacted > function > list for each of the function. Dwarf2+ has all the information that is needed to find inlines and clones already. e.g. systemtap and gdb and perf probes and most other debuggers support it fine to find all copies of a given line. I wrote parsing tools for that too and it's not too difficult to use. -Andi
Re: [RFC] GCC support for live-patching
On 18 October 2018 19:34:52 CEST, Qing Zhao wrote:
>A. an option to control GCC's IPA optimizations to provide a safe
>compilation for live-patching purpose. At the same time, provides
>multiple-level control of patch code-size and run time performance
>tradeoff.
>
>-fease-live-patching={none|only-inline-static|inline|inline-clone}
s/-fease-live-patching/-flive-patching/g
please.
TIA
