| Message ID | 20211221190809.89882-3-saul.wold@windriver.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enable package.bbclass to process kernel packages | expand |
On Tue, 2021-12-21 at 11:08 -0800, Saul Wold wrote: > Stop ignoring or skipping the kernel and kernel modules code in the > split debug and striping functions, this will allow create_spdx to > process the kernel and modules. > > Signed-off-by: Saul Wold <saul.wold@windriver.com> > --- > meta/classes/package.bbclass | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass > index 84eafbd529..4b7fe4f1e1 100644 > --- a/meta/classes/package.bbclass > +++ b/meta/classes/package.bbclass > @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir, > dvar = d.getVar('PKGD') > objcopy = d.getVar("OBJCOPY") > > - # We ignore kernel modules, we don't generate debug info files. > - if file.find("/lib/modules/") != -1 and file.endswith(".ko"): > - return (file, sources) > - > newmode = None > if not os.access(file, os.W_OK) or os.access(file, os.R_OK): > origmode = os.stat(file)[stat.ST_MODE] > @@ -1147,7 +1143,7 @@ python split_and_strip_files () { > > if file.endswith(".ko") and file.find("/lib/modules/") != -1: > kernmods.append(file) > - continue > + > if oe.package.is_static_lib(file): > staticlibs.append(file) > continue > @@ -1165,7 +1161,7 @@ python split_and_strip_files () { > continue > # Check its an executable > if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \ > - or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)): > + or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f): > > if cpath.islink(file): > checkelflinks[file] = ltarget edgerouter: https://autobuilder.yoctoproject.org/typhoon/#/builders/62/builds/4513 https://autobuilder.yoctoproject.org/typhoon/#/builders/111/builds/2507/steps/11/logs/stdio qemux86 musl cryptodev: https://autobuilder.yoctoproject.org/typhoon/#/builders/64/builds/4512/steps/11/logs/stdio qemux86-64 musl cryptodev: https://autobuilder.yoctoproject.org/typhoon/#/builders/45/builds/4526 qemux86 cryptodev: https://autobuilder.yoctoproject.org/typhoon/#/builders/52/builds/4476/steps/11/logs/stdio selftest failure in linux-yocto: https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/2981/steps/14/logs/stdio (file truncated makes it sound like a race?) stap kernel module failure: https://autobuilder.yoctoproject.org/typhoon/#/builders/110/builds/3395/steps/13/logs/stdio (race/intermittent?) another kernel module race: https://autobuilder.yoctoproject.org/typhoon/#/builders/63/builds/4480/steps/16/logs/stdio Cheers, Richard
On 12/22/21 01:09, Richard Purdie wrote: > On Tue, 2021-12-21 at 11:08 -0800, Saul Wold wrote: >> Stop ignoring or skipping the kernel and kernel modules code in the >> split debug and striping functions, this will allow create_spdx to >> process the kernel and modules. >> >> Signed-off-by: Saul Wold <saul.wold@windriver.com> >> --- >> meta/classes/package.bbclass | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass >> index 84eafbd529..4b7fe4f1e1 100644 >> --- a/meta/classes/package.bbclass >> +++ b/meta/classes/package.bbclass >> @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir, >> dvar = d.getVar('PKGD') >> objcopy = d.getVar("OBJCOPY") >> >> - # We ignore kernel modules, we don't generate debug info files. >> - if file.find("/lib/modules/") != -1 and file.endswith(".ko"): >> - return (file, sources) >> - >> newmode = None >> if not os.access(file, os.W_OK) or os.access(file, os.R_OK): >> origmode = os.stat(file)[stat.ST_MODE] >> @@ -1147,7 +1143,7 @@ python split_and_strip_files () { >> >> if file.endswith(".ko") and file.find("/lib/modules/") != -1: >> kernmods.append(file) >> - continue >> + >> if oe.package.is_static_lib(file): >> staticlibs.append(file) >> continue >> @@ -1165,7 +1161,7 @@ python split_and_strip_files () { >> continue >> # Check its an executable >> if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \ >> - or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)): >> + or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f): >> >> if cpath.islink(file): >> checkelflinks[file] = ltarget > > edgerouter: > https://autobuilder.yoctoproject.org/typhoon/#/builders/62/builds/4513 > https://autobuilder.yoctoproject.org/typhoon/#/builders/111/builds/2507/steps/11/logs/stdio > So I have been digging into this and it seems that an option was added a decade ago or so to strip the kernel/vmlinux when it's too big, this was done for at least the routerstationpro according to bug #3515 [0], and persists with the edgerouter, although I am not sure if it would still actually be required as the edgerouter also uses the KERNEL_ALT_IMAGETYPE to create a smaller binary kernel image. The change I proposed causes the all kernels to be stripped all the time as part of the split_and_strip_files(). As I see it there few different options: 1) Set KERNEL_IMAGE_EXTRA_STRIP_SECTIONS = "" in create_spdx.bbclass - This solves the problem with create_spdx.bbclass is in use, but not the general case 2) Remove the KERNEL_IMAGE_EXTRA_STRIP_SECTIONS from edgerouter.conf - Will solve the edgerouter case but may not solve other usages unknown to me. - Does anyone know of other machines/layers usage of this variable? 3) deprecate the kernel.bbclass:do_strip function in favor of using the split_and_strip_files() of package.bbclass 4) Change error to warning in packaging.bbclass for the kernel only - This would explain that a kernel image (vmlinux) is already stripped and extended package data would not be available for for SPDX creation. RP, Bruce, Joshua: Thoughts? Sau! [0] https://bugzilla.yoctoproject.org/show_bug.cgi?id=3515 > qemux86 musl cryptodev: > https://autobuilder.yoctoproject.org/typhoon/#/builders/64/builds/4512/steps/11/logs/stdio > > qemux86-64 musl cryptodev: > https://autobuilder.yoctoproject.org/typhoon/#/builders/45/builds/4526 > > qemux86 cryptodev: > https://autobuilder.yoctoproject.org/typhoon/#/builders/52/builds/4476/steps/11/logs/stdio > I tried these and have not been able to reproduce the failure. > selftest failure in linux-yocto: > https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/2981/steps/14/logs/stdio > (file truncated makes it sound like a race?) > > stap kernel module failure: > https://autobuilder.yoctoproject.org/typhoon/#/builders/110/builds/3395/steps/13/logs/stdio > (race/intermittent?) > > another kernel module race: > https://autobuilder.yoctoproject.org/typhoon/#/builders/63/builds/4480/steps/16/logs/stdio > Might need to try this on the AB once I resolve the kernel stripping issue above. Sau! > Cheers, > > Richard > > >
On Tue, Jan 4, 2022 at 5:08 PM Saul Wold <Saul.Wold@windriver.com> wrote: > > > > On 12/22/21 01:09, Richard Purdie wrote: > > On Tue, 2021-12-21 at 11:08 -0800, Saul Wold wrote: > >> Stop ignoring or skipping the kernel and kernel modules code in the > >> split debug and striping functions, this will allow create_spdx to > >> process the kernel and modules. > >> > >> Signed-off-by: Saul Wold <saul.wold@windriver.com> > >> --- > >> meta/classes/package.bbclass | 8 ++------ > >> 1 file changed, 2 insertions(+), 6 deletions(-) > >> > >> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass > >> index 84eafbd529..4b7fe4f1e1 100644 > >> --- a/meta/classes/package.bbclass > >> +++ b/meta/classes/package.bbclass > >> @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir, > >> dvar = d.getVar('PKGD') > >> objcopy = d.getVar("OBJCOPY") > >> > >> - # We ignore kernel modules, we don't generate debug info files. > >> - if file.find("/lib/modules/") != -1 and file.endswith(".ko"): > >> - return (file, sources) > >> - > >> newmode = None > >> if not os.access(file, os.W_OK) or os.access(file, os.R_OK): > >> origmode = os.stat(file)[stat.ST_MODE] > >> @@ -1147,7 +1143,7 @@ python split_and_strip_files () { > >> > >> if file.endswith(".ko") and file.find("/lib/modules/") != -1: > >> kernmods.append(file) > >> - continue > >> + > >> if oe.package.is_static_lib(file): > >> staticlibs.append(file) > >> continue > >> @@ -1165,7 +1161,7 @@ python split_and_strip_files () { > >> continue > >> # Check its an executable > >> if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \ > >> - or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)): > >> + or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f): > >> > >> if cpath.islink(file): > >> checkelflinks[file] = ltarget > > > > edgerouter: > > https://autobuilder.yoctoproject.org/typhoon/#/builders/62/builds/4513 > > https://autobuilder.yoctoproject.org/typhoon/#/builders/111/builds/2507/steps/11/logs/stdio > > > So I have been digging into this and it seems that an option was added a > decade ago or so to strip the kernel/vmlinux when it's too big, this was > done for at least the routerstationpro according to bug #3515 [0], and > persists with the edgerouter, although I am not sure if it would still > actually be required as the edgerouter also uses the > KERNEL_ALT_IMAGETYPE to create a smaller binary kernel image. I recall when we added that! It was used for some other boards as well, but most of them aren't around anymore. > > The change I proposed causes the all kernels to be stripped all the time > as part of the split_and_strip_files(). As I see it there few different > options: Having some way to have a custom set of sections to strip (along with skipping stripping (but that can be done via the standard inhibit)) is something I'd suggest we preserve. But I suppose if you inhibit stripping, you'll stop both the packaging one and the kernel custom one ? > > 1) Set KERNEL_IMAGE_EXTRA_STRIP_SECTIONS = "" in create_spdx.bbclass > - This solves the problem with create_spdx.bbclass is in use, but not > the general case What are you considering the general case in this instance ? Meaning a non-spdx user of that same board, will run into issues with the already stripped ? If they can inhibit the do_package stripping, there is a way around it. > > 2) Remove the KERNEL_IMAGE_EXTRA_STRIP_SECTIONS from edgerouter.conf > - Will solve the edgerouter case but may not solve other usages > unknown to me. > - Does anyone know of other machines/layers usage of this variable? > See above. There are some machines, and even if not common, it is something I'd like to preserve. > 3) deprecate the kernel.bbclass:do_strip function in favor of using the > split_and_strip_files() of package.bbclass > I'd prefer to not do #3. > 4) Change error to warning in packaging.bbclass for the kernel only > - This would explain that a kernel image (vmlinux) is already > stripped and extended package data would not be available for for SPDX > creation. #4 is what came to mind for me. We already have special cases for the kernel, so this isn't making things more complex .. or maybe there's a more elegant "co-operative" section removal flag that the kernel bbclass can set, and then the packaging not error or automatically inhibit the QA check ? But #1 is my second choice. Bruce > > RP, Bruce, Joshua: Thoughts? > > Sau! > > [0] https://bugzilla.yoctoproject.org/show_bug.cgi?id=3515 > > > > qemux86 musl cryptodev: > > https://autobuilder.yoctoproject.org/typhoon/#/builders/64/builds/4512/steps/11/logs/stdio > > > > qemux86-64 musl cryptodev: > > https://autobuilder.yoctoproject.org/typhoon/#/builders/45/builds/4526 > > > > qemux86 cryptodev: > > https://autobuilder.yoctoproject.org/typhoon/#/builders/52/builds/4476/steps/11/logs/stdio > > > I tried these and have not been able to reproduce the failure. > > > > > selftest failure in linux-yocto: > > https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/2981/steps/14/logs/stdio > > (file truncated makes it sound like a race?) > > > > stap kernel module failure: > > https://autobuilder.yoctoproject.org/typhoon/#/builders/110/builds/3395/steps/13/logs/stdio > > (race/intermittent?) > > > > another kernel module race: > > https://autobuilder.yoctoproject.org/typhoon/#/builders/63/builds/4480/steps/16/logs/stdio > > > Might need to try this on the AB once I resolve the kernel stripping > issue above. > > Sau! > > > Cheers, > > > > Richard > > > > > > > > -- > Sau! > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#160172): https://lists.openembedded.org/g/openembedded-core/message/160172 > Mute This Topic: https://lists.openembedded.org/mt/87884056/1050810 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > -- - Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end - "Use the force Harry" - Gandalf, Star Trek II
On Tue, 2022-01-04 at 14:07 -0800, Saul Wold wrote: > > On 12/22/21 01:09, Richard Purdie wrote: > > On Tue, 2021-12-21 at 11:08 -0800, Saul Wold wrote: > > > Stop ignoring or skipping the kernel and kernel modules code in the > > > split debug and striping functions, this will allow create_spdx to > > > process the kernel and modules. > > > > > > Signed-off-by: Saul Wold <saul.wold@windriver.com> > > > --- > > > meta/classes/package.bbclass | 8 ++------ > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass > > > index 84eafbd529..4b7fe4f1e1 100644 > > > --- a/meta/classes/package.bbclass > > > +++ b/meta/classes/package.bbclass > > > @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir, > > > dvar = d.getVar('PKGD') > > > objcopy = d.getVar("OBJCOPY") > > > > > > - # We ignore kernel modules, we don't generate debug info files. > > > - if file.find("/lib/modules/") != -1 and file.endswith(".ko"): > > > - return (file, sources) > > > - > > > newmode = None > > > if not os.access(file, os.W_OK) or os.access(file, os.R_OK): > > > origmode = os.stat(file)[stat.ST_MODE] > > > @@ -1147,7 +1143,7 @@ python split_and_strip_files () { > > > > > > if file.endswith(".ko") and file.find("/lib/modules/") != -1: > > > kernmods.append(file) > > > - continue > > > + > > > if oe.package.is_static_lib(file): > > > staticlibs.append(file) > > > continue > > > @@ -1165,7 +1161,7 @@ python split_and_strip_files () { > > > continue > > > # Check its an executable > > > if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \ > > > - or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)): > > > + or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f): > > > > > > if cpath.islink(file): > > > checkelflinks[file] = ltarget > > > > edgerouter: > > https://autobuilder.yoctoproject.org/typhoon/#/builders/62/builds/4513 > > https://autobuilder.yoctoproject.org/typhoon/#/builders/111/builds/2507/steps/11/logs/stdio > > > So I have been digging into this and it seems that an option was added a > decade ago or so to strip the kernel/vmlinux when it's too big, this was > done for at least the routerstationpro according to bug #3515 [0], and > persists with the edgerouter, although I am not sure if it would still > actually be required as the edgerouter also uses the > KERNEL_ALT_IMAGETYPE to create a smaller binary kernel image. > > The change I proposed causes the all kernels to be stripped all the time > as part of the split_and_strip_files(). As I see it there few different > options: > > 1) Set KERNEL_IMAGE_EXTRA_STRIP_SECTIONS = "" in create_spdx.bbclass > - This solves the problem with create_spdx.bbclass is in use, but not > the general case I don't think I like this as it is a side effect that isn't obvious or expected. > > 2) Remove the KERNEL_IMAGE_EXTRA_STRIP_SECTIONS from edgerouter.conf > - Will solve the edgerouter case but may not solve other usages > unknown to me. > - Does anyone know of other machines/layers usage of this variable? > > 3) deprecate the kernel.bbclass:do_strip function in favor of using the > split_and_strip_files() of package.bbclass I know Bruce has said he doesn't like this, however stepping back, these issues were from a time our stripping code was young and evolving. If we can standardise and have it all work together well in one set of functions, I think that is worth looking at. I'd prefer the kernel wasn't a special case if it no longer needs to be. That said, I don't remember the details of why we did this. > > 4) Change error to warning in packaging.bbclass for the kernel only > - This would explain that a kernel image (vmlinux) is already > stripped and extended package data would not be available for for SPDX > creation. > > RP, Bruce, Joshua: Thoughts? If we can simplify and stop the kernel being a special case for this code (or handle kernels generically) that would be worth a bit of effort IMO... Cheers, Richard
On Wed, Jan 5, 2022 at 12:07 PM Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Tue, 2022-01-04 at 14:07 -0800, Saul Wold wrote: > > > > On 12/22/21 01:09, Richard Purdie wrote: > > > On Tue, 2021-12-21 at 11:08 -0800, Saul Wold wrote: > > > > Stop ignoring or skipping the kernel and kernel modules code in the > > > > split debug and striping functions, this will allow create_spdx to > > > > process the kernel and modules. > > > > > > > > Signed-off-by: Saul Wold <saul.wold@windriver.com> > > > > --- > > > > meta/classes/package.bbclass | 8 ++------ > > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass > > > > index 84eafbd529..4b7fe4f1e1 100644 > > > > --- a/meta/classes/package.bbclass > > > > +++ b/meta/classes/package.bbclass > > > > @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir, > > > > dvar = d.getVar('PKGD') > > > > objcopy = d.getVar("OBJCOPY") > > > > > > > > - # We ignore kernel modules, we don't generate debug info files. > > > > - if file.find("/lib/modules/") != -1 and file.endswith(".ko"): > > > > - return (file, sources) > > > > - > > > > newmode = None > > > > if not os.access(file, os.W_OK) or os.access(file, os.R_OK): > > > > origmode = os.stat(file)[stat.ST_MODE] > > > > @@ -1147,7 +1143,7 @@ python split_and_strip_files () { > > > > > > > > if file.endswith(".ko") and file.find("/lib/modules/") != -1: > > > > kernmods.append(file) > > > > - continue > > > > + > > > > if oe.package.is_static_lib(file): > > > > staticlibs.append(file) > > > > continue > > > > @@ -1165,7 +1161,7 @@ python split_and_strip_files () { > > > > continue > > > > # Check its an executable > > > > if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \ > > > > - or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)): > > > > + or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f): > > > > > > > > if cpath.islink(file): > > > > checkelflinks[file] = ltarget > > > > > > edgerouter: > > > https://autobuilder.yoctoproject.org/typhoon/#/builders/62/builds/4513 > > > https://autobuilder.yoctoproject.org/typhoon/#/builders/111/builds/2507/steps/11/logs/stdio > > > > > So I have been digging into this and it seems that an option was added a > > decade ago or so to strip the kernel/vmlinux when it's too big, this was > > done for at least the routerstationpro according to bug #3515 [0], and > > persists with the edgerouter, although I am not sure if it would still > > actually be required as the edgerouter also uses the > > KERNEL_ALT_IMAGETYPE to create a smaller binary kernel image. > > > > The change I proposed causes the all kernels to be stripped all the time > > as part of the split_and_strip_files(). As I see it there few different > > options: > > > > 1) Set KERNEL_IMAGE_EXTRA_STRIP_SECTIONS = "" in create_spdx.bbclass > > - This solves the problem with create_spdx.bbclass is in use, but not > > the general case > > I don't think I like this as it is a side effect that isn't obvious or expected. > > > > > 2) Remove the KERNEL_IMAGE_EXTRA_STRIP_SECTIONS from edgerouter.conf > > - Will solve the edgerouter case but may not solve other usages > > unknown to me. > > - Does anyone know of other machines/layers usage of this variable? > > > > 3) deprecate the kernel.bbclass:do_strip function in favor of using the > > split_and_strip_files() of package.bbclass > > I know Bruce has said he doesn't like this, however stepping back, these issues > were from a time our stripping code was young and evolving. If we can > standardise and have it all work together well in one set of functions, I think > that is worth looking at. I'd prefer the kernel wasn't a special case if it no > longer needs to be. > > That said, I don't remember the details of why we did this. There's a middle ground of debug being possible, and some sections removed to keep the footprint a bit lower. There were also some unwinders, etc, that didn't work when everything was stripped and split into debug. The stripping was too aggressive, and removed some sections that were required. While I can't exactly point to the use cases for it now, with the 5K options in the kernel, they haven't all been removed, and I'd be very hesitant to remove the capability completely. Bruce > > > > > > 4) Change error to warning in packaging.bbclass for the kernel only > > - This would explain that a kernel image (vmlinux) is already > > stripped and extended package data would not be available for for SPDX > > creation. > > > > RP, Bruce, Joshua: Thoughts? > > If we can simplify and stop the kernel being a special case for this code (or > handle kernels generically) that would be worth a bit of effort IMO... > > Cheers, > > Richard > > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#160211): https://lists.openembedded.org/g/openembedded-core/message/160211 > Mute This Topic: https://lists.openembedded.org/mt/87884056/1050810 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Wed, 2022-01-05 at 12:30 -0500, Bruce Ashfield wrote: > On Wed, Jan 5, 2022 at 12:07 PM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > On Tue, 2022-01-04 at 14:07 -0800, Saul Wold wrote: > > > > > > On 12/22/21 01:09, Richard Purdie wrote: > > > > On Tue, 2021-12-21 at 11:08 -0800, Saul Wold wrote: > > > > > Stop ignoring or skipping the kernel and kernel modules code in the > > > > > split debug and striping functions, this will allow create_spdx to > > > > > process the kernel and modules. > > > > > > > > > > Signed-off-by: Saul Wold <saul.wold@windriver.com> > > > > > --- > > > > > meta/classes/package.bbclass | 8 ++------ > > > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass > > > > > index 84eafbd529..4b7fe4f1e1 100644 > > > > > --- a/meta/classes/package.bbclass > > > > > +++ b/meta/classes/package.bbclass > > > > > @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir, > > > > > dvar = d.getVar('PKGD') > > > > > objcopy = d.getVar("OBJCOPY") > > > > > > > > > > - # We ignore kernel modules, we don't generate debug info files. > > > > > - if file.find("/lib/modules/") != -1 and file.endswith(".ko"): > > > > > - return (file, sources) > > > > > - > > > > > newmode = None > > > > > if not os.access(file, os.W_OK) or os.access(file, os.R_OK): > > > > > origmode = os.stat(file)[stat.ST_MODE] > > > > > @@ -1147,7 +1143,7 @@ python split_and_strip_files () { > > > > > > > > > > if file.endswith(".ko") and file.find("/lib/modules/") != -1: > > > > > kernmods.append(file) > > > > > - continue > > > > > + > > > > > if oe.package.is_static_lib(file): > > > > > staticlibs.append(file) > > > > > continue > > > > > @@ -1165,7 +1161,7 @@ python split_and_strip_files () { > > > > > continue > > > > > # Check its an executable > > > > > if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \ > > > > > - or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)): > > > > > + or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f): > > > > > > > > > > if cpath.islink(file): > > > > > checkelflinks[file] = ltarget > > > > > > > > edgerouter: > > > > https://autobuilder.yoctoproject.org/typhoon/#/builders/62/builds/4513 > > > > https://autobuilder.yoctoproject.org/typhoon/#/builders/111/builds/2507/steps/11/logs/stdio > > > > > > > So I have been digging into this and it seems that an option was added a > > > decade ago or so to strip the kernel/vmlinux when it's too big, this was > > > done for at least the routerstationpro according to bug #3515 [0], and > > > persists with the edgerouter, although I am not sure if it would still > > > actually be required as the edgerouter also uses the > > > KERNEL_ALT_IMAGETYPE to create a smaller binary kernel image. > > > > > > The change I proposed causes the all kernels to be stripped all the time > > > as part of the split_and_strip_files(). As I see it there few different > > > options: > > > > > > 1) Set KERNEL_IMAGE_EXTRA_STRIP_SECTIONS = "" in create_spdx.bbclass > > > - This solves the problem with create_spdx.bbclass is in use, but not > > > the general case > > > > I don't think I like this as it is a side effect that isn't obvious or expected. > > > > > > > > 2) Remove the KERNEL_IMAGE_EXTRA_STRIP_SECTIONS from edgerouter.conf > > > - Will solve the edgerouter case but may not solve other usages > > > unknown to me. > > > - Does anyone know of other machines/layers usage of this variable? > > > > > > 3) deprecate the kernel.bbclass:do_strip function in favor of using the > > > split_and_strip_files() of package.bbclass > > > > I know Bruce has said he doesn't like this, however stepping back, these issues > > were from a time our stripping code was young and evolving. If we can > > standardise and have it all work together well in one set of functions, I think > > that is worth looking at. I'd prefer the kernel wasn't a special case if it no > > longer needs to be. > > > > That said, I don't remember the details of why we did this. > > There's a middle ground of debug being possible, and some sections > removed to keep the footprint a bit lower. There were also some > unwinders, etc, that didn't work when everything was stripped and > split into debug. The stripping was too aggressive, and removed some > sections that were required. > > While I can't exactly point to the use cases for it now, with the 5K > options in the kernel, they haven't all been removed, and I'd be very > hesitant to remove the capability completely. What may help would be to move the kernel stripping code from kernel.bbclass to live along with the package stripping code, which would then at least solve some of the QA warning issues and the two code blocks not fighting with each other. it would also allow the debug symbols to move to the linked debug files where appropriate more easily. I think one of the challenges in doing that is the do_deploy of the kernel image though which may not then have the right stripping :(. We may be able to call the strip function from do_deploy though? Cheers, Richard
On 1/5/22 09:30, Bruce Ashfield wrote: > On Wed, Jan 5, 2022 at 12:07 PM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: >> >> On Tue, 2022-01-04 at 14:07 -0800, Saul Wold wrote: >>> >>> On 12/22/21 01:09, Richard Purdie wrote: >>>> On Tue, 2021-12-21 at 11:08 -0800, Saul Wold wrote: >>>>> Stop ignoring or skipping the kernel and kernel modules code in the >>>>> split debug and striping functions, this will allow create_spdx to >>>>> process the kernel and modules. >>>>> >>>>> Signed-off-by: Saul Wold <saul.wold@windriver.com> >>>>> --- >>>>> meta/classes/package.bbclass | 8 ++------ >>>>> 1 file changed, 2 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass >>>>> index 84eafbd529..4b7fe4f1e1 100644 >>>>> --- a/meta/classes/package.bbclass >>>>> +++ b/meta/classes/package.bbclass >>>>> @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir, >>>>> dvar = d.getVar('PKGD') >>>>> objcopy = d.getVar("OBJCOPY") >>>>> >>>>> - # We ignore kernel modules, we don't generate debug info files. >>>>> - if file.find("/lib/modules/") != -1 and file.endswith(".ko"): >>>>> - return (file, sources) >>>>> - >>>>> newmode = None >>>>> if not os.access(file, os.W_OK) or os.access(file, os.R_OK): >>>>> origmode = os.stat(file)[stat.ST_MODE] >>>>> @@ -1147,7 +1143,7 @@ python split_and_strip_files () { >>>>> >>>>> if file.endswith(".ko") and file.find("/lib/modules/") != -1: >>>>> kernmods.append(file) >>>>> - continue >>>>> + >>>>> if oe.package.is_static_lib(file): >>>>> staticlibs.append(file) >>>>> continue >>>>> @@ -1165,7 +1161,7 @@ python split_and_strip_files () { >>>>> continue >>>>> # Check its an executable >>>>> if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \ >>>>> - or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)): >>>>> + or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f): >>>>> >>>>> if cpath.islink(file): >>>>> checkelflinks[file] = ltarget >>>> >>>> edgerouter: >>>> https://autobuilder.yoctoproject.org/typhoon/#/builders/62/builds/4513 >>>> https://autobuilder.yoctoproject.org/typhoon/#/builders/111/builds/2507/steps/11/logs/stdio >>>> >>> So I have been digging into this and it seems that an option was added a >>> decade ago or so to strip the kernel/vmlinux when it's too big, this was >>> done for at least the routerstationpro according to bug #3515 [0], and >>> persists with the edgerouter, although I am not sure if it would still >>> actually be required as the edgerouter also uses the >>> KERNEL_ALT_IMAGETYPE to create a smaller binary kernel image. >>> >>> The change I proposed causes the all kernels to be stripped all the time >>> as part of the split_and_strip_files(). As I see it there few different >>> options: >>> >>> 1) Set KERNEL_IMAGE_EXTRA_STRIP_SECTIONS = "" in create_spdx.bbclass >>> - This solves the problem with create_spdx.bbclass is in use, but not >>> the general case >> >> I don't think I like this as it is a side effect that isn't obvious or expected. >> >>> >>> 2) Remove the KERNEL_IMAGE_EXTRA_STRIP_SECTIONS from edgerouter.conf >>> - Will solve the edgerouter case but may not solve other usages >>> unknown to me. >>> - Does anyone know of other machines/layers usage of this variable? >>> >>> 3) deprecate the kernel.bbclass:do_strip function in favor of using the >>> split_and_strip_files() of package.bbclass >> >> I know Bruce has said he doesn't like this, however stepping back, these issues >> were from a time our stripping code was young and evolving. If we can >> standardise and have it all work together well in one set of functions, I think >> that is worth looking at. I'd prefer the kernel wasn't a special case if it no >> longer needs to be. >> >> That said, I don't remember the details of why we did this. > > There's a middle ground of debug being possible, and some sections > removed to keep the footprint a bit lower. There were also some > unwinders, etc, that didn't work when everything was stripped and > split into debug. The stripping was too aggressive, and removed some > sections that were required. > > While I can't exactly point to the use cases for it now, with the 5K > options in the kernel, they haven't all been removed, and I'd be very > hesitant to remove the capability completely. > I think this makes the most sense after thinking about it also, having one place where the striping occurs in runstrip() in lib/oe/package.py, seems reasonable. The one neck to ring as it were. We can extend the is_elf() types to add vmlinux and use the KERNEL_IMAGE_EXTRA_STRIP_SECTIONS there. So this could deprecate the do_strip() from the kernel.bbclass and keep the behavior. Sau! > Bruce > >> >> >>> >>> 4) Change error to warning in packaging.bbclass for the kernel only >>> - This would explain that a kernel image (vmlinux) is already >>> stripped and extended package data would not be available for for SPDX >>> creation. >>> >>> RP, Bruce, Joshua: Thoughts? >> >> If we can simplify and stop the kernel being a special case for this code (or >> handle kernels generically) that would be worth a bit of effort IMO... >> >> Cheers, >> >> Richard >> >> >> >> >> -=-=-=-=-=-=-=-=-=-=-=- >> Links: You receive all messages sent to this group. >> View/Reply Online (#160211): https://lists.openembedded.org/g/openembedded-core/message/160211 >> Mute This Topic: https://lists.openembedded.org/mt/87884056/1050810 >> Group Owner: openembedded-core+owner@lists.openembedded.org >> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com] >> -=-=-=-=-=-=-=-=-=-=-=- >> > >
diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass index 84eafbd529..4b7fe4f1e1 100644 --- a/meta/classes/package.bbclass +++ b/meta/classes/package.bbclass @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir, dvar = d.getVar('PKGD') objcopy = d.getVar("OBJCOPY") - # We ignore kernel modules, we don't generate debug info files. - if file.find("/lib/modules/") != -1 and file.endswith(".ko"): - return (file, sources) - newmode = None if not os.access(file, os.W_OK) or os.access(file, os.R_OK): origmode = os.stat(file)[stat.ST_MODE] @@ -1147,7 +1143,7 @@ python split_and_strip_files () { if file.endswith(".ko") and file.find("/lib/modules/") != -1: kernmods.append(file) - continue + if oe.package.is_static_lib(file): staticlibs.append(file) continue @@ -1165,7 +1161,7 @@ python split_and_strip_files () { continue # Check its an executable if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \ - or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)): + or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f): if cpath.islink(file): checkelflinks[file] = ltarget
Stop ignoring or skipping the kernel and kernel modules code in the split debug and striping functions, this will allow create_spdx to process the kernel and modules. Signed-off-by: Saul Wold <saul.wold@windriver.com> --- meta/classes/package.bbclass | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)