diff mbox series

[v2] Makefile: support compressed debug info

Message ID 20200520193637.6015-1-ndesaulniers@google.com (mailing list archive)
State New, archived
Headers show
Series [v2] Makefile: support compressed debug info | expand

Commit Message

Nick Desaulniers May 20, 2020, 7:36 p.m. UTC
As debug information gets larger and larger, it helps significantly save
the size of vmlinux images to compress the information in the debug
information sections. Note: this debug info is typically split off from
the final compressed kernel image, which is why vmlinux is what's used
in conjunction with GDB. Minimizing the debug info size should have no
impact on boot times, or final compressed kernel image size.

All of the debug sections will have a `C` flag set.
$ readelf -S <object file>

$ bloaty vmlinux.gcc75.compressed.dwarf4 -- \
    vmlinux.gcc75.uncompressed.dwarf4

    FILE SIZE        VM SIZE
 --------------  --------------
  +0.0%     +18  [ = ]       0    [Unmapped]
 -73.3%  -114Ki  [ = ]       0    .debug_aranges
 -76.2% -2.01Mi  [ = ]       0    .debug_frame
 -73.6% -2.89Mi  [ = ]       0    .debug_str
 -80.7% -4.66Mi  [ = ]       0    .debug_abbrev
 -82.9% -4.88Mi  [ = ]       0    .debug_ranges
 -70.5% -9.04Mi  [ = ]       0    .debug_line
 -79.3% -10.9Mi  [ = ]       0    .debug_loc
 -39.5% -88.6Mi  [ = ]       0    .debug_info
 -18.2%  -123Mi  [ = ]       0    TOTAL

$ bloaty vmlinux.clang11.compressed.dwarf4 -- \
    vmlinux.clang11.uncompressed.dwarf4

    FILE SIZE        VM SIZE
 --------------  --------------
  +0.0%     +23  [ = ]       0    [Unmapped]
 -65.6%    -871  [ = ]       0    .debug_aranges
 -77.4% -1.84Mi  [ = ]       0    .debug_frame
 -82.9% -2.33Mi  [ = ]       0    .debug_abbrev
 -73.1% -2.43Mi  [ = ]       0    .debug_str
 -84.8% -3.07Mi  [ = ]       0    .debug_ranges
 -65.9% -8.62Mi  [ = ]       0    .debug_line
 -86.2% -40.0Mi  [ = ]       0    .debug_loc
 -42.0% -64.1Mi  [ = ]       0    .debug_info
 -22.1%  -122Mi  [ = ]       0    TOTAL

For x86_64 defconfig + LLVM=1 (before):
Elapsed (wall clock) time (h:mm:ss or m:ss): 3:22.03
Maximum resident set size (kbytes): 43856

For x86_64 defconfig + LLVM=1 (after):
Elapsed (wall clock) time (h:mm:ss or m:ss): 3:32.52
Maximum resident set size (kbytes): 1566776

Suggested-by: David Blaikie <blakie@google.com>
Suggested-by: Fangrui Song <maskray@google.com>
Suggested-by: Nick Clifton <nickc@redhat.com>
Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1 -> V2:
* rebase on linux-next.
* Add assembler flags as per Fangrui.
* Add note about KDEB_COMPRESS+scripts/package/builddeb
  as per Sedat and Masahiro.
* Add note about bintutils version requirements as per Nick C.
* Add note about measured increased build time and max RSS.

 Makefile          |  6 ++++++
 lib/Kconfig.debug | 15 +++++++++++++++
 2 files changed, 21 insertions(+)

Comments

Nick Desaulniers May 20, 2020, 11:21 p.m. UTC | #1
On Wed, May 20, 2020 at 12:36 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> As debug information gets larger and larger, it helps significantly save
> the size of vmlinux images to compress the information in the debug
> information sections. Note: this debug info is typically split off from
> the final compressed kernel image, which is why vmlinux is what's used
> in conjunction with GDB. Minimizing the debug info size should have no
> impact on boot times, or final compressed kernel image size.
>
> All of the debug sections will have a `C` flag set.
> $ readelf -S <object file>
>
> $ bloaty vmlinux.gcc75.compressed.dwarf4 -- \
>     vmlinux.gcc75.uncompressed.dwarf4
>
>     FILE SIZE        VM SIZE
>  --------------  --------------
>   +0.0%     +18  [ = ]       0    [Unmapped]
>  -73.3%  -114Ki  [ = ]       0    .debug_aranges
>  -76.2% -2.01Mi  [ = ]       0    .debug_frame
>  -73.6% -2.89Mi  [ = ]       0    .debug_str
>  -80.7% -4.66Mi  [ = ]       0    .debug_abbrev
>  -82.9% -4.88Mi  [ = ]       0    .debug_ranges
>  -70.5% -9.04Mi  [ = ]       0    .debug_line
>  -79.3% -10.9Mi  [ = ]       0    .debug_loc
>  -39.5% -88.6Mi  [ = ]       0    .debug_info
>  -18.2%  -123Mi  [ = ]       0    TOTAL
>
> $ bloaty vmlinux.clang11.compressed.dwarf4 -- \
>     vmlinux.clang11.uncompressed.dwarf4
>
>     FILE SIZE        VM SIZE
>  --------------  --------------
>   +0.0%     +23  [ = ]       0    [Unmapped]
>  -65.6%    -871  [ = ]       0    .debug_aranges
>  -77.4% -1.84Mi  [ = ]       0    .debug_frame
>  -82.9% -2.33Mi  [ = ]       0    .debug_abbrev
>  -73.1% -2.43Mi  [ = ]       0    .debug_str
>  -84.8% -3.07Mi  [ = ]       0    .debug_ranges
>  -65.9% -8.62Mi  [ = ]       0    .debug_line
>  -86.2% -40.0Mi  [ = ]       0    .debug_loc
>  -42.0% -64.1Mi  [ = ]       0    .debug_info
>  -22.1%  -122Mi  [ = ]       0    TOTAL
>
> For x86_64 defconfig + LLVM=1 (before):
> Elapsed (wall clock) time (h:mm:ss or m:ss): 3:22.03
> Maximum resident set size (kbytes): 43856
>
> For x86_64 defconfig + LLVM=1 (after):
> Elapsed (wall clock) time (h:mm:ss or m:ss): 3:32.52
> Maximum resident set size (kbytes): 1566776
>
> Suggested-by: David Blaikie <blakie@google.com>

Sorry, I have mistyped David's email, it should be:
Suggested-by: David Blaikie <blaikie@google.com>
(I missed the first `i` in the last name)

> Suggested-by: Fangrui Song <maskray@google.com>
> Suggested-by: Nick Clifton <nickc@redhat.com>
> Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1 -> V2:
> * rebase on linux-next.
> * Add assembler flags as per Fangrui.
> * Add note about KDEB_COMPRESS+scripts/package/builddeb
>   as per Sedat and Masahiro.
> * Add note about bintutils version requirements as per Nick C.
> * Add note about measured increased build time and max RSS.
>
>  Makefile          |  6 ++++++
>  lib/Kconfig.debug | 15 +++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 71687bfe1cd9..be8835296754 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -822,6 +822,12 @@ DEBUG_CFLAGS       += $(call cc-option, -femit-struct-debug-baseonly) \
>                    $(call cc-option,-fno-var-tracking)
>  endif
>
> +ifdef CONFIG_DEBUG_INFO_COMPRESSED
> +DEBUG_CFLAGS   += -gz=zlib
> +KBUILD_AFLAGS  += -Wa,--compress-debug-sections=zlib
> +KBUILD_LDFLAGS += --compress-debug-sections=zlib
> +endif
> +
>  KBUILD_CFLAGS += $(DEBUG_CFLAGS)
>  export DEBUG_CFLAGS
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index b8f023e054b9..5a423cbfaea4 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -225,6 +225,21 @@ config DEBUG_INFO_REDUCED
>           DEBUG_INFO build and compile times are reduced too.
>           Only works with newer gcc versions.
>
> +config DEBUG_INFO_COMPRESSED
> +       bool "Compressed debugging information"
> +       depends on DEBUG_INFO
> +       depends on $(cc-option,-gz=zlib)
> +       depends on $(as-option,-Wa,--compress-debug-sections=zlib)
> +       depends on $(ld-option,--compress-debug-sections=zlib)
> +       help
> +         Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
> +         5.0+, binutils 2.26+, and zlib.
> +
> +         Users of dpkg-deb via scripts/package/builddeb may
> +         wish to set the $KDEB_COMPRESS env var to "none" to avoid recompressing
> +         the debug info again with a different compression scheme, which can
> +         result in larger binaries.
> +
>  config DEBUG_INFO_SPLIT
>         bool "Produce split debuginfo in .dwo files"
>         depends on DEBUG_INFO
> --
> 2.26.2.761.g0e0b3e54be-goog
>
Andrew Morton May 20, 2020, 11:22 p.m. UTC | #2
On Wed, 20 May 2020 12:36:36 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:

> As debug information gets larger and larger, it helps significantly save
> the size of vmlinux images to compress the information in the debug
> information sections. Note: this debug info is typically split off from
> the final compressed kernel image, which is why vmlinux is what's used
> in conjunction with GDB. Minimizing the debug info size should have no
> impact on boot times, or final compressed kernel image size.
> 
> All of the debug sections will have a `C` flag set.
> $ readelf -S <object file>
> 
> $ bloaty vmlinux.gcc75.compressed.dwarf4 -- \
>     vmlinux.gcc75.uncompressed.dwarf4
> 
>     FILE SIZE        VM SIZE
>  --------------  --------------
>   +0.0%     +18  [ = ]       0    [Unmapped]
>  -73.3%  -114Ki  [ = ]       0    .debug_aranges
>  -76.2% -2.01Mi  [ = ]       0    .debug_frame
>  -73.6% -2.89Mi  [ = ]       0    .debug_str
>  -80.7% -4.66Mi  [ = ]       0    .debug_abbrev
>  -82.9% -4.88Mi  [ = ]       0    .debug_ranges
>  -70.5% -9.04Mi  [ = ]       0    .debug_line
>  -79.3% -10.9Mi  [ = ]       0    .debug_loc
>  -39.5% -88.6Mi  [ = ]       0    .debug_info
>  -18.2%  -123Mi  [ = ]       0    TOTAL
> 
> $ bloaty vmlinux.clang11.compressed.dwarf4 -- \
>     vmlinux.clang11.uncompressed.dwarf4
> 
>     FILE SIZE        VM SIZE
>  --------------  --------------
>   +0.0%     +23  [ = ]       0    [Unmapped]
>  -65.6%    -871  [ = ]       0    .debug_aranges
>  -77.4% -1.84Mi  [ = ]       0    .debug_frame
>  -82.9% -2.33Mi  [ = ]       0    .debug_abbrev
>  -73.1% -2.43Mi  [ = ]       0    .debug_str
>  -84.8% -3.07Mi  [ = ]       0    .debug_ranges
>  -65.9% -8.62Mi  [ = ]       0    .debug_line
>  -86.2% -40.0Mi  [ = ]       0    .debug_loc
>  -42.0% -64.1Mi  [ = ]       0    .debug_info
>  -22.1%  -122Mi  [ = ]       0    TOTAL
> 
> For x86_64 defconfig + LLVM=1 (before):
> Elapsed (wall clock) time (h:mm:ss or m:ss): 3:22.03
> Maximum resident set size (kbytes): 43856
> 
> For x86_64 defconfig + LLVM=1 (after):
> Elapsed (wall clock) time (h:mm:ss or m:ss): 3:32.52
> Maximum resident set size (kbytes): 1566776

I'm not sure who we're expecting to merge this, but I like shiny things ;)

> --- a/Makefile
> +++ b/Makefile
> @@ -822,6 +822,12 @@ DEBUG_CFLAGS	+= $(call cc-option, -femit-struct-debug-baseonly) \
>  		   $(call cc-option,-fno-var-tracking)
>  endif
>  
> +ifdef CONFIG_DEBUG_INFO_COMPRESSED
> +DEBUG_CFLAGS	+= -gz=zlib
> +KBUILD_AFLAGS	+= -Wa,--compress-debug-sections=zlib
> +KBUILD_LDFLAGS	+= --compress-debug-sections=zlib
> +endif
> +
>  KBUILD_CFLAGS += $(DEBUG_CFLAGS)
>  export DEBUG_CFLAGS
>  
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index b8f023e054b9..5a423cbfaea4 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -225,6 +225,21 @@ config DEBUG_INFO_REDUCED
>  	  DEBUG_INFO build and compile times are reduced too.
>  	  Only works with newer gcc versions.
>  
> +config DEBUG_INFO_COMPRESSED
> +	bool "Compressed debugging information"
> +	depends on DEBUG_INFO
> +	depends on $(cc-option,-gz=zlib)
> +	depends on $(as-option,-Wa,--compress-debug-sections=zlib)
> +	depends on $(ld-option,--compress-debug-sections=zlib)

huh, I didn't know kbuild/kconfig could do this.  Does it all work as
expected when cross-compiling?

> +	help
> +	  Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
> +	  5.0+, binutils 2.26+, and zlib.
> +
> +	  Users of dpkg-deb via scripts/package/builddeb may
> +	  wish to set the $KDEB_COMPRESS env var to "none" to avoid recompressing
> +	  the debug info again with a different compression scheme, which can
> +	  result in larger binaries.
> +
Masahiro Yamada May 21, 2020, 2:47 a.m. UTC | #3
On Thu, May 21, 2020 at 4:36 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> As debug information gets larger and larger, it helps significantly save
> the size of vmlinux images to compress the information in the debug
> information sections. Note: this debug info is typically split off from
> the final compressed kernel image, which is why vmlinux is what's used
> in conjunction with GDB. Minimizing the debug info size should have no
> impact on boot times, or final compressed kernel image size.
>
> All of the debug sections will have a `C` flag set.
> $ readelf -S <object file>
>
> $ bloaty vmlinux.gcc75.compressed.dwarf4 -- \
>     vmlinux.gcc75.uncompressed.dwarf4
>
>     FILE SIZE        VM SIZE
>  --------------  --------------
>   +0.0%     +18  [ = ]       0    [Unmapped]
>  -73.3%  -114Ki  [ = ]       0    .debug_aranges
>  -76.2% -2.01Mi  [ = ]       0    .debug_frame
>  -73.6% -2.89Mi  [ = ]       0    .debug_str
>  -80.7% -4.66Mi  [ = ]       0    .debug_abbrev
>  -82.9% -4.88Mi  [ = ]       0    .debug_ranges
>  -70.5% -9.04Mi  [ = ]       0    .debug_line
>  -79.3% -10.9Mi  [ = ]       0    .debug_loc
>  -39.5% -88.6Mi  [ = ]       0    .debug_info
>  -18.2%  -123Mi  [ = ]       0    TOTAL
>
> $ bloaty vmlinux.clang11.compressed.dwarf4 -- \
>     vmlinux.clang11.uncompressed.dwarf4
>
>     FILE SIZE        VM SIZE
>  --------------  --------------
>   +0.0%     +23  [ = ]       0    [Unmapped]
>  -65.6%    -871  [ = ]       0    .debug_aranges
>  -77.4% -1.84Mi  [ = ]       0    .debug_frame
>  -82.9% -2.33Mi  [ = ]       0    .debug_abbrev
>  -73.1% -2.43Mi  [ = ]       0    .debug_str
>  -84.8% -3.07Mi  [ = ]       0    .debug_ranges
>  -65.9% -8.62Mi  [ = ]       0    .debug_line
>  -86.2% -40.0Mi  [ = ]       0    .debug_loc
>  -42.0% -64.1Mi  [ = ]       0    .debug_info
>  -22.1%  -122Mi  [ = ]       0    TOTAL
>
> For x86_64 defconfig + LLVM=1 (before):
> Elapsed (wall clock) time (h:mm:ss or m:ss): 3:22.03
> Maximum resident set size (kbytes): 43856
>
> For x86_64 defconfig + LLVM=1 (after):
> Elapsed (wall clock) time (h:mm:ss or m:ss): 3:32.52
> Maximum resident set size (kbytes): 1566776
>
> Suggested-by: David Blaikie <blakie@google.com>
> Suggested-by: Fangrui Song <maskray@google.com>


Suggested-by -> Reviewed-by

https://patchwork.kernel.org/patch/11524939/#23349551



> Suggested-by: Nick Clifton <nickc@redhat.com>


I do not know where this tag came from.

Nick Clifton taught us the version rule of binutils,but did not state
anything about this patch itself.

https://patchwork.kernel.org/patch/11524939/#23355175


> Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>

I do not see the source of this tag, either...



> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---

 snip

> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -225,6 +225,21 @@ config DEBUG_INFO_REDUCED
>           DEBUG_INFO build and compile times are reduced too.
>           Only works with newer gcc versions.
>
> +config DEBUG_INFO_COMPRESSED
> +       bool "Compressed debugging information"
> +       depends on DEBUG_INFO
> +       depends on $(cc-option,-gz=zlib)
> +       depends on $(as-option,-Wa,--compress-debug-sections=zlib)

This does not work. (always false)
You cannot enable this option.

The comma between -Wa and --compress-debug-sections=zlib
is eaten by Kconfig parser because commas are delimiters
of function parameters.


Please write like this.

    depends on $(as-option,-Wa$(comma)--compress-debug-sections=zlib)





> +       depends on $(ld-option,--compress-debug-sections=zlib)
> +       help
> +         Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
> +         5.0+, binutils 2.26+, and zlib.
> +
> +         Users of dpkg-deb via scripts/package/builddeb may
> +         wish to set the $KDEB_COMPRESS env var to "none" to avoid recompressing
> +         the debug info again with a different compression scheme, which can
> +         result in larger binaries.

No. This is not correct.

CONFIG_DEBUG_INFO_COMPRESSED compresses the only debug info part.
The other parts still get by benefit from the default KDEB_COMPRESS=xz.


The numbers are here:


CONFIG_DEBUG_INFO_COMPRESSED=y
-rw-r--r-- 1 masahiro masahiro 209077584 May 21 11:19
linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-26_amd64.deb


CONFIG_DEBUG_INFO_COMPRESSED=y and KDEB_COMPRESS=none
-rw-r--r-- 1 masahiro masahiro 643051712 May 21 11:22
linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-27_amd64.deb


CONFIG_DEBUG_INFO_COMPRESSED=n
-rw-r--r-- 1 masahiro masahiro 112200308 May 21 11:40
linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-30_amd64.deb




For the deb package size perspective,
it is better to keep KDEB_COMPRESS as default.

The main motivation for changing KDEB_COMPRESS
is the build speed.  (see commit 1a7f0a34ea7d05)




CONFIG_DEBUG_INFO_COMPRESSED has a downside
for the debug deb package, but we need to accept it.
Nick Desaulniers May 21, 2020, 9:57 p.m. UTC | #4
On Wed, May 20, 2020 at 7:48 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> > Suggested-by: Fangrui Song <maskray@google.com>
>
>
> Suggested-by -> Reviewed-by
>
> https://patchwork.kernel.org/patch/11524939/#23349551

Yes, my mistake.

> > Suggested-by: Nick Clifton <nickc@redhat.com>
>
>
> I do not know where this tag came from.
>
> Nick Clifton taught us the version rule of binutils,but did not state
> anything about this patch itself.
>
> https://patchwork.kernel.org/patch/11524939/#23355175
>
>
> > Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
>
> I do not see the source of this tag, either...

Not all contributions to open source need to be in the form of
patches.  Both Sedat and Nick gave suggestions which ultimately
informed the contents of this patch.  They should be rewarded for
their efforts, and incentivized to help improve the code base further.
I think suggested by tags are a good way to do that; but if it's
against a written convention or if you still disagree, it's not the
end of the world to me, and you may drop those tags from the v3.

> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -225,6 +225,21 @@ config DEBUG_INFO_REDUCED
> >           DEBUG_INFO build and compile times are reduced too.
> >           Only works with newer gcc versions.
> >
> > +config DEBUG_INFO_COMPRESSED
> > +       bool "Compressed debugging information"
> > +       depends on DEBUG_INFO
> > +       depends on $(cc-option,-gz=zlib)
> > +       depends on $(as-option,-Wa,--compress-debug-sections=zlib)
>
> This does not work. (always false)

Technically, always true. `-Wa` disables all warnings from the
assembler.  Also, I did test this via `make menuconfig`.

> You cannot enable this option.
>
> The comma between -Wa and --compress-debug-sections=zlib
> is eaten by Kconfig parser because commas are delimiters
> of function parameters.
>
>
> Please write like this.
>
>     depends on $(as-option,-Wa$(comma)--compress-debug-sections=zlib)

You're right, I knew this bug forgot. Will fix in v3.

> > +       depends on $(ld-option,--compress-debug-sections=zlib)
> > +       help
> > +         Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
> > +         5.0+, binutils 2.26+, and zlib.
> > +
> > +         Users of dpkg-deb via scripts/package/builddeb may
> > +         wish to set the $KDEB_COMPRESS env var to "none" to avoid recompressing
> > +         the debug info again with a different compression scheme, which can
> > +         result in larger binaries.
>
> No. This is not correct.
>
> CONFIG_DEBUG_INFO_COMPRESSED compresses the only debug info part.
> The other parts still get by benefit from the default KDEB_COMPRESS=xz.
>
>
> The numbers are here:
>
>
> CONFIG_DEBUG_INFO_COMPRESSED=y
> -rw-r--r-- 1 masahiro masahiro 209077584 May 21 11:19
> linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-26_amd64.deb
>
>
> CONFIG_DEBUG_INFO_COMPRESSED=y and KDEB_COMPRESS=none
> -rw-r--r-- 1 masahiro masahiro 643051712 May 21 11:22
> linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-27_amd64.deb
>
>
> CONFIG_DEBUG_INFO_COMPRESSED=n
> -rw-r--r-- 1 masahiro masahiro 112200308 May 21 11:40
> linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-30_amd64.deb
>
>
>
>
> For the deb package size perspective,
> it is better to keep KDEB_COMPRESS as default.
>
> The main motivation for changing KDEB_COMPRESS
> is the build speed.  (see commit 1a7f0a34ea7d05)
>
>
>
>
> CONFIG_DEBUG_INFO_COMPRESSED has a downside
> for the debug deb package, but we need to accept it.

Ah, I see. Thank you for those measurements.  I'll send a v3 with
fixed up help text, but ultimately, I don't really care what it says
here.  Please feel empowered to reword it should you choose to accept
+ apply it.
Masahiro Yamada May 24, 2020, 3:56 a.m. UTC | #5
On Fri, May 22, 2020 at 6:57 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Wed, May 20, 2020 at 7:48 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > > Suggested-by: Fangrui Song <maskray@google.com>
> >
> >
> > Suggested-by -> Reviewed-by
> >
> > https://patchwork.kernel.org/patch/11524939/#23349551
>
> Yes, my mistake.
>
> > > Suggested-by: Nick Clifton <nickc@redhat.com>
> >
> >
> > I do not know where this tag came from.
> >
> > Nick Clifton taught us the version rule of binutils,but did not state
> > anything about this patch itself.
> >
> > https://patchwork.kernel.org/patch/11524939/#23355175
> >
> >
> > > Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
> >
> > I do not see the source of this tag, either...
>
> Not all contributions to open source need to be in the form of
> patches.  Both Sedat and Nick gave suggestions which ultimately
> informed the contents of this patch.  They should be rewarded for
> their efforts, and incentivized to help improve the code base further.
> I think suggested by tags are a good way to do that; but if it's
> against a written convention or if you still disagree, it's not the
> end of the world to me, and you may drop those tags from the v3.


Documentation/process/submitting-patches.rst
gives the guideline.


"A Suggested-by: tag indicates that the patch idea is suggested by the person
named and ensures credit to the person for the idea. Please note that this
tag should not be added without the reporter's permission, especially if the
idea was not posted in a public forum. That said, if we diligently credit our
idea reporters, they will, hopefully, be inspired to help us again in the
future."


I think this tag should be given to people who
gave the main idea to come up with
the main part of the patch.


Is that David Blaikie who suggested to
compress the debug info ?
If so, definitely he deserves to have Suggested-by tag.

For the others, I do not think Suggested-by is a good fit.

I appreciate their contribution to improve this patch.
So, maybe you can give credit in other form, for example,
mention it in the commit log explicitly?

Nick Clifton helped us to provide the minimal binutils version.
Sedat Dilet found an increase in size of debug .deb package.


Thanks.

>
> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > > @@ -225,6 +225,21 @@ config DEBUG_INFO_REDUCED
> > >           DEBUG_INFO build and compile times are reduced too.
> > >           Only works with newer gcc versions.
> > >
> > > +config DEBUG_INFO_COMPRESSED
> > > +       bool "Compressed debugging information"
> > > +       depends on DEBUG_INFO
> > > +       depends on $(cc-option,-gz=zlib)
> > > +       depends on $(as-option,-Wa,--compress-debug-sections=zlib)
> >
> > This does not work. (always false)
>
> Technically, always true. `-Wa` disables all warnings from the
> assembler.  Also, I did test this via `make menuconfig`.
>
> > You cannot enable this option.
> >
> > The comma between -Wa and --compress-debug-sections=zlib
> > is eaten by Kconfig parser because commas are delimiters
> > of function parameters.
> >
> >
> > Please write like this.
> >
> >     depends on $(as-option,-Wa$(comma)--compress-debug-sections=zlib)
>
> You're right, I knew this bug forgot. Will fix in v3.
>
> > > +       depends on $(ld-option,--compress-debug-sections=zlib)
> > > +       help
> > > +         Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
> > > +         5.0+, binutils 2.26+, and zlib.
> > > +
> > > +         Users of dpkg-deb via scripts/package/builddeb may
> > > +         wish to set the $KDEB_COMPRESS env var to "none" to avoid recompressing
> > > +         the debug info again with a different compression scheme, which can
> > > +         result in larger binaries.
> >
> > No. This is not correct.
> >
> > CONFIG_DEBUG_INFO_COMPRESSED compresses the only debug info part.
> > The other parts still get by benefit from the default KDEB_COMPRESS=xz.
> >
> >
> > The numbers are here:
> >
> >
> > CONFIG_DEBUG_INFO_COMPRESSED=y
> > -rw-r--r-- 1 masahiro masahiro 209077584 May 21 11:19
> > linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-26_amd64.deb
> >
> >
> > CONFIG_DEBUG_INFO_COMPRESSED=y and KDEB_COMPRESS=none
> > -rw-r--r-- 1 masahiro masahiro 643051712 May 21 11:22
> > linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-27_amd64.deb
> >
> >
> > CONFIG_DEBUG_INFO_COMPRESSED=n
> > -rw-r--r-- 1 masahiro masahiro 112200308 May 21 11:40
> > linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-30_amd64.deb
> >
> >
> >
> >
> > For the deb package size perspective,
> > it is better to keep KDEB_COMPRESS as default.
> >
> > The main motivation for changing KDEB_COMPRESS
> > is the build speed.  (see commit 1a7f0a34ea7d05)
> >
> >
> >
> >
> > CONFIG_DEBUG_INFO_COMPRESSED has a downside
> > for the debug deb package, but we need to accept it.
>
> Ah, I see. Thank you for those measurements.  I'll send a v3 with
> fixed up help text, but ultimately, I don't really care what it says
> here.  Please feel empowered to reword it should you choose to accept
> + apply it.
> --
> Thanks,
> ~Nick Desaulniers
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOd%3DjOr4ZaLx-dSNTqZnGRATY1PZktUfu4JGWKRwRH%3DUjnw%40mail.gmail.com.
Sedat Dilek May 24, 2020, 7:48 a.m. UTC | #6
On Sun, May 24, 2020 at 5:57 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, May 22, 2020 at 6:57 AM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > On Wed, May 20, 2020 at 7:48 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > > Suggested-by: Fangrui Song <maskray@google.com>
> > >
> > >
> > > Suggested-by -> Reviewed-by
> > >
> > > https://patchwork.kernel.org/patch/11524939/#23349551
> >
> > Yes, my mistake.
> >
> > > > Suggested-by: Nick Clifton <nickc@redhat.com>
> > >
> > >
> > > I do not know where this tag came from.
> > >
> > > Nick Clifton taught us the version rule of binutils,but did not state
> > > anything about this patch itself.
> > >
> > > https://patchwork.kernel.org/patch/11524939/#23355175
> > >
> > >
> > > > Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > >
> > > I do not see the source of this tag, either...
> >
> > Not all contributions to open source need to be in the form of
> > patches.  Both Sedat and Nick gave suggestions which ultimately
> > informed the contents of this patch.  They should be rewarded for
> > their efforts, and incentivized to help improve the code base further.
> > I think suggested by tags are a good way to do that; but if it's
> > against a written convention or if you still disagree, it's not the
> > end of the world to me, and you may drop those tags from the v3.
>
>
> Documentation/process/submitting-patches.rst
> gives the guideline.
>
>
> "A Suggested-by: tag indicates that the patch idea is suggested by the person
> named and ensures credit to the person for the idea. Please note that this
> tag should not be added without the reporter's permission, especially if the
> idea was not posted in a public forum. That said, if we diligently credit our
> idea reporters, they will, hopefully, be inspired to help us again in the
> future."
>
>
> I think this tag should be given to people who
> gave the main idea to come up with
> the main part of the patch.
>
>
> Is that David Blaikie who suggested to
> compress the debug info ?
> If so, definitely he deserves to have Suggested-by tag.
>
> For the others, I do not think Suggested-by is a good fit.
>
> I appreciate their contribution to improve this patch.
> So, maybe you can give credit in other form, for example,
> mention it in the commit log explicitly?
>
> Nick Clifton helped us to provide the minimal binutils version.
> Sedat Dilet found an increase in size of debug .deb package.
>
>
> Thanks.

Hi,

first my last name is Dilek - just for the sake of completeness.
No, it is not my first name as Dilek is a female Turkish first name,
so I do not want to change my gender.

So this discussions come up again and again.

Thus some own words on this - this is my personal opinion.

Like the author of Curl and DOH said at FOSDEM 2019 in Bruessel:
I am doing all this work - first - for myself - in my build and
developing environment.
Very very egoistically!

"Share knowledge aggressively!"
...was Nick's words at First ClangBuiltLinux Meetup in Zurich 2020.
In a 2nd round I share my knowledge and I like this - that's why I am
doing Open Source.

For me it sounds like a "Suggested-by" tag or other credits like
"Reviewed-by" have a higher value than a Tested-by tag.

*** The opposite is the case. ***

Here, I am on a Samsung SandyBridge CPU/GPU aka 2nd generation
ultrabook series runing Debian/testing AMD64.

A slightly modified Debian-kernel linux-config takes me approx. 5 (in
words five) hours of compiling and generating Debian packages.

Plus, testing.
Plus, testing.
Plus, testing.

In Linux-next times I run the whole Linux-Test-Project tests plus some
FIO tests.

Finally, I decide depending from what is new and interesting to me to
attend a full single Linux-kernel release cycle.
The last was Linux v5.3 which was the first release to be
compile/link-able - with no modifications - with LLVM/Clang/LLD v9.0.
For upcoming Linux v5.7 I have built each single RC Linux-kernel and
used it in my daily work!
Since RC1 - for me running on bare metal counts - checking QEMU or
other VM is nice - but showed me that says sometimes nothing.

Plus, I am building llvm-toolchains (LLVM/Clang/LLD) and testing with
them (and report if needed).

"...if we diligently credit our idea reporters, they will, hopefully,
be inspired to help us again in the future."

These are some motivating words...

My Tested-by is like a certificate - like a "Made in Germany" seal :-).

Virtual Greeting from North-West Germany,
- Sedat -
Nick Desaulniers May 26, 2020, 3:58 p.m. UTC | #7
On Sun, May 24, 2020 at 12:48 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> *** The opposite is the case. ***
>
> Here, I am on a Samsung SandyBridge CPU/GPU aka 2nd generation
> ultrabook series runing Debian/testing AMD64.
>
> A slightly modified Debian-kernel linux-config takes me approx. 5 (in
> words five) hours of compiling and generating Debian packages.
>
> Plus, testing.
> Plus, testing.
> Plus, testing.
>
> In Linux-next times I run the whole Linux-Test-Project tests plus some
> FIO tests.
>
> Finally, I decide depending from what is new and interesting to me to
> attend a full single Linux-kernel release cycle.
> The last was Linux v5.3 which was the first release to be
> compile/link-able - with no modifications - with LLVM/Clang/LLD v9.0.
> For upcoming Linux v5.7 I have built each single RC Linux-kernel and
> used it in my daily work!
> Since RC1 - for me running on bare metal counts - checking QEMU or
> other VM is nice - but showed me that says sometimes nothing.
>
> Plus, I am building llvm-toolchains (LLVM/Clang/LLD) and testing with
> them (and report if needed).

This is a lot of invaluable work.  It means the world to me Sedat!

>
> "...if we diligently credit our idea reporters, they will, hopefully,
> be inspired to help us again in the future."
>
> These are some motivating words...
>
> My Tested-by is like a certificate - like a "Made in Germany" seal :-).

I love this, it is.  Maybe if folks on this thread are bored, they
could help me with a personal project of mine?
https://github.com/nickdesaulniers/What-Open-Source-Means-To-Me
We could use more German, and Japanese.

>
> Virtual Greeting from North-West Germany,
> - Sedat -
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 71687bfe1cd9..be8835296754 100644
--- a/Makefile
+++ b/Makefile
@@ -822,6 +822,12 @@  DEBUG_CFLAGS	+= $(call cc-option, -femit-struct-debug-baseonly) \
 		   $(call cc-option,-fno-var-tracking)
 endif
 
+ifdef CONFIG_DEBUG_INFO_COMPRESSED
+DEBUG_CFLAGS	+= -gz=zlib
+KBUILD_AFLAGS	+= -Wa,--compress-debug-sections=zlib
+KBUILD_LDFLAGS	+= --compress-debug-sections=zlib
+endif
+
 KBUILD_CFLAGS += $(DEBUG_CFLAGS)
 export DEBUG_CFLAGS
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b8f023e054b9..5a423cbfaea4 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -225,6 +225,21 @@  config DEBUG_INFO_REDUCED
 	  DEBUG_INFO build and compile times are reduced too.
 	  Only works with newer gcc versions.
 
+config DEBUG_INFO_COMPRESSED
+	bool "Compressed debugging information"
+	depends on DEBUG_INFO
+	depends on $(cc-option,-gz=zlib)
+	depends on $(as-option,-Wa,--compress-debug-sections=zlib)
+	depends on $(ld-option,--compress-debug-sections=zlib)
+	help
+	  Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
+	  5.0+, binutils 2.26+, and zlib.
+
+	  Users of dpkg-deb via scripts/package/builddeb may
+	  wish to set the $KDEB_COMPRESS env var to "none" to avoid recompressing
+	  the debug info again with a different compression scheme, which can
+	  result in larger binaries.
+
 config DEBUG_INFO_SPLIT
 	bool "Produce split debuginfo in .dwo files"
 	depends on DEBUG_INFO