Hi,
In case people didn't notice in the somewhat long analysis of the issue in the bug, the workaround is just a simple oneliner:
diff --git a/kernel.spec b/kernel.spec index cb3dec8..29c198a 100644 --- a/kernel.spec +++ b/kernel.spec @@ -183,6 +183,9 @@ Summary: The Linux kernel %define _enable_debug_packages 0 %endif %define debuginfodir /usr/lib/debug +# Needed because we override almost everything involving build-ids +# and debuginfo generation. Currently we rely on the old alldebug setting. +%global _build_id_links alldebug
# kernel PAE is only built on i686 and ARMv7. %ifnarch i686 armv7hl
Of course it would be nice if someone could cleanup the various places that the kernel.spec overrides rpm find-debuginfo.sh and debugedit and provides some requirements that would make this all easier for the kernel build. It looks like the current build does a lot redundant extra work that might be prevented if rpm provided better hooks to do automagically what the kernel spec build requires. One thing rpm wants to introduce in the future (already upstream) is parallel processing of debug files. Which the current kernel.spec seems to prevent because it serializes the processing itself already.
I would be happy to review any feedback on why the kernel.spec has the current hacks and suggestions for improvements to make this smoother.
Cheers,
Mark
On 03/20/2017 01:46 AM, Mark Wielaard wrote:
Hi,
In case people didn't notice in the somewhat long analysis of the issue in the bug, the workaround is just a simple oneliner:
diff --git a/kernel.spec b/kernel.spec index cb3dec8..29c198a 100644 --- a/kernel.spec +++ b/kernel.spec @@ -183,6 +183,9 @@ Summary: The Linux kernel %define _enable_debug_packages 0 %endif %define debuginfodir /usr/lib/debug +# Needed because we override almost everything involving build-ids +# and debuginfo generation. Currently we rely on the old alldebug setting. +%global _build_id_links alldebug
# kernel PAE is only built on i686 and ARMv7. %ifnarch i686 armv7hl
Of course it would be nice if someone could cleanup the various places that the kernel.spec overrides rpm find-debuginfo.sh and debugedit and provides some requirements that would make this all easier for the kernel build. It looks like the current build does a lot redundant extra work that might be prevented if rpm provided better hooks to do automagically what the kernel spec build requires. One thing rpm wants to introduce in the future (already upstream) is parallel processing of debug files. Which the current kernel.spec seems to prevent because it serializes the processing itself already.
I would be happy to review any feedback on why the kernel.spec has the current hacks and suggestions for improvements to make this smoother.
Cheers,
Mark
I put that into today's rawhide build to (hopefully) unblock people. Looking more closely at the debuginfo is on my TODO list for today/this week.
Thanks, Laura
On Mon, Mar 20, 2017 at 08:46:20AM -0000, Mark Wielaard wrote:
Hi,
In case people didn't notice in the somewhat long analysis of the issue in the bug, the workaround is just a simple oneliner:
diff --git a/kernel.spec b/kernel.spec index cb3dec8..29c198a 100644 --- a/kernel.spec +++ b/kernel.spec @@ -183,6 +183,9 @@ Summary: The Linux kernel %define _enable_debug_packages 0 %endif %define debuginfodir /usr/lib/debug +# Needed because we override almost everything involving build-ids +# and debuginfo generation. Currently we rely on the old alldebug setting. +%global _build_id_links alldebug
# kernel PAE is only built on i686 and ARMv7. %ifnarch i686 armv7hl
Of course it would be nice if someone could cleanup the various places that the kernel.spec overrides rpm find-debuginfo.sh and debugedit and provides some requirements that would make this all easier for the kernel build. It looks like the current build does a lot redundant extra work that might be prevented if rpm provided better hooks to do automagically what the kernel spec build requires. One thing rpm wants to introduce in the future (already upstream) is parallel processing of debug files. Which the current kernel.spec seems to prevent because it serializes the processing itself already.
I would be happy to review any feedback on why the kernel.spec has the current hacks and suggestions for improvements to make this smoother.
Hi Mark,
I can try and help explain some of the hacks. As I have been involved in some of it over the past 11 years. Maybe with some history behind the changes, we can make better decisions based on newer rpm technology?
To start, a bunch of the debuginfo-common hacks that Roland put in there 5-7 years ago was because the kernel was too big and we thought breaking them up would make it easier to download kernel-debuginfo and kernel-debug-debuginfo.
Of course this was back when rpm development was stale, so Roland just choose the rpm macro route to do a lot of this work.
He also may have done some of it to prep for his build-id technology he wanted to put into all the packages too.
Some of the other kernel.spec file hacks (debug files) were because the kernel was ahead of the rpm curve. If rpm has implemented some of the technology now, it would probably make sense to start using it.
I am not aware of some of those hacks being because rpm was broken, more like rpm lacked the feature. Well except for the strict patching stuff, rpm was broken and caused incorrectly patched kernel due to fuzzing.
Hope that helps.
Cheers, Don
On 03/20/2017 01:46 AM, Mark Wielaard wrote:
Hi,
In case people didn't notice in the somewhat long analysis of the issue in the bug, the workaround is just a simple oneliner:
diff --git a/kernel.spec b/kernel.spec index cb3dec8..29c198a 100644 --- a/kernel.spec +++ b/kernel.spec @@ -183,6 +183,9 @@ Summary: The Linux kernel %define _enable_debug_packages 0 %endif %define debuginfodir /usr/lib/debug +# Needed because we override almost everything involving build-ids +# and debuginfo generation. Currently we rely on the old alldebug setting. +%global _build_id_links alldebug
# kernel PAE is only built on i686 and ARMv7. %ifnarch i686 armv7hl
Of course it would be nice if someone could cleanup the various places that the kernel.spec overrides rpm find-debuginfo.sh and debugedit and provides some requirements that would make this all easier for the kernel build. It looks like the current build does a lot redundant extra work that might be prevented if rpm provided better hooks to do automagically what the kernel spec build requires. One thing rpm wants to introduce in the future (already upstream) is parallel processing of debug files. Which the current kernel.spec seems to prevent because it serializes the processing itself already.
I would be happy to review any feedback on why the kernel.spec has the current hacks and suggestions for improvements to make this smoother.
Cheers,
Mark
I spent some time looking at this and I have something that produces debuginfo without overriding find-debuginfo.sh or calling debugedit manually. I have no idea if this debuginfo is useful/correct so this needs more review/testing. Patch is attached if anyone wants to review and there is a scratch build going at https://koji.fedoraproject.org/koji/taskinfo?taskID=18701457.
One area that could really use improvement is the filtering. The kernel filters everything into many different debuginfo packages using the -p filter. Debugging those regexes is an absolute nightmare and my current proposal relies on knowing how find-debuginfo.sh sets the build-id. Do you have any suggestions on how to make the filtering more sustainable?
Thanks, Laura
On 03/31/2017 10:39 AM, Laura Abbott wrote:
On 03/20/2017 01:46 AM, Mark Wielaard wrote:
Hi,
In case people didn't notice in the somewhat long analysis of the issue in the bug, the workaround is just a simple oneliner:
diff --git a/kernel.spec b/kernel.spec index cb3dec8..29c198a 100644 --- a/kernel.spec +++ b/kernel.spec @@ -183,6 +183,9 @@ Summary: The Linux kernel %define _enable_debug_packages 0 %endif %define debuginfodir /usr/lib/debug +# Needed because we override almost everything involving build-ids +# and debuginfo generation. Currently we rely on the old alldebug setting. +%global _build_id_links alldebug
# kernel PAE is only built on i686 and ARMv7. %ifnarch i686 armv7hl
Of course it would be nice if someone could cleanup the various places that the kernel.spec overrides rpm find-debuginfo.sh and debugedit and provides some requirements that would make this all easier for the kernel build. It looks like the current build does a lot redundant extra work that might be prevented if rpm provided better hooks to do automagically what the kernel spec build requires. One thing rpm wants to introduce in the future (already upstream) is parallel processing of debug files. Which the current kernel.spec seems to prevent because it serializes the processing itself already.
I would be happy to review any feedback on why the kernel.spec has the current hacks and suggestions for improvements to make this smoother.
Cheers,
Mark
I spent some time looking at this and I have something that produces debuginfo without overriding find-debuginfo.sh or calling debugedit manually. I have no idea if this debuginfo is useful/correct so this needs more review/testing. Patch is attached if anyone wants to review and there is a scratch build going at https://koji.fedoraproject.org/koji/taskinfo?taskID=18701457.
One area that could really use improvement is the filtering. The kernel filters everything into many different debuginfo packages using the -p filter. Debugging those regexes is an absolute nightmare and my current proposal relies on knowing how find-debuginfo.sh sets the build-id. Do you have any suggestions on how to make the filtering more sustainable?
Thanks, Laura
It was pointed out that the attachment may not have made it through. Here is it inline
---8<---
From 2c1fb54ab0b8925cf11c84740981fb3dd51414a4 Mon Sep 17 00:00:00 2001 From: Laura Abbott labbott@redhat.com Date: Fri, 31 Mar 2017 10:22:16 -0700 Subject: [PATCH] Debuginfo?
--- kbuild-AFTER_LINK.patch | 126 ------------------------------------------------ kernel.spec | 37 +++----------- 2 files changed, 6 insertions(+), 157 deletions(-) delete mode 100644 kbuild-AFTER_LINK.patch
diff --git a/kbuild-AFTER_LINK.patch b/kbuild-AFTER_LINK.patch deleted file mode 100644 index ab738c62..00000000 --- a/kbuild-AFTER_LINK.patch +++ /dev/null @@ -1,126 +0,0 @@ -From 649d991ca7737dd227f2a1ca4f30247daf6a7b4b Mon Sep 17 00:00:00 2001 -From: Roland McGrath roland@redhat.com -Date: Mon, 6 Oct 2008 23:03:03 -0700 -Subject: [PATCH] kbuild: AFTER_LINK - -If the make variable AFTER_LINK is set, it is a command line to run -after each final link. This includes vmlinux itself and vDSO images. - -Bugzilla: N/A -Upstream-status: ?? - -Signed-off-by: Roland McGrath roland@redhat.com ---- - arch/arm64/kernel/vdso/Makefile | 3 ++- - arch/powerpc/kernel/vdso32/Makefile | 3 ++- - arch/powerpc/kernel/vdso64/Makefile | 3 ++- - arch/s390/kernel/vdso32/Makefile | 3 ++- - arch/s390/kernel/vdso64/Makefile | 3 ++- - arch/x86/entry/vdso/Makefile | 5 +++-- - scripts/link-vmlinux.sh | 4 ++++ - 7 files changed, 17 insertions(+), 7 deletions(-) - -diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile -index 62c84f7..f44236a 100644 ---- a/arch/arm64/kernel/vdso/Makefile -+++ b/arch/arm64/kernel/vdso/Makefile -@@ -54,7 +54,8 @@ $(obj-vdso): %.o: %.S FORCE - - # Actual build commands - quiet_cmd_vdsold = VDSOL $@ -- cmd_vdsold = $(CC) $(c_flags) -Wl,-n -Wl,-T $^ -o $@ -+ cmd_vdsold = $(CC) $(c_flags) -Wl,-n -Wl,-T $^ -o $@ \ -+ $(if $(AFTER_LINK),;$(AFTER_LINK)) - quiet_cmd_vdsoas = VDSOA $@ - cmd_vdsoas = $(CC) $(a_flags) -c -o $@ $< - -diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile -index 78a7449..c9592c0 100644 ---- a/arch/powerpc/kernel/vdso32/Makefile -+++ b/arch/powerpc/kernel/vdso32/Makefile -@@ -44,7 +44,8 @@ $(obj-vdso32): %.o: %.S FORCE - - # actual build commands - quiet_cmd_vdso32ld = VDSO32L $@ -- cmd_vdso32ld = $(CROSS32CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) -+ cmd_vdso32ld = $(CROSS32CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) - quiet_cmd_vdso32as = VDSO32A $@ - cmd_vdso32as = $(CROSS32CC) $(a_flags) -c -o $@ $< - -diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile -index 31107bf..96aded3 100644 ---- a/arch/powerpc/kernel/vdso64/Makefile -+++ b/arch/powerpc/kernel/vdso64/Makefile -@@ -33,7 +33,8 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE - - # actual build commands - quiet_cmd_vdso64ld = VDSO64L $@ -- cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) -+ cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) - - # install commands for the unstripped file - quiet_cmd_vdso_install = INSTALL $@ -diff --git a/arch/s390/kernel/vdso32/Makefile b/arch/s390/kernel/vdso32/Makefile -index 6cc9478..94fb536 100644 ---- a/arch/s390/kernel/vdso32/Makefile -+++ b/arch/s390/kernel/vdso32/Makefile -@@ -46,7 +46,8 @@ $(obj-vdso32): %.o: %.S - - # actual build commands - quiet_cmd_vdso32ld = VDSO32L $@ -- cmd_vdso32ld = $(CC) $(c_flags) -Wl,-T $^ -o $@ -+ cmd_vdso32ld = $(CC) $(c_flags) -Wl,-T $^ -o $@ \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) - quiet_cmd_vdso32as = VDSO32A $@ - cmd_vdso32as = $(CC) $(a_flags) -c -o $@ $< - -diff --git a/arch/s390/kernel/vdso64/Makefile b/arch/s390/kernel/vdso64/Makefile -index 2d54c18..a0e3e9d 100644 ---- a/arch/s390/kernel/vdso64/Makefile -+++ b/arch/s390/kernel/vdso64/Makefile -@@ -46,7 +46,8 @@ $(obj-vdso64): %.o: %.S - - # actual build commands - quiet_cmd_vdso64ld = VDSO64L $@ -- cmd_vdso64ld = $(CC) $(c_flags) -Wl,-T $^ -o $@ -+ cmd_vdso64ld = $(CC) $(c_flags) -Wl,-T $^ -o $@ \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) - quiet_cmd_vdso64as = VDSO64A $@ - cmd_vdso64as = $(CC) $(a_flags) -c -o $@ $< - -diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile -index d540966..eeb47b6 100644 ---- a/arch/x86/entry/vdso/Makefile -+++ b/arch/x86/entry/vdso/Makefile -@@ -167,8 +167,9 @@ $(obj)/vdso32.so.dbg: FORCE \ - quiet_cmd_vdso = VDSO $@ - cmd_vdso = $(CC) -nostdlib -o $@ \ - $(VDSO_LDFLAGS) $(VDSO_LDFLAGS_$(filter %.lds,$(^F))) \ -- -Wl,-T,$(filter %.lds,$^) $(filter %.o,$^) && \ -- sh $(srctree)/$(src)/checkundef.sh '$(NM)' '$@' -+ -Wl,-T,$(filter %.lds,$^) $(filter %.o,$^) \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) && \ -+ sh $(srctree)/$(src)/checkundef.sh '$(NM)' '$@' - - VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=both) \ - $(call cc-ldoption, -Wl$(comma)--build-id) -Wl,-Bsymbolic $(LTO_CFLAGS) -diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh -index f742c65..526eee4 100755 ---- a/scripts/link-vmlinux.sh -+++ b/scripts/link-vmlinux.sh -@@ -111,6 +111,10 @@ vmlinux_link() - -lutil -lrt -lpthread - rm -f linux - fi -+ if [ -n "${AFTER_LINK}" ]; then -+ /usr/lib/rpm/debugedit -b ${RPM_BUILD_DIR} -d /usr/src/debug -i ${2} \ -+ > ${2}.id -+ fi - } - - --- -2.7.4 - diff --git a/kernel.spec b/kernel.spec index 89b1a6c0..14a7e3c3 100644 --- a/kernel.spec +++ b/kernel.spec @@ -395,7 +395,8 @@ BuildRequires: pciutils-devel gettext ncurses-devel BuildConflicts: rhbuildsys(DiskFree) < 500Mb %if %{with_debuginfo} BuildRequires: rpm-build, elfutils -%define debuginfo_args --strict-build-id -r +%undefine _include_minidebuginfo +%undefine _find_debuginfo_dwz_opts %endif
%if %{signkernel}%{signmodules} @@ -492,9 +493,6 @@ Source5000: patch-4.%{base_sublevel}-git%{gitrev}.xz
## Patches needed for building this package
-# build tweak for build ID magic, even for -vanilla -Patch001: kbuild-AFTER_LINK.patch - ## compile fixes
# ongoing complaint, full discussion delayed until ksummit/plumbers @@ -711,7 +709,7 @@ This package provides debug information for the perf package. # symlinks because of the trailing nonmatching alternation and # the leading .*, because of find-debuginfo.sh's buggy handling # of matching the pattern against the symlinks file. -%{expand:%%global debuginfo_args %{?debuginfo_args} -p '.*%%{_bindir}/perf(.debug)?|.*%%{_libexecdir}/perf-core/.*|.*%%{_libdir}/traceevent/plugins/.*|XXX' -o perf-debuginfo.list} +%{expand:%%global _find_debuginfo_opts %{?_find_debuginfo_opts} -p '.*%%{_bindir}/perf-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_libexecdir}/perf-core/.*|.*%%{_libdir}/traceevent/plugins/.*|XXX' -o perf-debuginfo.list}
%package -n python-perf Summary: Python bindings for apps which will manipulate perf events @@ -732,7 +730,7 @@ AutoReqProv: no This package provides debug information for the perf python bindings.
# the python_sitearch macro should already be defined from above -%{expand:%%global debuginfo_args %{?debuginfo_args} -p '.*%%{python_sitearch}/perf.so(.debug)?|XXX' -o python-perf-debuginfo.list} +%{expand:%%global _find_debuginfo_opts %{?_find_debuginfo_opts} -p '.*%%{python_sitearch}/perf.so-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|XXX' -o python-perf-debuginfo.list}
%endif # with_perf @@ -787,7 +785,7 @@ This package provides debug information for package kernel-tools. # symlinks because of the trailing nonmatching alternation and # the leading .*, because of find-debuginfo.sh's buggy handling # of matching the pattern against the symlinks file. -%{expand:%%global debuginfo_args %{?debuginfo_args} -p '.*%%{_bindir}/centrino-decode(.debug)?|.*%%{_bindir}/powernow-k8-decode(.debug)?|.*%%{_bindir}/cpupower(.debug)?|.*%%{_libdir}/libcpupower.*|.*%%{_bindir}/turbostat(.debug)?|.*%%{_bindir}/x86_energy_perf_policy(.debug)?|.*%%{_bindir}/tmon(.debug)?|.*%%{_bindir}/lsgpio(.debug)?|.*%%{_bindir}/gpio-hammer(.debug)?|.*%%{_bindir}/gpio-event-mon(.debug)?|.*%%{_bindir}/iio_event_monitor(.debug)?|.*%%{_bindir}/iio_generic_buffer(.debug)?|.*%%{_bindir}/lsiio(.debug)?|XXX' -o kernel-tools-debuginfo.list} +%{expand:%%global _find_debuginfo_opts %{?_find_debuginfo_opts} -p '.*%%{_bindir}/centrino-decode-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/powernow-k8-decode-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/cpupower-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_libdir}/libcpupower.*|.*%%{_bindir}/turbostat-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/x86_energy_perf_policy-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/tmon-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/lsgpio-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/gpio-hammer-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/gpio-event-mon-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/iio_event_monitor-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/iio_generic_buffer-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|.*%%{_bindir}/lsiio-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?|XXX' -o kernel-tools-debuginfo.list}
%endif # with_tools
@@ -807,7 +805,7 @@ AutoReqProv: no\ %description %{?1:%{1}-}debuginfo\ This package provides debug information for package %{name}%{?1:-%{1}}.\ This is required to use SystemTap with %{name}%{?1:-%{1}}-%{KVERREL}.\ -%{expand:%%global debuginfo_args %{?debuginfo_args} -p '/.*/%%{KVERREL}%{?1:[+]%{1}}/.*|/.*%%{KVERREL}%{?1:+%{1}}(.debug)?' -o debuginfo%{?1}.list}\ +%{expand:%%global _find_debuginfo_opts %{?_find_debuginfo_opts} -p '/.*/%%{KVERREL}%{?1:[+]%{1}}/.*|/.*%%{KVERREL}%{?1:+%{1}}-%%{VERSION}-%%{RELEASE}.%%{_arch}(.debug)?' -o debuginfo%{?1}.list}\ %{nil}
# @@ -1288,18 +1286,6 @@ cd .. %define sparse_mflags C=1 %endif
-%if %{with_debuginfo} -# This override tweaks the kernel makefiles so that we run debugedit on an -# object before embedding it. When we later run find-debuginfo.sh, it will -# run debugedit again. The edits it does change the build ID bits embedded -# in the stripped object, but repeating debugedit is a no-op. We do it -# beforehand to get the proper final build ID bits into the embedded image. -# This affects the vDSO images in vmlinux, and the vmlinux image in bzImage. -export AFTER_LINK=\ -'sh -xc "/usr/lib/rpm/debugedit -b $$RPM_BUILD_DIR -d /usr/src/debug \ - -i $@ > $@.id"' -%endif - cp_vmlinux() { eu-strip --remove-comment -o "$2" "$1" @@ -1511,13 +1497,6 @@ BuildKernel() { cp $RPM_BUILD_ROOT/lib/modules/$KernelVer/build/.config $RPM_BUILD_ROOT/lib/modules/$KernelVer/build/include/config/auto.conf
%if %{with_debuginfo} - if test -s vmlinux.id; then - cp vmlinux.id $RPM_BUILD_ROOT/lib/modules/$KernelVer/build/vmlinux.id - else - echo >&2 "*** ERROR *** no vmlinux build ID! ***" - exit 1 - fi - # # save the vmlinux file for kernel debugging into the kernel-debuginfo rpm # @@ -1753,10 +1732,6 @@ popd
%if %{with_debuginfo}
-%define __debug_install_post \ - /usr/lib/rpm/find-debuginfo.sh %{debuginfo_args} %{_builddir}/%{?buildsubdir}\ -%{nil} - %ifnarch noarch %global __debug_package 1 %files -f debugfiles.list debuginfo-common-%{_target_cpu}
kernel@lists.fedoraproject.org