| Message ID | 20190121095338.8565-1-bp@alien8.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: Disable extra debugging info in .s output | expand |
Ping? On Mon, Jan 21, 2019 at 10:53:38AM +0100, Borislav Petkov wrote: > From: Borislav Petkov <bp@suse.de> > > Modern gcc adds view assignments, reset assertion checking in .loc > directives and a couple more additional debug markers, which clutters > the asm output unnecessarily: > > For example: > > bsp_resume: > .LFB3466: > .loc 1 1868 1 is_stmt 1 view -0 > .cfi_startproc > .loc 1 1869 2 view .LVU73 > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > .loc 1 1869 14 is_stmt 0 view .LVU74 > movq this_cpu(%rip), %rax # this_cpu, this_cpu > movq 64(%rax), %rax # this_cpu.94_1->c_bsp_resume, _2 > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > .loc 1 1869 5 view .LVU75 > testq %rax, %rax # _2 > je .L8 #, > .loc 1 1870 3 is_stmt 1 view .LVU76 > movq $boot_cpu_data, %rdi #, > jmp __x86_indirect_thunk_rax > > or > .loc 2 57 9 view .LVU478 > .loc 2 57 9 view .LVU479 > .loc 2 57 9 view .LVU480 > .loc 2 57 9 view .LVU481 > .LBB1385: > .LBB1383: > .LBB1379: > .LBB1377: > .LBB1375: > .loc 2 57 9 view .LVU482 > .loc 2 57 9 view .LVU483 > movl %edi, %edx # cpu, cpu > .LVL87: > .loc 2 57 9 is_stmt 0 view .LVU484 > > That MOV in there is drowned in debugging information and latter makes > it hard to follow the asm. And that DWARF info is not really needed for > asm output staring. > > Disable the debug information generation which clutters the asm output > unnecessarily: > > bsp_resume: > .LFB3466: > .loc 1 1868 1 > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > .loc 1 1869 14 > movq this_cpu(%rip), %rax # this_cpu, this_cpu > movq 64(%rax), %rax # this_cpu.94_1->c_bsp_resume, _2 > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > .loc 1 1869 5 > testq %rax, %rax # _2 > je .L8 #, > # arch/x86/kernel/cpu/common.c:1870: this_cpu->c_bsp_resume(&boot_cpu_data); > .loc 1 1870 3 > movq $boot_cpu_data, %rdi #, > jmp __x86_indirect_thunk_rax > > Signed-off-by: Borislav Petkov <bp@suse.de> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: Michal Marek <michal.lkml@markovi.net> > Cc: linux-kbuild@vger.kernel.org > --- > scripts/Makefile.build | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index fd03d60f6c5a..4f33fdab89d2 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -103,8 +103,12 @@ modkern_cflags = \ > $(KBUILD_CFLAGS_KERNEL) $(CFLAGS_KERNEL)) > quiet_modtag = $(if $(part-of-module),[M], ) > > +disable_extra_cc_dbg := $(call cc-option,-gno-as-locview-support) > +disable_extra_cc_dbg += $(call cc-option,-fno-dwarf2-cfi-asm) > +disable_extra_cc_dbg += $(call cc-option,-feliminate-unused-debug-symbols) > +disable_extra_cc_dbg += $(call cc-option,-gno-statement-frontiers) > quiet_cmd_cc_s_c = CC $(quiet_modtag) $@ > -cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< > +cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) $(disable_extra_cc_dbg) -fverbose-asm -S -o $@ $< > > $(obj)/%.s: $(src)/%.c FORCE > $(call if_changed_dep,cc_s_c) > -- > 2.19.1 >
On Thu, Jan 31, 2019 at 8:58 PM Borislav Petkov <bp@alien8.de> wrote: > > Ping? > > On Mon, Jan 21, 2019 at 10:53:38AM +0100, Borislav Petkov wrote: > > From: Borislav Petkov <bp@suse.de> > > > > Modern gcc adds view assignments, reset assertion checking in .loc > > directives and a couple more additional debug markers, which clutters > > the asm output unnecessarily: > > > > For example: > > > > bsp_resume: > > .LFB3466: > > .loc 1 1868 1 is_stmt 1 view -0 > > .cfi_startproc > > .loc 1 1869 2 view .LVU73 > > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > > .loc 1 1869 14 is_stmt 0 view .LVU74 > > movq this_cpu(%rip), %rax # this_cpu, this_cpu > > movq 64(%rax), %rax # this_cpu.94_1->c_bsp_resume, _2 > > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > > .loc 1 1869 5 view .LVU75 > > testq %rax, %rax # _2 > > je .L8 #, > > .loc 1 1870 3 is_stmt 1 view .LVU76 > > movq $boot_cpu_data, %rdi #, > > jmp __x86_indirect_thunk_rax > > > > or > > .loc 2 57 9 view .LVU478 > > .loc 2 57 9 view .LVU479 > > .loc 2 57 9 view .LVU480 > > .loc 2 57 9 view .LVU481 > > .LBB1385: > > .LBB1383: > > .LBB1379: > > .LBB1377: > > .LBB1375: > > .loc 2 57 9 view .LVU482 > > .loc 2 57 9 view .LVU483 > > movl %edi, %edx # cpu, cpu > > .LVL87: > > .loc 2 57 9 is_stmt 0 view .LVU484 > > > > That MOV in there is drowned in debugging information and latter makes > > it hard to follow the asm. And that DWARF info is not really needed for > > asm output staring. > > > > Disable the debug information generation which clutters the asm output > > unnecessarily: > > > > bsp_resume: > > .LFB3466: > > .loc 1 1868 1 > > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > > .loc 1 1869 14 > > movq this_cpu(%rip), %rax # this_cpu, this_cpu > > movq 64(%rax), %rax # this_cpu.94_1->c_bsp_resume, _2 > > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > > .loc 1 1869 5 > > testq %rax, %rax # _2 > > je .L8 #, > > # arch/x86/kernel/cpu/common.c:1870: this_cpu->c_bsp_resume(&boot_cpu_data); > > .loc 1 1870 3 > > movq $boot_cpu_data, %rdi #, > > jmp __x86_indirect_thunk_rax > > > > Signed-off-by: Borislav Petkov <bp@suse.de> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > Cc: Michal Marek <michal.lkml@markovi.net> > > Cc: linux-kbuild@vger.kernel.org > > --- > > scripts/Makefile.build | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > > index fd03d60f6c5a..4f33fdab89d2 100644 > > --- a/scripts/Makefile.build > > +++ b/scripts/Makefile.build > > @@ -103,8 +103,12 @@ modkern_cflags = \ > > $(KBUILD_CFLAGS_KERNEL) $(CFLAGS_KERNEL)) > > quiet_modtag = $(if $(part-of-module),[M], ) > > > > +disable_extra_cc_dbg := $(call cc-option,-gno-as-locview-support) > > +disable_extra_cc_dbg += $(call cc-option,-fno-dwarf2-cfi-asm) > > +disable_extra_cc_dbg += $(call cc-option,-feliminate-unused-debug-symbols) > > +disable_extra_cc_dbg += $(call cc-option,-gno-statement-frontiers) This would make the build speed slower because the compiler is invoked to check those flags in every directory. I tested this patch with x86_64_defconfig and GCC 8.2 (on ubuntu 18.10 in docker) but I saw no effect of this patch. CONFIG_DEBUG_INFO adds dwarf info, and clutters asm output. Did you enable CONFIG_DEBUG_INFO when you were debugging asm files?
On Fri, Feb 01, 2019 at 12:06:13PM +0900, Masahiro Yamada wrote: > This would make the build speed slower > because the compiler is invoked to check those flags in every directory. > > I tested this patch with x86_64_defconfig and GCC 8.2 > (on ubuntu 18.10 in docker) > but I saw no effect of this patch. Yes, because that's the .s target which is meant to be used to convert only a *single* .c file to assembly in order to stare at the resulting asm. For example: make arch/x86/kernel/cpu/common.s It is a debugging aid - and a very basic and important one - and is not usually used during a normal kernel build. AFAIK. So we shouldn't worry about any slowdowns here. Thx.
On Fri, Feb 1, 2019 at 6:50 PM Borislav Petkov <bp@alien8.de> wrote: > > On Fri, Feb 01, 2019 at 12:06:13PM +0900, Masahiro Yamada wrote: > > This would make the build speed slower > > because the compiler is invoked to check those flags in every directory. > > > > I tested this patch with x86_64_defconfig and GCC 8.2 > > (on ubuntu 18.10 in docker) > > but I saw no effect of this patch. > > Yes, because that's the .s target which is meant to be used to convert > only a *single* .c file to assembly in order to stare at the resulting > asm. For example: > > make arch/x86/kernel/cpu/common.s I do know this, and did compare the output from the *single* target "make arch/x86/kernel/cpu/common.s" > It is a debugging aid - and a very basic and important one - and is not > usually used during a normal kernel build. AFAIK. > > So we shouldn't worry about any slowdowns here. Your patch does slowdown the build system extremely. I provide the result of "perf stat" of the incremental build for you. Without your patch: Performance counter stats for 'make -j8' (20 runs): 2.721312445 seconds time elapsed ( +- 1.02% ) With you patch: Performance counter stats for 'make -j8' (20 runs): 5.038521415 seconds time elapsed ( +- 0.56% ) Why should the normal build affected by the debugging aid? One more thing, you did not answer my question. If you are complaining about the DWARF info enabled by CONFIG_DEBUG_INFO, I recommend to turn off CONFIG_DEBUG_INFO. > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. -- Best Regards Masahiro Yamada
On Fri, Feb 01, 2019 at 07:09:36PM +0900, Masahiro Yamada wrote: > Why should the normal build affected by the debugging aid? Hmm, ok, so then that target really is being used during the normal build. I don't know why yet so I'll have to do some more staring. > One more thing, you did not answer my question. > > If you are complaining about the DWARF info > enabled by CONFIG_DEBUG_INFO, > I recommend to turn off CONFIG_DEBUG_INFO. Yes but I don't want to be changing .config just so that I can look at the asm output. I'll think about a better solution. Thx.
On Fri, Feb 01, 2019 at 07:09:36PM +0900, Masahiro Yamada wrote: > I provide the result of "perf stat" of the incremental build for you. One more question: what exactly is that "incremental build" you're measuring with? Exact command line please. > Without your patch: > > > Performance counter stats for 'make -j8' (20 runs): > > 2.721312445 seconds time elapsed > ( +- 1.02% ) > > > > With you patch: > > > Performance counter stats for 'make -j8' (20 runs): > > 5.038521415 seconds time elapsed > ( +- 0.56% ) This can't be a full kernel build. Thx.
On Fri, Feb 01, 2019 at 11:30:38AM +0100, Borislav Petkov wrote:
> This can't be a full kernel build.
So I did this:
$(obj)/%.s: $(src)/%.c FORCE
@echo "HERE\n"
$(call if_changed_dep,cc_s_c)
and did a full kernel build with my .config. I got exactly *three*
"HERE"s in the build log.
So pls explain in more detail how you're measuring that slowdown.
Thx.
On Fri, Feb 1, 2019 at 7:32 PM Borislav Petkov <bp@alien8.de> wrote: > > On Fri, Feb 01, 2019 at 07:09:36PM +0900, Masahiro Yamada wrote: > > I provide the result of "perf stat" of the incremental build for you. > > One more question: what exactly is that "incremental build" you're > measuring with? I meant "make" after you the full build > Exact command line please. $ make defconfig $ make -j8 [Wait until the full build finished] $ perf stat --repeat 20 --null make -j8 [Wait until the perf result is displayed] > > Without your patch: > > > > > > Performance counter stats for 'make -j8' (20 runs): > > > > 2.721312445 seconds time elapsed > > ( +- 1.02% ) > > > > > > > > With you patch: > > > > > > Performance counter stats for 'make -j8' (20 runs): > > > > 5.038521415 seconds time elapsed > > ( +- 0.56% ) > > This can't be a full kernel build. > > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. -- Best Regards Masahiro Yamada
On Fri, Feb 1, 2019 at 7:25 PM Borislav Petkov <bp@alien8.de> wrote: > > On Fri, Feb 01, 2019 at 07:09:36PM +0900, Masahiro Yamada wrote: > > Why should the normal build affected by the debugging aid? > > Hmm, ok, so then that target really is being used during the normal > build. I don't know why yet so I'll have to do some more staring. > > > One more thing, you did not answer my question. > > > > If you are complaining about the DWARF info > > enabled by CONFIG_DEBUG_INFO, > > I recommend to turn off CONFIG_DEBUG_INFO. > > Yes but I don't want to be changing .config just so that I can look at > the asm output. Generally speaking, the build system should do what was requested by a set of CONFIG options. If you want to turn off DWARF info, you should disable the corresponding CONFIG. If you want to tweak the behavior from the command line, KCFLAGS is available though. make arch/x86/kernel/cpu/common.s KCFLAGS="-gno-as-locview-support -fno-dwarf2-cfi-asm -feliminate-unused-debug-symbols -gno-statement-frontiers" > I'll think about a better solution. > > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply.
On Fri, Feb 1, 2019 at 7:38 PM Borislav Petkov <bp@alien8.de> wrote: > > On Fri, Feb 01, 2019 at 11:30:38AM +0100, Borislav Petkov wrote: > > This can't be a full kernel build. > > So I did this: > > $(obj)/%.s: $(src)/%.c FORCE > @echo "HERE\n" > $(call if_changed_dep,cc_s_c) > > and did a full kernel build with my .config. I got exactly *three* > "HERE"s in the build log. > > So pls explain in more detail how you're measuring that slowdown. Variables assigned with ':=' are evaluated just on parsing Makefile. The scripts/Makefile.build is parsed over and over again, so the compiler is invoked hundreds times to test these four options even when you are not actually building any new objects. It is mitigated by replacing ':=' with '=', but they are still evaluated multiple times when generating asm-offset. Frankly, you cannot use cc-option in scripts/Makefile.build Your next question might be, "Is it OK to implement this in the top Makefile?" It would be better, but I am not convinced to do it.
On Fri, Feb 01, 2019 at 08:58:13PM +0900, Masahiro Yamada wrote: > Variables assigned with ':=' are evaluated just on parsing Makefile. > > The scripts/Makefile.build is parsed over and over again, > so the compiler is invoked hundreds times to test these four options > even when you are not actually building any new objects. > > It is mitigated by replacing ':=' with '=', > but they are still evaluated multiple times when generating asm-offset. Now that you mention it, how about ?= disable_extra_cc_dbg ?= $(call cc-option,-gno-as-locview-support) disable_extra_cc_dbg += $(call cc-option,-fno-dwarf2-cfi-asm) disable_extra_cc_dbg += $(call cc-option,-feliminate-unused-debug-symbols) disable_extra_cc_dbg += $(call cc-option,-gno-statement-frontiers) With it, the slowdown of the incremental build is ~0.2s (on an old laptop): defconfig without: 9.6128 +- 0.0327 seconds time elapsed ( +- 0.34% ) vs defconfig with: 9.8483 +- 0.0273 seconds time elapsed ( +- 0.28% )
On Sat, Feb 2, 2019 at 12:12 AM Borislav Petkov <bp@alien8.de> wrote: > > On Fri, Feb 01, 2019 at 08:58:13PM +0900, Masahiro Yamada wrote: > > Variables assigned with ':=' are evaluated just on parsing Makefile. > > > > The scripts/Makefile.build is parsed over and over again, > > so the compiler is invoked hundreds times to test these four options > > even when you are not actually building any new objects. > > > > It is mitigated by replacing ':=' with '=', > > but they are still evaluated multiple times when generating asm-offset. > > Now that you mention it, how about ?= '?=' is the same as '=' here. > disable_extra_cc_dbg ?= $(call cc-option,-gno-as-locview-support) > disable_extra_cc_dbg += $(call cc-option,-fno-dwarf2-cfi-asm) > disable_extra_cc_dbg += $(call cc-option,-feliminate-unused-debug-symbols) > disable_extra_cc_dbg += $(call cc-option,-gno-statement-frontiers) > > With it, the slowdown of the incremental build is ~0.2s (on an old laptop): > > defconfig without: 9.6128 +- 0.0327 seconds time elapsed ( +- 0.34% ) > > vs > > defconfig with: 9.8483 +- 0.0273 seconds time elapsed ( +- 0.28% ) > > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply.
On Sat, Feb 02, 2019 at 10:48:00PM +0900, Masahiro Yamada wrote:
> '?=' is the same as '=' here.
Sure but if the slowdown disappears, then make does something else for
'?=' apparently vs for '='.
On Sat, Feb 02, 2019 at 03:42:27PM +0100, Borislav Petkov wrote: > On Sat, Feb 02, 2019 at 10:48:00PM +0900, Masahiro Yamada wrote: > > '?=' is the same as '=' here. > > Sure but if the slowdown disappears, then make does something else for > '?=' apparently vs for '='. Ok, let me ask a different question then: would a separate target which will be used to generate only clean asm for debugging purposes ala make <path-to-file>.asm or so be an acceptable approach? Thx.
On Wed, Feb 6, 2019 at 7:49 PM Borislav Petkov <bp@alien8.de> wrote: > > On Sat, Feb 02, 2019 at 03:42:27PM +0100, Borislav Petkov wrote: > > On Sat, Feb 02, 2019 at 10:48:00PM +0900, Masahiro Yamada wrote: > > > '?=' is the same as '=' here. > > > > Sure but if the slowdown disappears, then make does something else for > > '?=' apparently vs for '='. '=' and '?=' are the same in the sense that the right-hand side is evaluated when it is used. > Ok, let me ask a different question then: would a separate target which > will be used to generate only clean asm for debugging purposes ala > > make <path-to-file>.asm Sorry, I do not like to duplicate the code. I do not understand why you need to test such complicated compiler flags in order to negate CONFIG_DEBUG_INFO. I am still wondering, but if this is really worth doing in upstream code, I think the following is a simpler idea. diff --git a/Makefile b/Makefile index 3142e67..131164a 100644 --- a/Makefile +++ b/Makefile @@ -729,25 +729,28 @@ KBUILD_CFLAGS += -fomit-frame-pointer endif endif -KBUILD_CFLAGS += $(call cc-option, -fno-var-tracking-assignments) +DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments) ifdef CONFIG_DEBUG_INFO ifdef CONFIG_DEBUG_INFO_SPLIT -KBUILD_CFLAGS += $(call cc-option, -gsplit-dwarf, -g) +DEBUG_CFLAGS += $(call cc-option, -gsplit-dwarf, -g) else -KBUILD_CFLAGS += -g +DEBUG_CFLAGS += -g endif KBUILD_AFLAGS += -Wa,-gdwarf-2 endif ifdef CONFIG_DEBUG_INFO_DWARF4 -KBUILD_CFLAGS += $(call cc-option, -gdwarf-4,) +DEBUG_CFLAGS += $(call cc-option, -gdwarf-4,) endif ifdef CONFIG_DEBUG_INFO_REDUCED -KBUILD_CFLAGS += $(call cc-option, -femit-struct-debug-baseonly) \ +DEBUG_CFLAGS += $(call cc-option, -femit-struct-debug-baseonly) \ $(call cc-option,-fno-var-tracking) endif +KBUILD_CFLAGS += $(DEBUG_CFLAGS) +export DEBUG_CFLAGS + ifdef CONFIG_FUNCTION_TRACER ifdef CONFIG_FTRACE_MCOUNT_RECORD # gcc 5 supports generating the mcount tables directly diff --git a/scripts/Makefile.build b/scripts/Makefile.build index fd03d60..bc8e0ae 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -104,7 +104,7 @@ modkern_cflags = \ quiet_modtag = $(if $(part-of-module),[M], ) quiet_cmd_cc_s_c = CC $(quiet_modtag) $@ -cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< +cmd_cc_s_c = $(CC) $(filter-out $(DEBUG_CFLAGS), $(c_flags)) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< $(obj)/%.s: $(src)/%.c FORCE $(call if_changed_dep,cc_s_c) > or so be an acceptable approach? > > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. -- Best Regards Masahiro Yamada
On Sun, Feb 10, 2019 at 03:51:01PM +0900, Masahiro Yamada wrote: > I am still wondering, > but if this is really worth doing in upstream code, Yes, it is very worth doing it: make <path-to-file>.s is one of the basic steps one does when trying to pinpoint the Code: line in a splat back to the code gcc generated. > I think the following is a simpler idea. Yes, you can do it this way too but I can't apply it here to play with it because gmail probably corrupted the diff: checking file Makefile Hunk #1 FAILED at 729. 1 out of 1 hunk FAILED checking file scripts/Makefile.build patch: **** malformed patch at line 98: $(DISABLE_LTO) -fverbose-asm -S -o $@ $< Please attach the diff or send it from another mail server. Thx.
On Sun, Feb 10, 2019 at 01:39:23PM +0100, Borislav Petkov wrote: > Please attach the diff or send it from another mail server. So you couldn't be bothered to send me an applicable version so I went and typed it in by hand. Thanks. ;-\ Anyway, this variant works too, pls queue it. Acked-by: Borislav Petkov <bp@suse.de> Tested-by: Borislav Petkov <bp@suse.de> Thx.
Borislav, On Mon, Feb 18, 2019 at 5:30 PM Borislav Petkov <bp@alien8.de> wrote: > > On Sun, Feb 10, 2019 at 01:39:23PM +0100, Borislav Petkov wrote: > > Please attach the diff or send it from another mail server. > > So you couldn't be bothered to send me an applicable version so I went > and typed it in by hand. Thanks. ;-\ I admit I must fix my unfriendly attitude. Sorry for annoying you. > Anyway, this variant works too, pls queue it. > > Acked-by: Borislav Petkov <bp@suse.de> > Tested-by: Borislav Petkov <bp@suse.de> Could you send v2 as I suggested? Your commit log describes your motivation perfectly. (I lost the applicable patch because I just modified the code locally and pasted it into the previous email.) > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply.
On Mon, Feb 18, 2019 at 06:10:57PM +0900, Masahiro Yamada wrote: > Could you send v2 as I suggested? > Your commit log describes your motivation perfectly. > > (I lost the applicable patch because > I just modified the code locally and pasted it > into the previous email.) Sure, here it is. Thx. --- From: Masahiro Yamada <yamada.masahiro@socionext.com> Date: Sun, 25 Nov 2018 23:58:00 +0100 Subject: [PATCH] kbuild: Disable extra debugging info in .s output Modern gcc adds view assignments, reset assertion checking in .loc directives and a couple more additional debug markers, which clutters the asm output unnecessarily: For example: bsp_resume: .LFB3466: .loc 1 1868 1 is_stmt 1 view -0 .cfi_startproc .loc 1 1869 2 view .LVU73 # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) .loc 1 1869 14 is_stmt 0 view .LVU74 movq this_cpu(%rip), %rax # this_cpu, this_cpu movq 64(%rax), %rax # this_cpu.94_1->c_bsp_resume, _2 # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) .loc 1 1869 5 view .LVU75 testq %rax, %rax # _2 je .L8 #, .loc 1 1870 3 is_stmt 1 view .LVU76 movq $boot_cpu_data, %rdi #, jmp __x86_indirect_thunk_rax or .loc 2 57 9 view .LVU478 .loc 2 57 9 view .LVU479 .loc 2 57 9 view .LVU480 .loc 2 57 9 view .LVU481 .LBB1385: .LBB1383: .LBB1379: .LBB1377: .LBB1375: .loc 2 57 9 view .LVU482 .loc 2 57 9 view .LVU483 movl %edi, %edx # cpu, cpu .LVL87: .loc 2 57 9 is_stmt 0 view .LVU484 That MOV in there is drowned in debugging information and latter makes it hard to follow the asm. And that DWARF info is not really needed for asm output staring. Disable the debug information generation which clutters the asm output unnecessarily: bsp_resume: # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) movq this_cpu(%rip), %rax # this_cpu, this_cpu movq 64(%rax), %rax # this_cpu.94_1->c_bsp_resume, _2 # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) testq %rax, %rax # _2 je .L8 #, # arch/x86/kernel/cpu/common.c:1870: this_cpu->c_bsp_resume(&boot_cpu_data); movq $boot_cpu_data, %rdi #, jmp __x86_indirect_thunk_rax .L8: # arch/x86/kernel/cpu/common.c:1871: } rep ret .size bsp_resume, .-bsp_resume [ bp: write commit message. ] Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Signed-off-by: Borislav Petkov <bp@suse.de> Cc: Michal Marek <michal.lkml@markovi.net> Cc: linux-kbuild@vger.kernel.org --- Makefile | 13 ++++++++----- scripts/Makefile.build | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 96c5335e7ee4..c5e7d3ac8bd2 100644 --- a/Makefile +++ b/Makefile @@ -729,25 +729,28 @@ KBUILD_CFLAGS += -fomit-frame-pointer endif endif -KBUILD_CFLAGS += $(call cc-option, -fno-var-tracking-assignments) +DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments) ifdef CONFIG_DEBUG_INFO ifdef CONFIG_DEBUG_INFO_SPLIT -KBUILD_CFLAGS += $(call cc-option, -gsplit-dwarf, -g) +DEBUG_CFLAGS += $(call cc-option, -gsplit-dwarf, -g) else -KBUILD_CFLAGS += -g +DEBUG_CFLAGS += -g endif KBUILD_AFLAGS += -Wa,-gdwarf-2 endif ifdef CONFIG_DEBUG_INFO_DWARF4 -KBUILD_CFLAGS += $(call cc-option, -gdwarf-4,) +DEBUG_CFLAGS += $(call cc-option, -gdwarf-4,) endif ifdef CONFIG_DEBUG_INFO_REDUCED -KBUILD_CFLAGS += $(call cc-option, -femit-struct-debug-baseonly) \ +DEBUG_CFLAGS += $(call cc-option, -femit-struct-debug-baseonly) \ $(call cc-option,-fno-var-tracking) endif +KBUILD_CFLAGS += $(DEBUG_CFLAGS) +export DEBUG_CFLAGS + ifdef CONFIG_FUNCTION_TRACER ifdef CONFIG_FTRACE_MCOUNT_RECORD # gcc 5 supports generating the mcount tables directly diff --git a/scripts/Makefile.build b/scripts/Makefile.build index fd03d60f6c5a..8f5c86da77b1 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -104,7 +104,7 @@ modkern_cflags = \ quiet_modtag = $(if $(part-of-module),[M], ) quiet_cmd_cc_s_c = CC $(quiet_modtag) $@ -cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< +cmd_cc_s_c = $(CC) $(filter-out $(DEBUG_CFLAGS), $(c_flags)) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< $(obj)/%.s: $(src)/%.c FORCE $(call if_changed_dep,cc_s_c)
On Mon, Feb 18, 2019 at 11:32 PM Borislav Petkov <bp@alien8.de> wrote: > > On Mon, Feb 18, 2019 at 06:10:57PM +0900, Masahiro Yamada wrote: > > Could you send v2 as I suggested? > > Your commit log describes your motivation perfectly. > > > > (I lost the applicable patch because > > I just modified the code locally and pasted it > > into the previous email.) > > Sure, here it is. > > Thx. Applied to linux-kbuild. Thanks. > --- > From: Masahiro Yamada <yamada.masahiro@socionext.com> > Date: Sun, 25 Nov 2018 23:58:00 +0100 > Subject: [PATCH] kbuild: Disable extra debugging info in .s output > > Modern gcc adds view assignments, reset assertion checking in .loc > directives and a couple more additional debug markers, which clutters > the asm output unnecessarily: > > For example: > > bsp_resume: > .LFB3466: > .loc 1 1868 1 is_stmt 1 view -0 > .cfi_startproc > .loc 1 1869 2 view .LVU73 > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > .loc 1 1869 14 is_stmt 0 view .LVU74 > movq this_cpu(%rip), %rax # this_cpu, this_cpu > movq 64(%rax), %rax # this_cpu.94_1->c_bsp_resume, _2 > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > .loc 1 1869 5 view .LVU75 > testq %rax, %rax # _2 > je .L8 #, > .loc 1 1870 3 is_stmt 1 view .LVU76 > movq $boot_cpu_data, %rdi #, > jmp __x86_indirect_thunk_rax > > or > .loc 2 57 9 view .LVU478 > .loc 2 57 9 view .LVU479 > .loc 2 57 9 view .LVU480 > .loc 2 57 9 view .LVU481 > .LBB1385: > .LBB1383: > .LBB1379: > .LBB1377: > .LBB1375: > .loc 2 57 9 view .LVU482 > .loc 2 57 9 view .LVU483 > movl %edi, %edx # cpu, cpu > .LVL87: > .loc 2 57 9 is_stmt 0 view .LVU484 > > That MOV in there is drowned in debugging information and latter makes > it hard to follow the asm. And that DWARF info is not really needed for > asm output staring. > > Disable the debug information generation which clutters the asm output > unnecessarily: > > bsp_resume: > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > movq this_cpu(%rip), %rax # this_cpu, this_cpu > movq 64(%rax), %rax # this_cpu.94_1->c_bsp_resume, _2 > # arch/x86/kernel/cpu/common.c:1869: if (this_cpu->c_bsp_resume) > testq %rax, %rax # _2 > je .L8 #, > # arch/x86/kernel/cpu/common.c:1870: this_cpu->c_bsp_resume(&boot_cpu_data); > movq $boot_cpu_data, %rdi #, > jmp __x86_indirect_thunk_rax > .L8: > # arch/x86/kernel/cpu/common.c:1871: } > rep ret > .size bsp_resume, .-bsp_resume > > [ bp: write commit message. ] > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > Signed-off-by: Borislav Petkov <bp@suse.de> > Cc: Michal Marek <michal.lkml@markovi.net> > Cc: linux-kbuild@vger.kernel.org > --- > Makefile | 13 ++++++++----- > scripts/Makefile.build | 2 +- > 2 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/Makefile b/Makefile > index 96c5335e7ee4..c5e7d3ac8bd2 100644 > --- a/Makefile > +++ b/Makefile > @@ -729,25 +729,28 @@ KBUILD_CFLAGS += -fomit-frame-pointer > endif > endif > > -KBUILD_CFLAGS += $(call cc-option, -fno-var-tracking-assignments) > +DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments) > > ifdef CONFIG_DEBUG_INFO > ifdef CONFIG_DEBUG_INFO_SPLIT > -KBUILD_CFLAGS += $(call cc-option, -gsplit-dwarf, -g) > +DEBUG_CFLAGS += $(call cc-option, -gsplit-dwarf, -g) > else > -KBUILD_CFLAGS += -g > +DEBUG_CFLAGS += -g > endif > KBUILD_AFLAGS += -Wa,-gdwarf-2 > endif > ifdef CONFIG_DEBUG_INFO_DWARF4 > -KBUILD_CFLAGS += $(call cc-option, -gdwarf-4,) > +DEBUG_CFLAGS += $(call cc-option, -gdwarf-4,) > endif > > ifdef CONFIG_DEBUG_INFO_REDUCED > -KBUILD_CFLAGS += $(call cc-option, -femit-struct-debug-baseonly) \ > +DEBUG_CFLAGS += $(call cc-option, -femit-struct-debug-baseonly) \ > $(call cc-option,-fno-var-tracking) > endif > > +KBUILD_CFLAGS += $(DEBUG_CFLAGS) > +export DEBUG_CFLAGS > + > ifdef CONFIG_FUNCTION_TRACER > ifdef CONFIG_FTRACE_MCOUNT_RECORD > # gcc 5 supports generating the mcount tables directly > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index fd03d60f6c5a..8f5c86da77b1 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -104,7 +104,7 @@ modkern_cflags = \ > quiet_modtag = $(if $(part-of-module),[M], ) > > quiet_cmd_cc_s_c = CC $(quiet_modtag) $@ > -cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< > +cmd_cc_s_c = $(CC) $(filter-out $(DEBUG_CFLAGS), $(c_flags)) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< > > $(obj)/%.s: $(src)/%.c FORCE > $(call if_changed_dep,cc_s_c) > -- > 2.19.1 > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply.
diff --git a/scripts/Makefile.build b/scripts/Makefile.build index fd03d60f6c5a..4f33fdab89d2 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -103,8 +103,12 @@ modkern_cflags = \ $(KBUILD_CFLAGS_KERNEL) $(CFLAGS_KERNEL)) quiet_modtag = $(if $(part-of-module),[M], ) +disable_extra_cc_dbg := $(call cc-option,-gno-as-locview-support) +disable_extra_cc_dbg += $(call cc-option,-fno-dwarf2-cfi-asm) +disable_extra_cc_dbg += $(call cc-option,-feliminate-unused-debug-symbols) +disable_extra_cc_dbg += $(call cc-option,-gno-statement-frontiers) quiet_cmd_cc_s_c = CC $(quiet_modtag) $@ -cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< +cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) $(disable_extra_cc_dbg) -fverbose-asm -S -o $@ $< $(obj)/%.s: $(src)/%.c FORCE $(call if_changed_dep,cc_s_c)