| Message ID | 20220720113043.196185-1-jose.quaresma@foundries.io |
|---|---|
| State | New |
| Headers | show |
| Series | kernel.bbclass: Add shared_workdir_prepare task | expand |
On 7/20/22 13:30, Jose Quaresma wrote: > The task do_compile_kernelmodules runs after the shared_workdir and > is installing some files in STAGING_KERNEL_BUILDDIR, this can races > in other recipes that depends on "virtual/kernel:do_shared_workdir" > as the STAGING_KERNEL_BUILDDIR is not fully populated when the > shared_workdir task ends. > > To address this issue a new task is added in place of the previows one > so the shared_workdir will run after the do_compile_kernelmodules and > the new shared_workdir_prepare will replce of the old shared_workdir. > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > --- > meta/classes/kernel.bbclass | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass > index 5d2f17c3be..5558769c92 100644 > --- a/meta/classes/kernel.bbclass > +++ b/meta/classes/kernel.bbclass > @@ -504,7 +504,8 @@ do_kernel_version_sanity_check() { > exit 0 > } > > -addtask shared_workdir after do_compile before do_compile_kernelmodules > +addtask shared_workdir_prepare after do_compile before do_compile_kernelmodules > +addtask shared_workdir after do_compile_kernelmodules > addtask shared_workdir_setscene > > do_shared_workdir_setscene () { > @@ -520,10 +521,16 @@ emit_depmod_pkgdata() { > > PACKAGEFUNCS += "emit_depmod_pkgdata" > > -do_shared_workdir[cleandirs] += " ${STAGING_KERNEL_BUILDDIR}" > do_shared_workdir () { > cd ${B} > > + kerneldir=${STAGING_KERNEL_BUILDDIR} > +} Does the above do anything actually useful ? I thought neither the current workdir or a variable set in a shell function would be preserved for the next task ? > + > +do_shared_workdir_prepare[cleandirs] += " ${STAGING_KERNEL_BUILDDIR}" > +do_shared_workdir_prepare () { > + cd ${B} > + > kerneldir=${STAGING_KERNEL_BUILDDIR} > install -d $kerneldir > Jacob
Hi Jacob, Jacob Kroon <jacob.kroon@gmail.com> escreveu no dia terça, 2/08/2022 à(s) 17:19: > On 7/20/22 13:30, Jose Quaresma wrote: > > The task do_compile_kernelmodules runs after the shared_workdir and > > is installing some files in STAGING_KERNEL_BUILDDIR, this can races > > in other recipes that depends on "virtual/kernel:do_shared_workdir" > > as the STAGING_KERNEL_BUILDDIR is not fully populated when the > > shared_workdir task ends. > > > > To address this issue a new task is added in place of the previows one > > so the shared_workdir will run after the do_compile_kernelmodules and > > the new shared_workdir_prepare will replce of the old shared_workdir. > > > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > > --- > > meta/classes/kernel.bbclass | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass > > index 5d2f17c3be..5558769c92 100644 > > --- a/meta/classes/kernel.bbclass > > +++ b/meta/classes/kernel.bbclass > > @@ -504,7 +504,8 @@ do_kernel_version_sanity_check() { > > exit 0 > > } > > > > -addtask shared_workdir after do_compile before do_compile_kernelmodules > > +addtask shared_workdir_prepare after do_compile before > do_compile_kernelmodules > > +addtask shared_workdir after do_compile_kernelmodules > > addtask shared_workdir_setscene > > > > do_shared_workdir_setscene () { > > @@ -520,10 +521,16 @@ emit_depmod_pkgdata() { > > > > PACKAGEFUNCS += "emit_depmod_pkgdata" > > > > -do_shared_workdir[cleandirs] += " ${STAGING_KERNEL_BUILDDIR}" > > do_shared_workdir () { > > cd ${B} > > > > + kerneldir=${STAGING_KERNEL_BUILDDIR} > > +} > > Does the above do anything actually useful ? I thought neither the > current workdir or a variable set in a shell function would be preserved > for the next task ? > It is strictly necessary to be backward compatible with what we have before, otherwise any possible do_shared_workdir:appends that may exist in other layers would stop working. For setting the task work dir you can use the [dirs] flag in the task where the last directory will be the task workdir. https://docs.yoctoproject.org/bitbake/2.0/bitbake-user-manual/bitbake-user-manual-metadata.html#variable-flags Without it the work dir of the task is the current directory where you run bitbake. About the variable created during tasks it only exists until the end of the task where they were created. You can check that inside $WORKDIR/temp/run.TASK, in each case is $WORKDIR/temp/run.do_shared_workdir So we have to keep the working directory as well as the variables from the do_shared_workdir_prepare that are expected at the end of the do_shared_workdir. Jose > > + > > +do_shared_workdir_prepare[cleandirs] += " ${STAGING_KERNEL_BUILDDIR}" > > +do_shared_workdir_prepare () { > > + cd ${B} > > + > > kerneldir=${STAGING_KERNEL_BUILDDIR} > > install -d $kerneldir > > > > Jacob >
Sorry for the late reply, but we were discussing this patch on IRC today, and I wanted to follow up with some questions / clarifications. On Wed, Jul 20, 2022 at 7:30 AM Jose Quaresma <quaresma.jose@gmail.com> wrote: > > The task do_compile_kernelmodules runs after the shared_workdir and > is installing some files in STAGING_KERNEL_BUILDDIR, this can races > in other recipes that depends on "virtual/kernel:do_shared_workdir" > as the STAGING_KERNEL_BUILDDIR is not fully populated when the > shared_workdir task ends. I remember considering this when we did the update of the module.lds in do_compile_kernemodules(), but we had at the time (and still do) use cases that there are no modules, so there isn't always an update to the shared_workdir Is that the race that you were seeing ? And dependent builds are seeing missing elements of it ? Or is there something else ? We also were (and also still are) concerned about making the already long kernel build dependency even longer, without a way to at least allow recipe authors that know what they are doing, to avoid the extra time required for modules to build. Which leads to one question. Tasks that do need module.lds or update it, could just use do_compile_kernelmodules() as their explicit dependency. Was that considered and did meet the need ? Admittedly doing that leaves a gap that a new developer could shoot through and cause themselves some failures or subtle mis-behaviour. > > To address this issue a new task is added in place of the previows one > so the shared_workdir will run after the do_compile_kernelmodules and > the new shared_workdir_prepare will replce of the old shared_workdir. > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > --- > meta/classes/kernel.bbclass | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass > index 5d2f17c3be..5558769c92 100644 > --- a/meta/classes/kernel.bbclass > +++ b/meta/classes/kernel.bbclass > @@ -504,7 +504,8 @@ do_kernel_version_sanity_check() { > exit 0 > } > > -addtask shared_workdir after do_compile before do_compile_kernelmodules > +addtask shared_workdir_prepare after do_compile before do_compile_kernelmodules > +addtask shared_workdir after do_compile_kernelmodules Assuming that we do go for the extra compilation in the default shared_workdir dependency chain, can't we just change shared_workdir to be after do_compile_kernelmodules for a smaller footprint change ? ... unless the do_compile_kernelmodules() are using part of the shared_workdir for its build, but I don't recall that being the case. Bruce > addtask shared_workdir_setscene > > do_shared_workdir_setscene () { > @@ -520,10 +521,16 @@ emit_depmod_pkgdata() { > > PACKAGEFUNCS += "emit_depmod_pkgdata" > > -do_shared_workdir[cleandirs] += " ${STAGING_KERNEL_BUILDDIR}" > do_shared_workdir () { > cd ${B} > > + kerneldir=${STAGING_KERNEL_BUILDDIR} > +} > + > +do_shared_workdir_prepare[cleandirs] += " ${STAGING_KERNEL_BUILDDIR}" > +do_shared_workdir_prepare () { > + cd ${B} > + > kerneldir=${STAGING_KERNEL_BUILDDIR} > install -d $kerneldir > > -- > 2.37.1 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#168359): https://lists.openembedded.org/g/openembedded-core/message/168359 > Mute This Topic: https://lists.openembedded.org/mt/92502346/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/kernel.bbclass b/meta/classes/kernel.bbclass index 5d2f17c3be..5558769c92 100644 --- a/meta/classes/kernel.bbclass +++ b/meta/classes/kernel.bbclass @@ -504,7 +504,8 @@ do_kernel_version_sanity_check() { exit 0 } -addtask shared_workdir after do_compile before do_compile_kernelmodules +addtask shared_workdir_prepare after do_compile before do_compile_kernelmodules +addtask shared_workdir after do_compile_kernelmodules addtask shared_workdir_setscene do_shared_workdir_setscene () { @@ -520,10 +521,16 @@ emit_depmod_pkgdata() { PACKAGEFUNCS += "emit_depmod_pkgdata" -do_shared_workdir[cleandirs] += " ${STAGING_KERNEL_BUILDDIR}" do_shared_workdir () { cd ${B} + kerneldir=${STAGING_KERNEL_BUILDDIR} +} + +do_shared_workdir_prepare[cleandirs] += " ${STAGING_KERNEL_BUILDDIR}" +do_shared_workdir_prepare () { + cd ${B} + kerneldir=${STAGING_KERNEL_BUILDDIR} install -d $kerneldir
The task do_compile_kernelmodules runs after the shared_workdir and is installing some files in STAGING_KERNEL_BUILDDIR, this can races in other recipes that depends on "virtual/kernel:do_shared_workdir" as the STAGING_KERNEL_BUILDDIR is not fully populated when the shared_workdir task ends. To address this issue a new task is added in place of the previows one so the shared_workdir will run after the do_compile_kernelmodules and the new shared_workdir_prepare will replce of the old shared_workdir. Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> --- meta/classes/kernel.bbclass | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)