Re: GCC options for kernel live-patching (Was: Add a new option to control inlining only on static functions)

["Martin Liška" <mliska@suse.cz>]

On 10/2/18 4:46 PM, Qing Zhao wrote:
> 
>> On Oct 2, 2018, at 9:02 AM, Martin Liška <mliska@suse.cz> wrote:
>>
>> On 10/2/18 3:28 PM, Qing Zhao wrote:
>>>
>>>> On Oct 2, 2018, at 3:33 AM, Martin Jambor <mjambor@suse.cz> wrote:
>>>>
>>>> Hi,
>>>>
>>>> my apologies for being terse, I'm in a meeting.
>>>>
>>>> On Mon, Oct 01 2018, Qing Zhao wrote:
>>>>> Hi, Martin,
>>>>>
>>>>> I have studied a little more on
>>>>>
>>>>> https://github.com/marxin/kgraft-analysis-tool/blob/master/README.md <https://github.com/marxin/kgraft-analysis-tool/blob/master/README.md>
>>>>>
>>>>> in the Section “Usages”, from the example, we can see:
>>>>>
>>>>> the tool will report a list of affected functions for a function that will be patched.
>>>>> In this list, it includes all callers of the patched function, and the cloned functions from the patched function due to ipa const-propogation or ipa sra. 
>>>>>
>>>>> My question:
>>>>>
>>>>> what’s the current action to handle the cloned functions from the
>>>>> patched function due to ipa const-proposation or ipa sra, etc?
>>>>
>>>> If we want to patch an inlined, cloned, or IPA-SRAed function, we also
>>>> patch all of its callers.
>>>
>>> take the example from the link:
>>>
>>> $ gcc /home/marxin/Programming/linux/aesni-intel_glue.i -O2 -fdump-ipa-clones -c
>>> $ ./kgraft-ipa-analysis.py aesni-intel_glue.i.000i.ipa-clones
>>>
>>> [..skipped..]
>>> Function: fls64/63 (./arch/x86/include/asm/bitops.h:479:90)
>>>  inlining to: __ilog2_u64/132 (include/linux/log2.h:40:5)
>>>    inlining to: ablkcipher_request_alloc/1639 (include/linux/crypto.h:979:82)
>>>      constprop: ablkcipher_request_alloc.constprop.8/3198 (include/linux/crypto.h:979:82)
>>>    inlining to: helper_rfc4106_decrypt/3007 (arch/x86/crypto/aesni-intel_glue.c:1016:12)
>>>    inlining to: helper_rfc4106_encrypt/3006 (arch/x86/crypto/aesni-intel_glue.c:939:12)
>>>
>>>  Affected functions: 5
>>>    __ilog2_u64/132 (include/linux/log2.h:40:5)
>>>    ablkcipher_request_alloc/1639 (include/linux/crypto.h:979:82)
>>>    ablkcipher_request_alloc.constprop.8/3198 (include/linux/crypto.h:979:82)
>>>    helper_rfc4106_decrypt/3007 (arch/x86/crypto/aesni-intel_glue.c:1016:12)
>>>    helper_rfc4106_encrypt/3006 (arch/x86/crypto/aesni-intel_glue.c:939:12)
>>> [..skipped..]
>>>
>>>
>>> if we want to patch the function “fls64/63”,  what else functions we need to patch, too? my guess is:
>>
>> Hi.
>>
>> Yes, 'Affected functions' is exactly the list of functions you want to patch.
>>
>>>
>>> **all the callers:
>>> __ilog2_u64/132
>>> ablkcipher_request_alloc/1639
>>> helper_rfc4106_decrypt/3007
>>> helper_rfc4106_encrypt/3006 
>>> **and:
>>> ablkcipher_request_alloc.constprop.8/3198
>>> is the above correct?
>>>
>>> how to generate patch for ablkcipher_request_alloc.constprop.8/3198? since it’s not a function in the source code?
>>
>> Well, it's a 'static inline' function in a header file thus the function will be inlined in all usages.
>> In this particular case there's no such caller function, so you're fine.
> 
> So, for cloned functions, you have to analyze them case by case manually to see their callers?

No, the tool should provide complete list of affected functions.

> why not just disable ipa-cp or ipa-sra completely?

Because the optimizations create function clones, which are trackable with my tool
and one knows then all affected functions.

You can disable the optimizations, but you'll miss some performance benefit provide
by compiler.

Note that as Martin Jambor mentioned in point 2) there are also IPA optimizations that
do not create clones. These should be listed and eventually disabled for kernel live
patching.

Martin

> 
> Qing
>>
>