[PATCH] kbuild: Disable extra debugging info in .s output

[PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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
> 

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Masahiro Yamada]

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?


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Masahiro Yamada]

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

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[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.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Masahiro Yamada]

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.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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% )


-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Masahiro Yamada]

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.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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 '='.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Masahiro Yamada]

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

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Masahiro Yamada]

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.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Borislav Petkov]

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)
-- 
2.19.1

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Masahiro Yamada]

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.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: Disable extra debugging info in .s output

[Masahiro Yamada]

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