diff mbox series

kbuild: add debug package to pacman PKGBUILD

Message ID 20240801132945.47963-1-jose.fernandez@linux.dev (mailing list archive)
State New
Headers show
Series kbuild: add debug package to pacman PKGBUILD | expand

Commit Message

Jose Fernandez Aug. 1, 2024, 1:29 p.m. UTC
Add a new -debug package to the pacman PKGBUILD that will contain the
vmlinux image for debugging purposes. This package depends on the
-headers package and will be installed in /usr/src/debug/${pkgbase}.

The vmlinux image is needed to debug core dumps with tools like crash.

Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
Reviewed-by: Peter Jung <ptr1337@cachyos.org>
---
 scripts/package/PKGBUILD | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Nathan Chancellor Aug. 1, 2024, 6:36 p.m. UTC | #1
Hi Jose,

On Thu, Aug 01, 2024 at 07:29:40AM -0600, Jose Fernandez wrote:
> Add a new -debug package to the pacman PKGBUILD that will contain the
> vmlinux image for debugging purposes. This package depends on the
> -headers package and will be installed in /usr/src/debug/${pkgbase}.
> 
> The vmlinux image is needed to debug core dumps with tools like crash.
> 
> Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> Reviewed-by: Peter Jung <ptr1337@cachyos.org>

This appears to add a non-trivial amount of time to the build when benchmarking
with Arch Linux's configuration (I measure 9% with hyperfine):

Benchmark 1: pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too")
  Time (mean ± σ):     579.541 s ±  0.585 s    [User: 22156.731 s, System: 3681.698 s]
  Range (min … max):   578.894 s … 580.033 s    3 runs

Benchmark 2: pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
  Time (mean ± σ):     633.419 s ±  0.972 s    [User: 22247.886 s, System: 3673.879 s]
  Range (min … max):   632.302 s … 634.070 s    3 runs

Summary
  pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too") ran
    1.09 ± 0.00 times faster than pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")

It would be nice to add some option to avoid building this package for
developers who may not want it (I know I personally would not want it
with that penalty because I do a lot of bisects) or maybe adding a
target to build this package with the rest like 'pacman-pkg-with-dbg' or
something? Also, couldn't vmlinux be obtained from vmlinuz that already
exists in the main package via scripts/extract-vmlinux?

Cheers,
Nathan

> ---
>  scripts/package/PKGBUILD | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> index 663ce300dd06..beda3db21863 100644
> --- a/scripts/package/PKGBUILD
> +++ b/scripts/package/PKGBUILD
> @@ -6,6 +6,7 @@ pkgbase=${PACMAN_PKGBASE:-linux-upstream}
>  pkgname=("${pkgbase}" "${pkgbase}-api-headers")
>  if grep -q CONFIG_MODULES=y include/config/auto.conf; then
>  	pkgname+=("${pkgbase}-headers")
> +	pkgname+=("${pkgbase}-debug")
>  fi
>  pkgver="${KERNELRELEASE//-/_}"
>  # The PKGBUILD is evaluated multiple times.
> @@ -89,6 +90,15 @@ _package-headers() {
>  	ln -sr "${builddir}" "${pkgdir}/usr/src/${pkgbase}"
>  }
>  
> +_package-debug(){
> +    pkgdesc="Non-stripped vmlinux file for the ${pkgdesc} kernel"
> +    depends=(${pkgbase}-headers)
> +
> +    cd "${objtree}"
> +    mkdir -p "$pkgdir/usr/src/debug/${pkgbase}"
> +    install -Dt "$pkgdir/usr/src/debug/${pkgbase}" -m644 vmlinux
> +}
> +
>  _package-api-headers() {
>  	pkgdesc="Kernel headers sanitized for use in userspace"
>  	provides=(linux-api-headers)
> -- 
> 2.46.0
>
Thomas Weißschuh Aug. 1, 2024, 6:53 p.m. UTC | #2
On 2024-08-01 11:36:37+0000, Nathan Chancellor wrote:
> Hi Jose,
> 
> On Thu, Aug 01, 2024 at 07:29:40AM -0600, Jose Fernandez wrote:
> > Add a new -debug package to the pacman PKGBUILD that will contain the
> > vmlinux image for debugging purposes. This package depends on the
> > -headers package and will be installed in /usr/src/debug/${pkgbase}.
> > 
> > The vmlinux image is needed to debug core dumps with tools like crash.
> > 
> > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> 
> This appears to add a non-trivial amount of time to the build when benchmarking
> with Arch Linux's configuration (I measure 9% with hyperfine):

As nothing more is compiled, I guess this is just the additional
packaging.

> Benchmark 1: pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too")
>   Time (mean ± σ):     579.541 s ±  0.585 s    [User: 22156.731 s, System: 3681.698 s]
>   Range (min … max):   578.894 s … 580.033 s    3 runs
> 
> Benchmark 2: pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
>   Time (mean ± σ):     633.419 s ±  0.972 s    [User: 22247.886 s, System: 3673.879 s]
>   Range (min … max):   632.302 s … 634.070 s    3 runs
> 
> Summary
>   pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too") ran
>     1.09 ± 0.00 times faster than pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> 
> It would be nice to add some option to avoid building this package for
> developers who may not want it (I know I personally would not want it
> with that penalty because I do a lot of bisects) or maybe adding a
> target to build this package with the rest like 'pacman-pkg-with-dbg' or
> something? Also, couldn't vmlinux be obtained from vmlinuz that already
> exists in the main package via scripts/extract-vmlinux?

Jose:

In the vanilla PKGBUILD vmlinux is part of the linux-headers package:
linux-headers /usr/lib/modules/6.10.2-arch1-1/build/vmlinux

Given that you already gate the new -debug package on CONFIG_MODULES,
why not add the file to that package?

Then we could still discuss if it makes sense to gate vmlinux on
CONFIG_MODULES or if -headers should be enabled unconditionally again.
Or we wait for somebody to show up and ask for it.

> Cheers,
> Nathan
> 
> > ---
> >  scripts/package/PKGBUILD | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > index 663ce300dd06..beda3db21863 100644
> > --- a/scripts/package/PKGBUILD
> > +++ b/scripts/package/PKGBUILD
> > @@ -6,6 +6,7 @@ pkgbase=${PACMAN_PKGBASE:-linux-upstream}
> >  pkgname=("${pkgbase}" "${pkgbase}-api-headers")
> >  if grep -q CONFIG_MODULES=y include/config/auto.conf; then
> >  	pkgname+=("${pkgbase}-headers")
> > +	pkgname+=("${pkgbase}-debug")
> >  fi
> >  pkgver="${KERNELRELEASE//-/_}"
> >  # The PKGBUILD is evaluated multiple times.
> > @@ -89,6 +90,15 @@ _package-headers() {
> >  	ln -sr "${builddir}" "${pkgdir}/usr/src/${pkgbase}"
> >  }
> >  
> > +_package-debug(){
> > +    pkgdesc="Non-stripped vmlinux file for the ${pkgdesc} kernel"
> > +    depends=(${pkgbase}-headers)
> > +
> > +    cd "${objtree}"
> > +    mkdir -p "$pkgdir/usr/src/debug/${pkgbase}"
> > +    install -Dt "$pkgdir/usr/src/debug/${pkgbase}" -m644 vmlinux
> > +}
> > +
> >  _package-api-headers() {
> >  	pkgdesc="Kernel headers sanitized for use in userspace"
> >  	provides=(linux-api-headers)
> > -- 
> > 2.46.0
> >
Nathan Chancellor Aug. 1, 2024, 7:20 p.m. UTC | #3
On Thu, Aug 01, 2024 at 08:53:54PM +0200, Thomas Weißschuh wrote:
> On 2024-08-01 11:36:37+0000, Nathan Chancellor wrote:
> > Hi Jose,
> > 
> > On Thu, Aug 01, 2024 at 07:29:40AM -0600, Jose Fernandez wrote:
> > > Add a new -debug package to the pacman PKGBUILD that will contain the
> > > vmlinux image for debugging purposes. This package depends on the
> > > -headers package and will be installed in /usr/src/debug/${pkgbase}.
> > > 
> > > The vmlinux image is needed to debug core dumps with tools like crash.
> > > 
> > > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> > 
> > This appears to add a non-trivial amount of time to the build when benchmarking
> > with Arch Linux's configuration (I measure 9% with hyperfine):
> 
> As nothing more is compiled, I guess this is just the additional
> packaging.
> 
> > Benchmark 1: pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too")
> >   Time (mean ± σ):     579.541 s ±  0.585 s    [User: 22156.731 s, System: 3681.698 s]
> >   Range (min … max):   578.894 s … 580.033 s    3 runs
> > 
> > Benchmark 2: pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> >   Time (mean ± σ):     633.419 s ±  0.972 s    [User: 22247.886 s, System: 3673.879 s]
> >   Range (min … max):   632.302 s … 634.070 s    3 runs
> > 
> > Summary
> >   pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too") ran
> >     1.09 ± 0.00 times faster than pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> > 
> > It would be nice to add some option to avoid building this package for
> > developers who may not want it (I know I personally would not want it
> > with that penalty because I do a lot of bisects) or maybe adding a
> > target to build this package with the rest like 'pacman-pkg-with-dbg' or
> > something? Also, couldn't vmlinux be obtained from vmlinuz that already
> > exists in the main package via scripts/extract-vmlinux?
> 
> Jose:
> 
> In the vanilla PKGBUILD vmlinux is part of the linux-headers package:
> linux-headers /usr/lib/modules/6.10.2-arch1-1/build/vmlinux
> 
> Given that you already gate the new -debug package on CONFIG_MODULES,
> why not add the file to that package?
> 
> Then we could still discuss if it makes sense to gate vmlinux on
> CONFIG_MODULES or if -headers should be enabled unconditionally again.

I think having -headers be enabled/built by default is probably okay
since a regular user/builder might have external modules that need to be
rebuilt against the new kernel. However, I (and likely many other
upstream developers, however many use Arch) never build external modules
against my kernels, so it would be nice to have some way to opt out of
this penalty. I suspect it is just compressing down such a massive
vmlinux that causes this? I have not had access to a build log with my
testing, so unsure.

> Or we wait for somebody to show up and ask for it.

I won't insist though so if we want to wait for someone else, that's
fine with me too.

> > Cheers,
> > Nathan
> > 
> > > ---
> > >  scripts/package/PKGBUILD | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > > index 663ce300dd06..beda3db21863 100644
> > > --- a/scripts/package/PKGBUILD
> > > +++ b/scripts/package/PKGBUILD
> > > @@ -6,6 +6,7 @@ pkgbase=${PACMAN_PKGBASE:-linux-upstream}
> > >  pkgname=("${pkgbase}" "${pkgbase}-api-headers")
> > >  if grep -q CONFIG_MODULES=y include/config/auto.conf; then
> > >  	pkgname+=("${pkgbase}-headers")
> > > +	pkgname+=("${pkgbase}-debug")
> > >  fi
> > >  pkgver="${KERNELRELEASE//-/_}"
> > >  # The PKGBUILD is evaluated multiple times.
> > > @@ -89,6 +90,15 @@ _package-headers() {
> > >  	ln -sr "${builddir}" "${pkgdir}/usr/src/${pkgbase}"
> > >  }
> > >  
> > > +_package-debug(){
> > > +    pkgdesc="Non-stripped vmlinux file for the ${pkgdesc} kernel"
> > > +    depends=(${pkgbase}-headers)
> > > +
> > > +    cd "${objtree}"
> > > +    mkdir -p "$pkgdir/usr/src/debug/${pkgbase}"
> > > +    install -Dt "$pkgdir/usr/src/debug/${pkgbase}" -m644 vmlinux
> > > +}
> > > +
> > >  _package-api-headers() {
> > >  	pkgdesc="Kernel headers sanitized for use in userspace"
> > >  	provides=(linux-api-headers)
> > > -- 
> > > 2.46.0
> > >
Jose Fernandez Aug. 2, 2024, 4:27 a.m. UTC | #4
On 24/08/01 12:20PM, Nathan Chancellor wrote:
> On Thu, Aug 01, 2024 at 08:53:54PM +0200, Thomas Weißschuh wrote:
> > On 2024-08-01 11:36:37+0000, Nathan Chancellor wrote:
> > > Hi Jose,
> > > 
> > > On Thu, Aug 01, 2024 at 07:29:40AM -0600, Jose Fernandez wrote:
> > > > Add a new -debug package to the pacman PKGBUILD that will contain the
> > > > vmlinux image for debugging purposes. This package depends on the
> > > > -headers package and will be installed in /usr/src/debug/${pkgbase}.
> > > > 
> > > > The vmlinux image is needed to debug core dumps with tools like crash.
> > > > 
> > > > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > > > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> > > 
> > > This appears to add a non-trivial amount of time to the build when benchmarking
> > > with Arch Linux's configuration (I measure 9% with hyperfine):
> > 
> > As nothing more is compiled, I guess this is just the additional
> > packaging.
> > 
> > > Benchmark 1: pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too")
> > >   Time (mean ± σ):     579.541 s ±  0.585 s    [User: 22156.731 s, System: 3681.698 s]
> > >   Range (min … max):   578.894 s … 580.033 s    3 runs
> > > 
> > > Benchmark 2: pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> > >   Time (mean ± σ):     633.419 s ±  0.972 s    [User: 22247.886 s, System: 3673.879 s]
> > >   Range (min … max):   632.302 s … 634.070 s    3 runs
> > > 
> > > Summary
> > >   pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too") ran
> > >     1.09 ± 0.00 times faster than pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")


Hi Nathan,
Thanks for taking the time to benchmark the change. I did notice that the build 
time for the -debug package felt a bit longer than I expected. I attributed it
to having to package a large file.

> > > It would be nice to add some option to avoid building this package for
> > > developers who may not want it (I know I personally would not want it
> > > with that penalty because I do a lot of bisects) or maybe adding a
> > > target to build this package with the rest like 'pacman-pkg-with-dbg' or
> > > something? 

My original idea was to add a make config to enable the -debug package, ie:
`make pacman-pkg DEBUG=1`. Your suggestion of a separate target is also a good
idea. I don't have strong opinion on this, so I'm open to what you and Thomas
prefer.

> > > Also, couldn't vmlinux be obtained from vmlinuz that already
> > > exists in the main package via scripts/extract-vmlinux?

I didn't know you could do that. However, I suspect that more people will 
encounter this issue and will want the vmlinux file available without knowing
you can extract it from vmlinuz. I think it's better to have it available 
explicitly in a package.

> > 
> > Jose:
> > 
> > In the vanilla PKGBUILD vmlinux is part of the linux-headers package:
> > linux-headers /usr/lib/modules/6.10.2-arch1-1/build/vmlinux
> > 
> > Given that you already gate the new -debug package on CONFIG_MODULES,
> > why not add the file to that package?

Hi Thomas,
My only concern with that approach is that it won't address Nathan's concern
about increasing the build time. I think an opt-in -debug package is the best
solution to address this.

Additionally, Peter from CachyOS (cced) shared this merge request with me:
https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/merge_requests/3

It looks like the vanilla PKGBUILD will start including a debug package as well.
I think it may be good to align with the pattern they will be using.

> > 
> > Then we could still discuss if it makes sense to gate vmlinux on
> > CONFIG_MODULES or if -headers should be enabled unconditionally again.
> 
> I think having -headers be enabled/built by default is probably okay
> since a regular user/builder might have external modules that need to be
> rebuilt against the new kernel. However, I (and likely many other
> upstream developers, however many use Arch) never build external modules
> against my kernels, so it would be nice to have some way to opt out of
> this penalty. I suspect it is just compressing down such a massive
> vmlinux that causes this? I have not had access to a build log with my
> testing, so unsure.
> 
> > Or we wait for somebody to show up and ask for it.
> 
> I won't insist though so if we want to wait for someone else, that's
> fine with me too.

I will lean on your preference for how to proceed with this. I'm happy to make
the changes you think are best. To me, it appears that the next iteration to 
satisfy everyone is to make the -debug package opt-in via a new make target or
a make config option.

Thanks,
Jose

> 
> > > Cheers,
> > > Nathan
> > > 
> > > > ---
> > > >  scripts/package/PKGBUILD | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> > > > 
> > > > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > > > index 663ce300dd06..beda3db21863 100644
> > > > --- a/scripts/package/PKGBUILD
> > > > +++ b/scripts/package/PKGBUILD
> > > > @@ -6,6 +6,7 @@ pkgbase=${PACMAN_PKGBASE:-linux-upstream}
> > > >  pkgname=("${pkgbase}" "${pkgbase}-api-headers")
> > > >  if grep -q CONFIG_MODULES=y include/config/auto.conf; then
> > > >  	pkgname+=("${pkgbase}-headers")
> > > > +	pkgname+=("${pkgbase}-debug")
> > > >  fi
> > > >  pkgver="${KERNELRELEASE//-/_}"
> > > >  # The PKGBUILD is evaluated multiple times.
> > > > @@ -89,6 +90,15 @@ _package-headers() {
> > > >  	ln -sr "${builddir}" "${pkgdir}/usr/src/${pkgbase}"
> > > >  }
> > > >  
> > > > +_package-debug(){
> > > > +    pkgdesc="Non-stripped vmlinux file for the ${pkgdesc} kernel"
> > > > +    depends=(${pkgbase}-headers)
> > > > +
> > > > +    cd "${objtree}"
> > > > +    mkdir -p "$pkgdir/usr/src/debug/${pkgbase}"
> > > > +    install -Dt "$pkgdir/usr/src/debug/${pkgbase}" -m644 vmlinux
> > > > +}
> > > > +
> > > >  _package-api-headers() {
> > > >  	pkgdesc="Kernel headers sanitized for use in userspace"
> > > >  	provides=(linux-api-headers)
> > > > -- 
> > > > 2.46.0
> > > >
Thomas Weißschuh Aug. 2, 2024, 3:40 p.m. UTC | #5
On 2024-08-01 22:27:45+0000, Jose Fernandez wrote:
> On 24/08/01 12:20PM, Nathan Chancellor wrote:
> > On Thu, Aug 01, 2024 at 08:53:54PM +0200, Thomas Weißschuh wrote:
> > > On 2024-08-01 11:36:37+0000, Nathan Chancellor wrote:
> > > > On Thu, Aug 01, 2024 at 07:29:40AM -0600, Jose Fernandez wrote:
> > > > > Add a new -debug package to the pacman PKGBUILD that will contain the
> > > > > vmlinux image for debugging purposes. This package depends on the
> > > > > -headers package and will be installed in /usr/src/debug/${pkgbase}.
> > > > > 
> > > > > The vmlinux image is needed to debug core dumps with tools like crash.
> > > > > 
> > > > > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > > > > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> > > > 
> > > > This appears to add a non-trivial amount of time to the build when benchmarking
> > > > with Arch Linux's configuration (I measure 9% with hyperfine):
> > > 
> > > As nothing more is compiled, I guess this is just the additional
> > > packaging.
> > > 
> > > > Benchmark 1: pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too")
> > > >   Time (mean ± σ):     579.541 s ±  0.585 s    [User: 22156.731 s, System: 3681.698 s]
> > > >   Range (min … max):   578.894 s … 580.033 s    3 runs
> > > > 
> > > > Benchmark 2: pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> > > >   Time (mean ± σ):     633.419 s ±  0.972 s    [User: 22247.886 s, System: 3673.879 s]
> > > >   Range (min … max):   632.302 s … 634.070 s    3 runs
> > > > 
> > > > Summary
> > > >   pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too") ran
> > > >     1.09 ± 0.00 times faster than pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> 
> 
> Hi Nathan,
> Thanks for taking the time to benchmark the change. I did notice that the build 
> time for the -debug package felt a bit longer than I expected. I attributed it
> to having to package a large file.
> 
> > > > It would be nice to add some option to avoid building this package for
> > > > developers who may not want it (I know I personally would not want it
> > > > with that penalty because I do a lot of bisects) or maybe adding a
> > > > target to build this package with the rest like 'pacman-pkg-with-dbg' or
> > > > something? 
> 
> My original idea was to add a make config to enable the -debug package, ie:
> `make pacman-pkg DEBUG=1`. Your suggestion of a separate target is also a good
> idea. I don't have strong opinion on this, so I'm open to what you and Thomas
> prefer.

I'm also in favor of a variable.
The new target would be implemented through a variable anyways.
Instead of a specific variable it should be more generic.
The -api-headers package is most likely used even less than the debug one.

PACMAN_EXTRAPACKAGES="headers api-headers debug"
(Assuming the main kernel package will always be built)

> > > > Also, couldn't vmlinux be obtained from vmlinuz that already
> > > > exists in the main package via scripts/extract-vmlinux?
> 
> I didn't know you could do that. However, I suspect that more people will 
> encounter this issue and will want the vmlinux file available without knowing
> you can extract it from vmlinuz. I think it's better to have it available 
> explicitly in a package.

Agreed.

> > > 
> > > Jose:
> > > 
> > > In the vanilla PKGBUILD vmlinux is part of the linux-headers package:
> > > linux-headers /usr/lib/modules/6.10.2-arch1-1/build/vmlinux
> > > 
> > > Given that you already gate the new -debug package on CONFIG_MODULES,
> > > why not add the file to that package?
> 
> Hi Thomas,
> My only concern with that approach is that it won't address Nathan's concern
> about increasing the build time. I think an opt-in -debug package is the best
> solution to address this.
> 
> Additionally, Peter from CachyOS (cced) shared this merge request with me:
> https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/merge_requests/3
> 
> It looks like the vanilla PKGBUILD will start including a debug package as well.
> I think it may be good to align with the pattern they will be using.

This looks very good. But I it seems it will be some time pacman 7.0
will be available (especially in the derivate distros)
Also even with this the custom configuration mechanism would still be
used.

> > > 
> > > Then we could still discuss if it makes sense to gate vmlinux on
> > > CONFIG_MODULES or if -headers should be enabled unconditionally again.
> > 
> > I think having -headers be enabled/built by default is probably okay
> > since a regular user/builder might have external modules that need to be
> > rebuilt against the new kernel. However, I (and likely many other
> > upstream developers, however many use Arch) never build external modules
> > against my kernels, so it would be nice to have some way to opt out of
> > this penalty. I suspect it is just compressing down such a massive
> > vmlinux that causes this? I have not had access to a build log with my
> > testing, so unsure.
> > 
> > > Or we wait for somebody to show up and ask for it.
> > 
> > I won't insist though so if we want to wait for someone else, that's
> > fine with me too.
> 
> I will lean on your preference for how to proceed with this. I'm happy to make
> the changes you think are best. To me, it appears that the next iteration to 
> satisfy everyone is to make the -debug package opt-in via a new make target or
> a make config option.

Ack.
Masahiro Yamada Aug. 3, 2024, 2:40 p.m. UTC | #6
On Fri, Aug 2, 2024 at 3:54 AM Thomas Weißschuh <linux@weissschuh.net> wrote:
>
> On 2024-08-01 11:36:37+0000, Nathan Chancellor wrote:
> > Hi Jose,
> >
> > On Thu, Aug 01, 2024 at 07:29:40AM -0600, Jose Fernandez wrote:
> > > Add a new -debug package to the pacman PKGBUILD that will contain the
> > > vmlinux image for debugging purposes. This package depends on the
> > > -headers package and will be installed in /usr/src/debug/${pkgbase}.
> > >
> > > The vmlinux image is needed to debug core dumps with tools like crash.
> > >
> > > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> >
> > This appears to add a non-trivial amount of time to the build when benchmarking
> > with Arch Linux's configuration (I measure 9% with hyperfine):
>
> As nothing more is compiled, I guess this is just the additional
> packaging.
>
> > Benchmark 1: pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too")
> >   Time (mean ± σ):     579.541 s ±  0.585 s    [User: 22156.731 s, System: 3681.698 s]
> >   Range (min … max):   578.894 s … 580.033 s    3 runs
> >
> > Benchmark 2: pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> >   Time (mean ± σ):     633.419 s ±  0.972 s    [User: 22247.886 s, System: 3673.879 s]
> >   Range (min … max):   632.302 s … 634.070 s    3 runs
> >
> > Summary
> >   pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too") ran
> >     1.09 ± 0.00 times faster than pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> >
> > It would be nice to add some option to avoid building this package for
> > developers who may not want it (I know I personally would not want it
> > with that penalty because I do a lot of bisects) or maybe adding a
> > target to build this package with the rest like 'pacman-pkg-with-dbg' or
> > something? Also, couldn't vmlinux be obtained from vmlinuz that already
> > exists in the main package via scripts/extract-vmlinux?
>
> Jose:
>
> In the vanilla PKGBUILD vmlinux is part of the linux-headers package:
> linux-headers /usr/lib/modules/6.10.2-arch1-1/build/vmlinux
>
> Given that you already gate the new -debug package on CONFIG_MODULES,
> why not add the file to that package?


I do not know why CONFIG_MODULES controls the debug package.

If this is really for debugging purposes, CONFIG_DEBUG_INFO is more suitable.
Without CONFIG_DEBUG_INFO, vmlinux contains only limited amount of
information.


scripts/package/mkdebian use CONFIG_DEBUG_INFO to enable
the debug package.
Jose Fernandez Aug. 3, 2024, 4:41 p.m. UTC | #7
On 24/08/02 05:40PM, Thomas Weißschuh wrote:
> On 2024-08-01 22:27:45+0000, Jose Fernandez wrote:
> > On 24/08/01 12:20PM, Nathan Chancellor wrote:
> > > On Thu, Aug 01, 2024 at 08:53:54PM +0200, Thomas Weißschuh wrote:
> > > > On 2024-08-01 11:36:37+0000, Nathan Chancellor wrote:
> > > > > On Thu, Aug 01, 2024 at 07:29:40AM -0600, Jose Fernandez wrote:
> > > > > > Add a new -debug package to the pacman PKGBUILD that will contain the
> > > > > > vmlinux image for debugging purposes. This package depends on the
> > > > > > -headers package and will be installed in /usr/src/debug/${pkgbase}.
> > > > > > 
> > > > > > The vmlinux image is needed to debug core dumps with tools like crash.
> > > > > > 
> > > > > > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > > > > > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> > > > > 
> > > > > This appears to add a non-trivial amount of time to the build when benchmarking
> > > > > with Arch Linux's configuration (I measure 9% with hyperfine):
> > > > 
> > > > As nothing more is compiled, I guess this is just the additional
> > > > packaging.
> > > > 
> > > > > Benchmark 1: pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too")
> > > > >   Time (mean ± σ):     579.541 s ±  0.585 s    [User: 22156.731 s, System: 3681.698 s]
> > > > >   Range (min … max):   578.894 s … 580.033 s    3 runs
> > > > > 
> > > > > Benchmark 2: pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> > > > >   Time (mean ± σ):     633.419 s ±  0.972 s    [User: 22247.886 s, System: 3673.879 s]
> > > > >   Range (min … max):   632.302 s … 634.070 s    3 runs
> > > > > 
> > > > > Summary
> > > > >   pacman-pkg @ 21b136cc63d2 ("minmax: fix up min3() and max3() too") ran
> > > > >     1.09 ± 0.00 times faster than pacman-pkg @ c5af4db0563b ("kbuild: add debug package to pacman PKGBUILD")
> > 
> > 
> > Hi Nathan,
> > Thanks for taking the time to benchmark the change. I did notice that the build 
> > time for the -debug package felt a bit longer than I expected. I attributed it
> > to having to package a large file.
> > 
> > > > > It would be nice to add some option to avoid building this package for
> > > > > developers who may not want it (I know I personally would not want it
> > > > > with that penalty because I do a lot of bisects) or maybe adding a
> > > > > target to build this package with the rest like 'pacman-pkg-with-dbg' or
> > > > > something? 
> > 
> > My original idea was to add a make config to enable the -debug package, ie:
> > `make pacman-pkg DEBUG=1`. Your suggestion of a separate target is also a good
> > idea. I don't have strong opinion on this, so I'm open to what you and Thomas
> > prefer.
> 
> I'm also in favor of a variable.
> The new target would be implemented through a variable anyways.
> Instead of a specific variable it should be more generic.
> The -api-headers package is most likely used even less than the debug one.
> 
> PACMAN_EXTRAPACKAGES="headers api-headers debug"
> (Assuming the main kernel package will always be built)

Thomas,
I will work a new patch that adds a PACMAN_EXTRAPACKAGES variable to 
determine which extra packages to build. The main kernel package will always
be built, and we will stop building -headers and -api-headers by default.

To confirm, this also means I should remove the CONFIG_MODULES check from the
PKGBUILD, correct?

> 
> > > > > Also, couldn't vmlinux be obtained from vmlinuz that already
> > > > > exists in the main package via scripts/extract-vmlinux?
> > 
> > I didn't know you could do that. However, I suspect that more people will 
> > encounter this issue and will want the vmlinux file available without knowing
> > you can extract it from vmlinuz. I think it's better to have it available 
> > explicitly in a package.
> 
> Agreed.
> 
> > > > 
> > > > Jose:
> > > > 
> > > > In the vanilla PKGBUILD vmlinux is part of the linux-headers package:
> > > > linux-headers /usr/lib/modules/6.10.2-arch1-1/build/vmlinux
> > > > 
> > > > Given that you already gate the new -debug package on CONFIG_MODULES,
> > > > why not add the file to that package?
> > 
> > Hi Thomas,
> > My only concern with that approach is that it won't address Nathan's concern
> > about increasing the build time. I think an opt-in -debug package is the best
> > solution to address this.
> > 
> > Additionally, Peter from CachyOS (cced) shared this merge request with me:
> > https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/merge_requests/3
> > 
> > It looks like the vanilla PKGBUILD will start including a debug package as well.
> > I think it may be good to align with the pattern they will be using.
> 
> This looks very good. But I it seems it will be some time pacman 7.0
> will be available (especially in the derivate distros)
> Also even with this the custom configuration mechanism would still be
> used.
> 
> > > > 
> > > > Then we could still discuss if it makes sense to gate vmlinux on
> > > > CONFIG_MODULES or if -headers should be enabled unconditionally again.
> > > 
> > > I think having -headers be enabled/built by default is probably okay
> > > since a regular user/builder might have external modules that need to be
> > > rebuilt against the new kernel. However, I (and likely many other
> > > upstream developers, however many use Arch) never build external modules
> > > against my kernels, so it would be nice to have some way to opt out of
> > > this penalty. I suspect it is just compressing down such a massive
> > > vmlinux that causes this? I have not had access to a build log with my
> > > testing, so unsure.
> > > 
> > > > Or we wait for somebody to show up and ask for it.
> > > 
> > > I won't insist though so if we want to wait for someone else, that's
> > > fine with me too.
> > 
> > I will lean on your preference for how to proceed with this. I'm happy to make
> > the changes you think are best. To me, it appears that the next iteration to 
> > satisfy everyone is to make the -debug package opt-in via a new make target or
> > a make config option.
> 
> Ack.
Christian Heusel Aug. 6, 2024, 2:28 p.m. UTC | #8
On 24/08/01 08:53PM, Thomas Weißschuh wrote:
> On 2024-08-01 11:36:37+0000, Nathan Chancellor wrote:
> > Hi Jose,
> > 
> > On Thu, Aug 01, 2024 at 07:29:40AM -0600, Jose Fernandez wrote:
> > > Add a new -debug package to the pacman PKGBUILD that will contain the
> > > vmlinux image for debugging purposes. This package depends on the
> > > -headers package and will be installed in /usr/src/debug/${pkgbase}.
> > > 
> > > The vmlinux image is needed to debug core dumps with tools like crash.
> > > 
> > > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> > 
> > This appears to add a non-trivial amount of time to the build when benchmarking
> > with Arch Linux's configuration (I measure 9% with hyperfine):
> 
> As nothing more is compiled, I guess this is just the additional
> packaging.

I would expect that the overhead stems from the extra compression that
needs to be done for the resulting package and depending on what you
have set in your /etc/makepkg.conf this can add significant overhead.
For packages that are not planned to be distributed it might make sense
to completely disable compression[0] by setting PKGEXT. The referenced
forum discussion also explains that this can be hard-disabled in the
PKGBUILD itself, but I would prefer if we could respect the user config
in that regard (especially for people who would like to save some space
on their system).

Additionally the default makepkg configuration is ZSTD compression with
a few parameters that are pretty time consuming (as they are meant to be
used by packagers), which will most likely be reverted again in favour
of a more widely applicable set of defaults with the next pacman
package release. See [2] for the related discussion on the Arch Linux
bugtracker.

The rest of the changes looks fine to me, adding in some configuration
options on which package is actually built via the other patch also
sounds good.

Reviewed-by: Christian Heusel <christian@heusel.eu>

Cheers,
Christian

[0]: https://bbs.archlinux.org/viewtopic.php?id=127894
[1]: https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/issues/23
diff mbox series

Patch

diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
index 663ce300dd06..beda3db21863 100644
--- a/scripts/package/PKGBUILD
+++ b/scripts/package/PKGBUILD
@@ -6,6 +6,7 @@  pkgbase=${PACMAN_PKGBASE:-linux-upstream}
 pkgname=("${pkgbase}" "${pkgbase}-api-headers")
 if grep -q CONFIG_MODULES=y include/config/auto.conf; then
 	pkgname+=("${pkgbase}-headers")
+	pkgname+=("${pkgbase}-debug")
 fi
 pkgver="${KERNELRELEASE//-/_}"
 # The PKGBUILD is evaluated multiple times.
@@ -89,6 +90,15 @@  _package-headers() {
 	ln -sr "${builddir}" "${pkgdir}/usr/src/${pkgbase}"
 }
 
+_package-debug(){
+    pkgdesc="Non-stripped vmlinux file for the ${pkgdesc} kernel"
+    depends=(${pkgbase}-headers)
+
+    cd "${objtree}"
+    mkdir -p "$pkgdir/usr/src/debug/${pkgbase}"
+    install -Dt "$pkgdir/usr/src/debug/${pkgbase}" -m644 vmlinux
+}
+
 _package-api-headers() {
 	pkgdesc="Kernel headers sanitized for use in userspace"
 	provides=(linux-api-headers)