Re: [PATCHv3] Update debuginfo generation

2017-05-23 Thread Laura Abbott 
On 05/23/2017 04:23 AM, Mark Wielaard wrote:
> Hi,
> 
> On Fri, 2017-05-05 at 10:40 -0400, Don Zickus wrote:
>> On Thu, May 04, 2017 at 04:38:25PM -0700, Laura Abbott wrote:
>>>
>>> Once upon a time, the kernel needed a lot of special handling to
>>> generate proper debuginfo as the kernel was ahead in technology. These
>>> days, rpm has improved debuginfo support. The kernel has not kept up
>>> with this and it's forward looking calls are now out of date. Switch to
>>> more standard invocations of debuginfo calls.
>>> ---
>>> v3: Adds the new flag to never touch the buildids. I think I got the
>>> BuildConflicts tag correct?
> 
> Yes, I believe so. Version 4.13.0.1-19 has all the fixes needed.
> 
>> Thanks for the work!  The patch seems reasonable to me.  I will let Mark
>> comment on it too.
> 
> Yes, it looks like a good cleanup. I am glad this gets rid of the
> AFTER_LINK patch which assumed that double debugedit invocation is
> idempotent. Which it isn't anymore now that we want to generate unique
> debug-names and build-ids. We still have to figure out some way to
> enable that for the kernel builds though. I think rpm needs to become a
> little smarter about finding out which files might embed other images
> that might contain build-ids (the vdsos, the compressed kernel modules
> and the compressed kernel image itself for which the kernel.spec does
> contain workaround currently).
> 

Yes, I would like to get unique names going sometime as well.

>>> diff --git a/kernel.spec b/kernel.spec
>>> index 27c4fe13..06fcf3d4 100644
>>> --- a/kernel.spec
>>> +++ b/kernel.spec
>>> @@ -395,7 +395,16 @@ BuildRequires: pciutils-devel gettext ncurses-devel
>>>  BuildConflicts: rhbuildsys(DiskFree) < 500Mb
>>>  %if %{with_debuginfo}
>>>  BuildRequires: rpm-build, elfutils
>>> -%define debuginfo_args --strict-build-id -r
>>> +BuildConflicts: rpm < 4.13.0.1-19
>>> +# Most of these should be enabled after more investigation
>>> +%undefine _include_minidebuginfo
> 
> I think with 4.13.0.1-19 you can drop this undefine. Because it has:
>  - Minisymtab should only be added for executables or shared libraries.
> Or you could first do a version with it undefined and then remove it in
> a later patch if you want to double check.
>
I'd prefer to just keep it off unless we want actual Minisymtab support
for the kernel.
 
> Thanks,
> 
> Mark

I dropped the patch into rawhide so it should start showing up
in builds in the next few days.

Thanks for all the review and feeback!

Laura
___
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org

Re: [PATCHv3] Update debuginfo generation

2017-05-23 Thread Mark Wielaard 
Hi,

On Fri, 2017-05-05 at 10:40 -0400, Don Zickus wrote:
> On Thu, May 04, 2017 at 04:38:25PM -0700, Laura Abbott wrote:
> > 
> > Once upon a time, the kernel needed a lot of special handling to
> > generate proper debuginfo as the kernel was ahead in technology. These
> > days, rpm has improved debuginfo support. The kernel has not kept up
> > with this and it's forward looking calls are now out of date. Switch to
> > more standard invocations of debuginfo calls.
> > ---
> > v3: Adds the new flag to never touch the buildids. I think I got the
> > BuildConflicts tag correct?

Yes, I believe so. Version 4.13.0.1-19 has all the fixes needed.

> Thanks for the work!  The patch seems reasonable to me.  I will let Mark
> comment on it too.

Yes, it looks like a good cleanup. I am glad this gets rid of the
AFTER_LINK patch which assumed that double debugedit invocation is
idempotent. Which it isn't anymore now that we want to generate unique
debug-names and build-ids. We still have to figure out some way to
enable that for the kernel builds though. I think rpm needs to become a
little smarter about finding out which files might embed other images
that might contain build-ids (the vdsos, the compressed kernel modules
and the compressed kernel image itself for which the kernel.spec does
contain workaround currently).

> > diff --git a/kernel.spec b/kernel.spec
> > index 27c4fe13..06fcf3d4 100644
> > --- a/kernel.spec
> > +++ b/kernel.spec
> > @@ -395,7 +395,16 @@ BuildRequires: pciutils-devel gettext ncurses-devel
> >  BuildConflicts: rhbuildsys(DiskFree) < 500Mb
> >  %if %{with_debuginfo}
> >  BuildRequires: rpm-build, elfutils
> > -%define debuginfo_args --strict-build-id -r
> > +BuildConflicts: rpm < 4.13.0.1-19
> > +# Most of these should be enabled after more investigation
> > +%undefine _include_minidebuginfo

I think with 4.13.0.1-19 you can drop this undefine. Because it has:
 - Minisymtab should only be added for executables or shared libraries.
Or you could first do a version with it undefined and then remove it in
a later patch if you want to double check.

Thanks,

Mark
___
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org

Re: [PATCHv3] Update debuginfo generation

2017-05-10 Thread Laura Abbott 
On 05/05/2017 07:40 AM, Don Zickus wrote:
> On Thu, May 04, 2017 at 04:38:25PM -0700, Laura Abbott wrote:
>>
>> Once upon a time, the kernel needed a lot of special handling to
>> generate proper debuginfo as the kernel was ahead in technology. These
>> days, rpm has improved debuginfo support. The kernel has not kept up
>> with this and it's forward looking calls are now out of date. Switch to
>> more standard invocations of debuginfo calls.
>> ---
>> v3: Adds the new flag to never touch the buildids. I think I got the
>> BuildConflicts tag correct?
> 
> 
> Hi Laura,
> 
> Thanks for the work!  The patch seems reasonable to me.  I will let Mark
> comment on it too.  Do you have a scratch build that I could look at?
> 
> Cheers,
> Don
> 

Sorry for the slow response, here is one you can test
https://koji.fedoraproject.org/koji/taskinfo?taskID=19489850

Thanks,
Laura

>> ---
>>  kbuild-AFTER_LINK.patch | 126 
>> 
>>  kernel.spec |  49 ---
>>  2 files changed, 19 insertions(+), 156 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..
>> --- a/kbuild-AFTER_LINK.patch
>> +++ /dev/null
>> @@ -1,126 +0,0 @@
>> -From 649d991ca7737dd227f2a1ca4f30247daf6a7b4b Mon Sep 17 00:00:00 2001
>> -From: Roland McGrath 
>> -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 
>> 
>> - 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
>>

Re: [PATCHv3] Update debuginfo generation

2017-05-05 Thread Don Zickus 
On Thu, May 04, 2017 at 04:38:25PM -0700, Laura Abbott wrote:
> 
> Once upon a time, the kernel needed a lot of special handling to
> generate proper debuginfo as the kernel was ahead in technology. These
> days, rpm has improved debuginfo support. The kernel has not kept up
> with this and it's forward looking calls are now out of date. Switch to
> more standard invocations of debuginfo calls.
> ---
> v3: Adds the new flag to never touch the buildids. I think I got the
> BuildConflicts tag correct?


Hi Laura,

Thanks for the work!  The patch seems reasonable to me.  I will let Mark
comment on it too.  Do you have a scratch build that I could look at?

Cheers,
Don

> ---
>  kbuild-AFTER_LINK.patch | 126 
> 
>  kernel.spec |  49 ---
>  2 files changed, 19 insertions(+), 156 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..
> --- a/kbuild-AFTER_LINK.patch
> +++ /dev/null
> @@ -1,126 +0,0 @@
> -From 649d991ca7737dd227f2a1ca4f30247daf6a7b4b Mon Sep 17 00:00:00 2001
> -From: Roland McGrath 
> -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 
> 
> - 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