> On Oct 23, 2018, at 4:11 AM, Miroslav Benes <mbe...@suse.cz> 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, 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.
in the option -fpreserve-function-abi, will inlining and cloning are enabled by
default?
thanks.
Qing
>
> Regards,
> Miroslav
