dump-ipa-clones reporting enhancement

[joe-lawrence]

I found some time to hack up kpatch-build to see if I could use the new -fdump-ipa-clones flag to generate a before and after report comparison. For those not familiar, this gcc option leaves behind a bunch of .ipa-clones files that describe inlining decisions by the compiler. Comparing these reports around kpatching could be helpful in patch review or kpatch-build bug triaging.

Here's a really simple demo patch:

Force show_val_kb calls to be inlined into its lone caller, meminfo_proc_show

--- src.orig/fs/proc/meminfo.c  2019-10-04 14:23:27.301741298 -0400
+++ src/fs/proc/meminfo.c       2019-10-04 14:25:14.920930984 -0400
@@ -24,7 +24,7 @@ void __attribute__((weak)) arch_report_m
 {
 }

-static void show_val_kb(struct seq_file *m, const char *s, unsigned long num)
+static __always_inline void show_val_kb(struct seq_file *m, const char *s, unsigned long num)
 {
        seq_put_decimal_ull_width(m, s, num << (PAGE_SHIFT - 10), 8);
        seq_write(m, " kB\n", 4);

My hacked-up kpatch-build saves off orig and patched ipa-clones files. Then manually run commands (for now) from ~/.kpatch/tmp through the kgraft-analysis-tool:

ANALYSIS=/root/kgraft-analysis-tool/kgraft-ipa-analysis.py
FILES=$(diff -Nupr orig/ipa-clones/ patched/ipa-clones/ | diffstat -l -p0)

for file in $FILES; do

        orig="orig/${file#*/}"
        patched="patched/${file#*/}"

        diff -U0 \
                --label "$orig"    <($ANALYSIS <(echo $orig)    | sed 's/object file: .*ipa-clones\///') \
                --label "$patched" <($ANALYSIS <(echo $patched) | sed 's/object file: .*ipa-clones\///')
done

Generates this report:

--- orig/ipa-clones/fs/proc/meminfo.c.000i.ipa-clones
+++ patched/ipa-clones/fs/proc/meminfo.c.000i.ipa-clones
@@ -94,0 +95,6 @@
+Function: show_val_kb/3169 (fs/proc/meminfo.c:27:29) [REMOVED] [fs/proc/meminfo.c.000i.ipa-clones]
+  inlining to: meminfo_proc_show/3170 (fs/proc/meminfo.c:33:12) [edges: 36]
+
+  Affected functions: 1
+    meminfo_proc_show/3170 (fs/proc/meminfo.c:33:12)
+

I think there would need to be further sed filtering of the input/output to account for LOC drift, but I just wanted to post this up here to gauge interest. Ideally the inlining report could be saved into a file, or further summarized for build.log inclusion.

[CentUser]

I compiled and installed the kgraft-xxx, found that there could be one error in the script if one following the instruction of the git source:https://github.com/marxin/kgraft-analysis-tool. (Warning: Using this tool need download and compile a new gcc and get it installed. While I was installing it, found that centos-8 with gcc version: 8.2.1 could not compile it successfully. but centos 7 with gcc version: 4.8.5,official version, could compile successfully.)

After installing gcc, executing the script could run into this error: Parsing file (1/28): Callgraph clone;putchar;32;/usr/include/bits/stdio.h;79;1;<-;print_config;47;scripts/basic/fixdep.c;140;13;optimization:;inlining to Traceback (most recent call last): File "./kgraft-ipa-analysis.py", line 168, in <module> for line in open(f).readlines(): FileNotFoundError: [Errno 2] No such file or directory: 'Callgraph clone;putchar;32;/usr/include/bits/stdio.h;79;1;<-;print_config;47;scripts/basic/fixdep.c;140;13;optimization:;inlining to'

After inspecting the code, I think one should modify this code:

https://github.com/marxin/kgraft-analysis-tool/blob/0661faaa18ebe81d4e6e56aefa9a2026af9a5e79/kgraft-ipa-analysis.py#L157

to : files = [ args.file_list ]

And that would be ok. Enjoy.

[joe-lawrence]

Yeah, that script isn't exactly intuitive that you are supposed to pass it a file that contains a list of the files you are interested in analyzing. Should we actually consider this enhancement, we can modify the scripts so that it's easier to use and need to apply less sed filtering of the output. Here's my work in progress if you were interested: ipa-clones branch.

[CentUser]

Through the installation process, I think, this tool depends on a modified gcc(perhaps, some version of gcc might not support fdump-ipa-clones. CentOS-7 gcc-4.8.5, not recognize this parameter.). While compile and install a new gcc could be tricky. While this enhancement is very helpful.

[joe-lawrence]

Right, it requires newer version of gcc... Hence the WIP was attempting to detect that support. I think it might have been backported to later rhel 7 versions, but I'd have to go investigate that to verify.

On Fri, Oct 11, 2019 at 6:49 AM CentUser notifications@github.com wrote:

Through the installation process, I think, this tool depends on a modified gcc(perhaps, some version of gcc might not support fdump-ipa-clones. CentOS-7 gcc-4.8.5, not recognize this parameter.). While compile and install a new gcc could be tricky. While this enhancement is very helpful.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dynup/kpatch/issues/1045?email_source=notifications&email_token=ABOSXGLB6JEZMPSSX7NDWI3QOBK4TA5CNFSM4I5TMRD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEA7T3OI#issuecomment-541015481, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOSXGP6VHUWJMREOF5PD7LQOBK4TANCNFSM4I5TMRDQ .

[joe-lawrence]

No activity on this for a long time, closing.