diff mbox series

[RFC] Allow fitimage + initramfs rebuild to be accelerated

Message ID 1667435031-7224-1-git-send-email-paul.eggleton@linux.microsoft.com
State New
Headers show
Series [RFC] Allow fitimage + initramfs rebuild to be accelerated | expand

Commit Message

Paul Eggleton Nov. 3, 2022, 12:23 a.m. UTC 
From: Paul Eggleton <paul.eggleton@microsoft.com>

We have a usecase where our initramfs changes on every build (since it
contains a version number which is based on the date), and we're
incorporating that into a fitImage. Most of the time our builds are done
from shared state and we want them to be as efficient as possible.
Currently, when the initramfs signature is different this setup requires
the kernel to be recompiled on every build, which is not ideal. I've
come up with the following rough patch to address this, which does the
following:

* Avoid the dependency of do_bundle_initramfs on ${INITRAMFS_IMAGE} if
  we are going to be building that for the fit image later anyway
* Add a do_preserve_kernel_build task to preserve the files needed to
  re-run do_assemble_fitimage_initramfs in a separate location from
  ${B}, which can be saved to sstate
* Add a do_deploy_fitimage task to deploy just the files written out by
  do_assemble_fitimage_initramfs to ${DEPLOY_DIR_IMAGE}; this allows us
  to preserve less in sstate than we would have to if we allowed
  do_deploy to rerun. Of course the downside is we then need to add a
  dependency from the image's do_build on the kernel's
  do_deploy_fitimage, and I had to introduce a variable to control that
  which is a bit awkward.

This patch is quite rough and I'd like to take suggestions on how to
improve it. One of my challenges is I don't have a complete grasp of all
the usage modes supported by this code so I'd appreciate folks who do
taking a look.

Thanks!
Paul
---
 meta/classes-recipe/image.bbclass           |  5 +--
 meta/classes-recipe/kernel-fitimage.bbclass | 55 ++++++++++++++++++++++++-----
 meta/classes-recipe/kernel.bbclass          |  2 +-
 3 files changed, 51 insertions(+), 11 deletions(-)

Comments

Paul Eggleton Nov. 9, 2022, 7:41 p.m. UTC | #1 
Hi folks

Any comments? Alternative approaches?

Thanks
Paul


----- original message -----
Subject: [OE-core] [RFC] [PATCH] Allow fitimage + initramfs rebuild to be accelerated
Date: Thursday, 3 November 2022, 13:23:51 NZDT
From: Paul Eggleton <paul.eggleton@linux.microsoft.com>
To: openembedded-core@lists.openembedded.org

We have a usecase where our initramfs changes on every build (since it
contains a version number which is based on the date), and we're
incorporating that into a fitImage. Most of the time our builds are done
from shared state and we want them to be as efficient as possible.
Currently, when the initramfs signature is different this setup requires
the kernel to be recompiled on every build, which is not ideal. I've
come up with the following rough patch to address this, which does the
following:

* Avoid the dependency of do_bundle_initramfs on ${INITRAMFS_IMAGE} if
  we are going to be building that for the fit image later anyway
* Add a do_preserve_kernel_build task to preserve the files needed to
  re-run do_assemble_fitimage_initramfs in a separate location from
  ${B}, which can be saved to sstate
* Add a do_deploy_fitimage task to deploy just the files written out by
  do_assemble_fitimage_initramfs to ${DEPLOY_DIR_IMAGE}; this allows us
  to preserve less in sstate than we would have to if we allowed
  do_deploy to rerun. Of course the downside is we then need to add a
  dependency from the image's do_build on the kernel's
  do_deploy_fitimage, and I had to introduce a variable to control that
  which is a bit awkward.

This patch is quite rough and I'd like to take suggestions on how to
improve it. One of my challenges is I don't have a complete grasp of all
the usage modes supported by this code so I'd appreciate folks who do
taking a look.

Thanks!
Paul
---
 meta/classes-recipe/image.bbclass           |  5 +--
 meta/classes-recipe/kernel-fitimage.bbclass | 55 ++++++++++++++++++++++++-----
 meta/classes-recipe/kernel.bbclass          |  2 +-
 3 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/meta/classes-recipe/image.bbclass b/meta/classes-recipe/image.bbclass
index e387645..cedd913 100644
--- a/meta/classes-recipe/image.bbclass
+++ b/meta/classes-recipe/image.bbclass
@@ -137,13 +137,14 @@ def rootfs_variables(d):
 
 do_rootfs[vardeps] += "${@rootfs_variables(d)}"
 
+IMAGE_FITIMAGE_DEPEND = "${@'virtual/kernel:do_deploy_fitimage' if d.getVar('IMAGE_FITIMAGE') == '1' else ''}"
+
 # This is needed to have kernel image in DEPLOY_DIR.
 # This follows many common usecases and user expectations.
 # But if you are building an image which doesn't need the kernel image at all,
 # you can unset this variable manually.
 KERNEL_DEPLOY_DEPEND ?= "virtual/kernel:do_deploy"
-do_build[depends] += "${KERNEL_DEPLOY_DEPEND}"
-
+do_build[depends] += "${KERNEL_DEPLOY_DEPEND} ${IMAGE_FITIMAGE_DEPEND}"
 
 python () {
     def extraimage_getdepends(task):
diff --git a/meta/classes-recipe/kernel-fitimage.bbclass b/meta/classes-recipe/kernel-fitimage.bbclass
index 7980910..40406fc 100644
--- a/meta/classes-recipe/kernel-fitimage.bbclass
+++ b/meta/classes-recipe/kernel-fitimage.bbclass
@@ -731,10 +731,32 @@ do_install:append() {
        fi
 }
 
+python do_preserve_kernel_build() {
+    archdir = d.getVar('KERNEL_OUTPUT_DIR')
+    src = d.getVar('B')
+    dst = d.getVar('INITRAMFSDIR')
+    oe.path.copyhardlinktree(os.path.join(src, archdir), os.path.join(dst, archdir))
+    for fn in ['vmlinux', 'fit-image.its']:
+        oe.path.copyhardlink(os.path.join(src, fn), os.path.join(dst, fn))
+}
+
+INITRAMFSDIR = "${WORKDIR}/initramfs-work"
+SSTATETASKS += "do_preserve_kernel_build"
+do_preserve_kernel_build[cleandirs] = "${INITRAMFSDIR}"
+do_preserve_kernel_build[sstate-plaindirs] = "${INITRAMFSDIR}"
+
+python do_preserve_kernel_build_setscene () {
+    sstate_setscene(d)
+}
+addtask preserve_kernel_build_setscene
+
+
+addtask preserve_kernel_build after do_bundle_initramfs
+
 do_assemble_fitimage_initramfs() {
        if echo ${KERNEL_IMAGETYPES} | grep -wq "fitImage" && \
                test -n "${INITRAMFS_IMAGE}" ; then
-               cd ${B}
+               cd ${INITRAMFSDIR}
                if [ "${INITRAMFS_IMAGE_BUNDLE}" = "1" ]; then
                        fitimage_assemble fit-image-${INITRAMFS_IMAGE}.its fitImage-bundle ""
                        ln -sf fitImage-bundle ${B}/${KERNEL_OUTPUT_DIR}/fitImage
@@ -744,7 +766,9 @@ do_assemble_fitimage_initramfs() {
        fi
 }
 
-addtask assemble_fitimage_initramfs before do_deploy after do_bundle_initramfs
+addtask assemble_fitimage_initramfs after do_preserve_kernel_build
+
+do_assemble_fitimage_initramfs[depends] += "virtual/${TARGET_PREFIX}binutils:do_populate_sysroot"
 
 do_kernel_generate_rsa_keys() {
        if [ "${UBOOT_SIGN_ENABLE}" = "0" ] && [ "${FIT_GENERATE_KEYS}" = "1" ]; then
@@ -793,20 +817,33 @@ do_kernel_generate_rsa_keys() {
 
 addtask kernel_generate_rsa_keys before do_assemble_fitimage after do_compile
 
-kernel_do_deploy[vardepsexclude] = "DATETIME"
-kernel_do_deploy:append() {
+FITIMGDEPLOYDIR = "${WORKDIR}/fitimage-deploy"
+SSTATETASKS += "do_deploy_fitimage"
+do_deploy_fitimage[sstate-inputdirs] = "${FITIMGDEPLOYDIR}"
+do_deploy_fitimage[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}"
+
+python do_deploy_fitimage_setscene () {
+        sstate_setscene(d)
+}
+addtask do_deploy_fitimage_setscene
+
+do_deploy_fitimage[dirs] = "${FITIMGDEPLOYDIR} ${INITRAMFSDIR}"
+
+do_deploy_fitimage[vardepsexclude] = "DATETIME"
+do_deploy_fitimage() {
+       deployDir="${FITIMGDEPLOYDIR}"
        # Update deploy directory
        if echo ${KERNEL_IMAGETYPES} | grep -wq "fitImage"; then
 
                if [ "${INITRAMFS_IMAGE_BUNDLE}" != "1" ]; then
                        bbnote "Copying fit-image.its source file..."
-                       install -m 0644 ${B}/fit-image.its "$deployDir/fitImage-its-${KERNEL_FIT_NAME}.its"
+                       install -m 0644 ${INITRAMFSDIR}/fit-image.its "$deployDir/fitImage-its-${KERNEL_FIT_NAME}.its"
                        if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
                                ln -snf fitImage-its-${KERNEL_FIT_NAME}.its "$deployDir/fitImage-its-${KERNEL_FIT_LINK_NAME}"
                        fi
 
                        bbnote "Copying linux.bin file..."
-                       install -m 0644 ${B}/linux.bin $deployDir/fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}
+                       install -m 0644 ${INITRAMFSDIR}/linux.bin $deployDir/fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}
                        if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
                                ln -snf fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT} "$deployDir/fitImage-linux.bin-${KERNEL_FIT_LINK_NAME}"
                        fi
@@ -814,14 +851,14 @@ kernel_do_deploy:append() {
 
                if [ -n "${INITRAMFS_IMAGE}" ]; then
                        bbnote "Copying fit-image-${INITRAMFS_IMAGE}.its source file..."
-                       install -m 0644 ${B}/fit-image-${INITRAMFS_IMAGE}.its "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its"
+                       install -m 0644 ${INITRAMFSDIR}/fit-image-${INITRAMFS_IMAGE}.its "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its"
                        if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
                                ln -snf fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_LINK_NAME}"
                        fi
 
                        if [ "${INITRAMFS_IMAGE_BUNDLE}" != "1" ]; then
                                bbnote "Copying fitImage-${INITRAMFS_IMAGE} file..."
-                               install -m 0644 ${B}/${KERNEL_OUTPUT_DIR}/fitImage-${INITRAMFS_IMAGE} "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}"
+                               install -m 0644 ${INITRAMFSDIR}/${KERNEL_OUTPUT_DIR}/fitImage-${INITRAMFS_IMAGE} "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}"
                                if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
                                        ln -snf fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT} "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_LINK_NAME}"
                                fi
@@ -829,3 +866,5 @@ kernel_do_deploy:append() {
                fi
        fi
 }
+
+addtask do_deploy_fitimage before do_build after do_deploy do_assemble_fitimage_initramfs
diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
index 7bb3449..f2efb3e 100644
--- a/meta/classes-recipe/kernel.bbclass
+++ b/meta/classes-recipe/kernel.bbclass
@@ -140,7 +140,7 @@ set -e
     # If the INTIRAMFS_IMAGE is set but the INITRAMFS_IMAGE_BUNDLE is set to 0,
     # the do_bundle_initramfs does nothing, but the INITRAMFS_IMAGE is built
     # standalone for use by wic and other tools.
-    if image:
+    if image and 'fitImage' not in d.getVar('KERNEL_IMAGETYPES'):
         if d.getVar('INITRAMFS_MULTICONFIG'):
             d.appendVarFlag('do_bundle_initramfs', 'mcdepends', ' mc::${INITRAMFS_MULTICONFIG}:${INITRAMFS_IMAGE}:do_image_complete')
         else:
Martin Jansa Nov. 9, 2022, 7:53 p.m. UTC | #2 
https://bugzilla.yoctoproject.org/show_bug.cgi?id=12937 might be
alternative solution which we at LGE were using for different usecase than
fit image, but in theory should work as well.

Basically the initramfs do_deploy could be without version and then the
hardlink with datetime (or any other version) is created in fast
do_deploy_links task.

Similarly whole kernel can be reused from sstate and only hardlinks created
with new version if the do_deploy signatures still match.

There are still some selftest issues caused by my changes for #12937, last
weekend I've left selftest running for 24 hours and got:
2022-11-06 00:02:22,894 - oe-selftest - INFO - oe-selftest () - Ran 520
tests in 86409.229s
2022-11-06 00:02:22,894 - oe-selftest - INFO - oe-selftest - FAIL -
Required tests failed (successes=427, skipped=11, failures=36, errors=53)

Now going through the failed tests and some aren't reproducible and some
are caused by hardcoded image name expectations in selftest (e.g. not
expecting now configureable ".rootfs" suffix for images).

Regards,

On Wed, Nov 9, 2022 at 8:42 PM Paul Eggleton <
bluelightning@bluelightning.org> wrote:

> Hi folks
>
> Any comments? Alternative approaches?
>
> Thanks
> Paul
>
>
> ----- original message -----
> Subject: [OE-core] [RFC] [PATCH] Allow fitimage + initramfs rebuild to be
> accelerated
> Date: Thursday, 3 November 2022, 13:23:51 NZDT
> From: Paul Eggleton <paul.eggleton@linux.microsoft.com>
> To: openembedded-core@lists.openembedded.org
>
> We have a usecase where our initramfs changes on every build (since it
> contains a version number which is based on the date), and we're
> incorporating that into a fitImage. Most of the time our builds are done
> from shared state and we want them to be as efficient as possible.
> Currently, when the initramfs signature is different this setup requires
> the kernel to be recompiled on every build, which is not ideal. I've
> come up with the following rough patch to address this, which does the
> following:
>
> * Avoid the dependency of do_bundle_initramfs on ${INITRAMFS_IMAGE} if
>   we are going to be building that for the fit image later anyway
> * Add a do_preserve_kernel_build task to preserve the files needed to
>   re-run do_assemble_fitimage_initramfs in a separate location from
>   ${B}, which can be saved to sstate
> * Add a do_deploy_fitimage task to deploy just the files written out by
>   do_assemble_fitimage_initramfs to ${DEPLOY_DIR_IMAGE}; this allows us
>   to preserve less in sstate than we would have to if we allowed
>   do_deploy to rerun. Of course the downside is we then need to add a
>   dependency from the image's do_build on the kernel's
>   do_deploy_fitimage, and I had to introduce a variable to control that
>   which is a bit awkward.
>
> This patch is quite rough and I'd like to take suggestions on how to
> improve it. One of my challenges is I don't have a complete grasp of all
> the usage modes supported by this code so I'd appreciate folks who do
> taking a look.
>
> Thanks!
> Paul
> ---
>  meta/classes-recipe/image.bbclass           |  5 +--
>  meta/classes-recipe/kernel-fitimage.bbclass | 55
> ++++++++++++++++++++++++-----
>  meta/classes-recipe/kernel.bbclass          |  2 +-
>  3 files changed, 51 insertions(+), 11 deletions(-)
>
> diff --git a/meta/classes-recipe/image.bbclass
> b/meta/classes-recipe/image.bbclass
> index e387645..cedd913 100644
> --- a/meta/classes-recipe/image.bbclass
> +++ b/meta/classes-recipe/image.bbclass
> @@ -137,13 +137,14 @@ def rootfs_variables(d):
>
>  do_rootfs[vardeps] += "${@rootfs_variables(d)}"
>
> +IMAGE_FITIMAGE_DEPEND = "${@'virtual/kernel:do_deploy_fitimage' if
> d.getVar('IMAGE_FITIMAGE') == '1' else ''}"
> +
>  # This is needed to have kernel image in DEPLOY_DIR.
>  # This follows many common usecases and user expectations.
>  # But if you are building an image which doesn't need the kernel image at
> all,
>  # you can unset this variable manually.
>  KERNEL_DEPLOY_DEPEND ?= "virtual/kernel:do_deploy"
> -do_build[depends] += "${KERNEL_DEPLOY_DEPEND}"
> -
> +do_build[depends] += "${KERNEL_DEPLOY_DEPEND} ${IMAGE_FITIMAGE_DEPEND}"
>
>  python () {
>      def extraimage_getdepends(task):
> diff --git a/meta/classes-recipe/kernel-fitimage.bbclass
> b/meta/classes-recipe/kernel-fitimage.bbclass
> index 7980910..40406fc 100644
> --- a/meta/classes-recipe/kernel-fitimage.bbclass
> +++ b/meta/classes-recipe/kernel-fitimage.bbclass
> @@ -731,10 +731,32 @@ do_install:append() {
>         fi
>  }
>
> +python do_preserve_kernel_build() {
> +    archdir = d.getVar('KERNEL_OUTPUT_DIR')
> +    src = d.getVar('B')
> +    dst = d.getVar('INITRAMFSDIR')
> +    oe.path.copyhardlinktree(os.path.join(src, archdir),
> os.path.join(dst, archdir))
> +    for fn in ['vmlinux', 'fit-image.its']:
> +        oe.path.copyhardlink(os.path.join(src, fn), os.path.join(dst, fn))
> +}
> +
> +INITRAMFSDIR = "${WORKDIR}/initramfs-work"
> +SSTATETASKS += "do_preserve_kernel_build"
> +do_preserve_kernel_build[cleandirs] = "${INITRAMFSDIR}"
> +do_preserve_kernel_build[sstate-plaindirs] = "${INITRAMFSDIR}"
> +
> +python do_preserve_kernel_build_setscene () {
> +    sstate_setscene(d)
> +}
> +addtask preserve_kernel_build_setscene
> +
> +
> +addtask preserve_kernel_build after do_bundle_initramfs
> +
>  do_assemble_fitimage_initramfs() {
>         if echo ${KERNEL_IMAGETYPES} | grep -wq "fitImage" && \
>                 test -n "${INITRAMFS_IMAGE}" ; then
> -               cd ${B}
> +               cd ${INITRAMFSDIR}
>                 if [ "${INITRAMFS_IMAGE_BUNDLE}" = "1" ]; then
>                         fitimage_assemble fit-image-${INITRAMFS_IMAGE}.its
> fitImage-bundle ""
>                         ln -sf fitImage-bundle
> ${B}/${KERNEL_OUTPUT_DIR}/fitImage
> @@ -744,7 +766,9 @@ do_assemble_fitimage_initramfs() {
>         fi
>  }
>
> -addtask assemble_fitimage_initramfs before do_deploy after
> do_bundle_initramfs
> +addtask assemble_fitimage_initramfs after do_preserve_kernel_build
> +
> +do_assemble_fitimage_initramfs[depends] +=
> "virtual/${TARGET_PREFIX}binutils:do_populate_sysroot"
>
>  do_kernel_generate_rsa_keys() {
>         if [ "${UBOOT_SIGN_ENABLE}" = "0" ] && [ "${FIT_GENERATE_KEYS}" =
> "1" ]; then
> @@ -793,20 +817,33 @@ do_kernel_generate_rsa_keys() {
>
>  addtask kernel_generate_rsa_keys before do_assemble_fitimage after
> do_compile
>
> -kernel_do_deploy[vardepsexclude] = "DATETIME"
> -kernel_do_deploy:append() {
> +FITIMGDEPLOYDIR = "${WORKDIR}/fitimage-deploy"
> +SSTATETASKS += "do_deploy_fitimage"
> +do_deploy_fitimage[sstate-inputdirs] = "${FITIMGDEPLOYDIR}"
> +do_deploy_fitimage[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}"
> +
> +python do_deploy_fitimage_setscene () {
> +        sstate_setscene(d)
> +}
> +addtask do_deploy_fitimage_setscene
> +
> +do_deploy_fitimage[dirs] = "${FITIMGDEPLOYDIR} ${INITRAMFSDIR}"
> +
> +do_deploy_fitimage[vardepsexclude] = "DATETIME"
> +do_deploy_fitimage() {
> +       deployDir="${FITIMGDEPLOYDIR}"
>         # Update deploy directory
>         if echo ${KERNEL_IMAGETYPES} | grep -wq "fitImage"; then
>
>                 if [ "${INITRAMFS_IMAGE_BUNDLE}" != "1" ]; then
>                         bbnote "Copying fit-image.its source file..."
> -                       install -m 0644 ${B}/fit-image.its
> "$deployDir/fitImage-its-${KERNEL_FIT_NAME}.its"
> +                       install -m 0644 ${INITRAMFSDIR}/fit-image.its
> "$deployDir/fitImage-its-${KERNEL_FIT_NAME}.its"
>                         if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
>                                 ln -snf
> fitImage-its-${KERNEL_FIT_NAME}.its
> "$deployDir/fitImage-its-${KERNEL_FIT_LINK_NAME}"
>                         fi
>
>                         bbnote "Copying linux.bin file..."
> -                       install -m 0644 ${B}/linux.bin
> $deployDir/fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}
> +                       install -m 0644 ${INITRAMFSDIR}/linux.bin
> $deployDir/fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}
>                         if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
>                                 ln -snf
> fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}
> "$deployDir/fitImage-linux.bin-${KERNEL_FIT_LINK_NAME}"
>                         fi
> @@ -814,14 +851,14 @@ kernel_do_deploy:append() {
>
>                 if [ -n "${INITRAMFS_IMAGE}" ]; then
>                         bbnote "Copying fit-image-${INITRAMFS_IMAGE}.its
> source file..."
> -                       install -m 0644
> ${B}/fit-image-${INITRAMFS_IMAGE}.its
> "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its"
> +                       install -m 0644
> ${INITRAMFSDIR}/fit-image-${INITRAMFS_IMAGE}.its
> "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its"
>                         if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
>                                 ln -snf
> fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its
> "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_LINK_NAME}"
>                         fi
>
>                         if [ "${INITRAMFS_IMAGE_BUNDLE}" != "1" ]; then
>                                 bbnote "Copying
> fitImage-${INITRAMFS_IMAGE} file..."
> -                               install -m 0644
> ${B}/${KERNEL_OUTPUT_DIR}/fitImage-${INITRAMFS_IMAGE}
> "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}"
> +                               install -m 0644
> ${INITRAMFSDIR}/${KERNEL_OUTPUT_DIR}/fitImage-${INITRAMFS_IMAGE}
> "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}"
>                                 if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
>                                         ln -snf
> fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}
> "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_LINK_NAME}"
>                                 fi
> @@ -829,3 +866,5 @@ kernel_do_deploy:append() {
>                 fi
>         fi
>  }
> +
> +addtask do_deploy_fitimage before do_build after do_deploy
> do_assemble_fitimage_initramfs
> diff --git a/meta/classes-recipe/kernel.bbclass
> b/meta/classes-recipe/kernel.bbclass
> index 7bb3449..f2efb3e 100644
> --- a/meta/classes-recipe/kernel.bbclass
> +++ b/meta/classes-recipe/kernel.bbclass
> @@ -140,7 +140,7 @@ set -e
>      # If the INTIRAMFS_IMAGE is set but the INITRAMFS_IMAGE_BUNDLE is set
> to 0,
>      # the do_bundle_initramfs does nothing, but the INITRAMFS_IMAGE is
> built
>      # standalone for use by wic and other tools.
> -    if image:
> +    if image and 'fitImage' not in d.getVar('KERNEL_IMAGETYPES'):
>          if d.getVar('INITRAMFS_MULTICONFIG'):
>              d.appendVarFlag('do_bundle_initramfs', 'mcdepends', '
> mc::${INITRAMFS_MULTICONFIG}:${INITRAMFS_IMAGE}:do_image_complete')
>          else:
> --
> 1.8.3.1
>
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#173049):
> https://lists.openembedded.org/g/openembedded-core/message/173049
> Mute This Topic: https://lists.openembedded.org/mt/94747626/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> Martin.Jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Richard Purdie Nov. 9, 2022, 10:14 p.m. UTC | #3 
On Wed, 2022-11-02 at 17:23 -0700, Paul Eggleton wrote:
> From: Paul Eggleton <paul.eggleton@microsoft.com>
> 
> We have a usecase where our initramfs changes on every build (since it
> contains a version number which is based on the date), and we're
> incorporating that into a fitImage. Most of the time our builds are done
> from shared state and we want them to be as efficient as possible.
> Currently, when the initramfs signature is different this setup requires
> the kernel to be recompiled on every build, which is not ideal. I've
> come up with the following rough patch to address this, which does the
> following:
> 
> * Avoid the dependency of do_bundle_initramfs on ${INITRAMFS_IMAGE} if
>   we are going to be building that for the fit image later anyway
> * Add a do_preserve_kernel_build task to preserve the files needed to
>   re-run do_assemble_fitimage_initramfs in a separate location from
>   ${B}, which can be saved to sstate
> * Add a do_deploy_fitimage task to deploy just the files written out by
>   do_assemble_fitimage_initramfs to ${DEPLOY_DIR_IMAGE}; this allows us
>   to preserve less in sstate than we would have to if we allowed
>   do_deploy to rerun. Of course the downside is we then need to add a
>   dependency from the image's do_build on the kernel's
>   do_deploy_fitimage, and I had to introduce a variable to control that
>   which is a bit awkward.
> 
> This patch is quite rough and I'd like to take suggestions on how to
> improve it. One of my challenges is I don't have a complete grasp of all
> the usage modes supported by this code so I'd appreciate folks who do
> taking a look.

I don't have good answers unfortunately.

We have tried really hard to avoid putting all of the kernel build
output into sstate. The main reason for that was that recompiling the
kernel from source took around the same amount of time as compressing
it and moving it around in sstate, it was huge.

If we're going to start putting all the kernel bits into sstate, we
would probably rethink kernel-devsrc and a few other pieces too. I'm
not sure we should do that but they're all connected. Perhaps we do
give in and do that but I doubt people will like the build times or
sstate size increases :(.

I'd also observe that we don't have good tests for a lot of these
different use cases. The code has slowly been turning into an if/else
labyrinth and it is very unclear which usecases are being used by
people. We have seen a rise in test cases and I have pushed for them
where I can but the situation isn't great and is a big worry whenever
we make changes in here.

Adding in yet further if/else paths with magic variables to control
them isn't filling me with confidence.

The recent work Sean Anderson did on fitimage with u-boot looked like a
promising de-cluttering of some of the maze.

I added Bruce to Cc:.

Cheers,

Richard
Paul Eggleton Nov. 10, 2022, 8:21 p.m. UTC | #4 
On Thursday, 10 November 2022 08:53:02 NZDT Martin Jansa wrote:
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=12937 might be
> alternative solution which we at LGE were using for different usecase than
> fit image, but in theory should work as well.
> 
> Basically the initramfs do_deploy could be without version and then the
> hardlink with datetime (or any other version) is created in fast
> do_deploy_links task.

Unless I've missed something I don't think this can help - in our case the 
content of the initramfs is changing, not just the filename due to the version.

Cheers
Paul
Paul Eggleton Nov. 10, 2022, 8:32 p.m. UTC | #5 
Hi Richard

On Thursday, 10 November 2022 11:14:24 NZDT Richard Purdie wrote:
> We have tried really hard to avoid putting all of the kernel build
> output into sstate. The main reason for that was that recompiling the
> kernel from source took around the same amount of time as compressing
> it and moving it around in sstate, it was huge.
> 
> If we're going to start putting all the kernel bits into sstate, we
> would probably rethink kernel-devsrc and a few other pieces too. I'm
> not sure we should do that but they're all connected. Perhaps we do
> give in and do that but I doubt people will like the build times or
> sstate size increases :(.

Right, and I had avoided putting too much into sstate with this patch - 
basically just vmlinux and the contents of arch/${ARCH}/boot (via the perhaps 
a bit broadly named KERNEL_OUTPUT_DIR) - possibly even that could be trimmed a 
bit. For our case building the kernel takes a significant time because (long 
story short) there are actually two kernels building (debug and regular 
flavours) and there are a number of items that depend upon them -> they also 
get rebuilt if we don't untangle this a little bit.

I'm now wondering if part of the solution here would be to move some of the fit 
image + initramfs assembly to the initramfs image context. That would mean 
that do_deploy would need to save away everything necessary to do that, but 
that shouldn't be a huge amount of data.

> I'd also observe that we don't have good tests for a lot of these
> different use cases. The code has slowly been turning into an if/else
> labyrinth and it is very unclear which usecases are being used by
> people. We have seen a rise in test cases and I have pushed for them
> where I can but the situation isn't great and is a big worry whenever
> we make changes in here.

I'm all for adding additional tests, certainly, and I'm happy to at least some 
of that work. I'd need to get a better understanding of some of the other use 
cases though.

> Adding in yet further if/else paths with magic variables to control
> them isn't filling me with confidence.

I understand that. I was hoping to figure out a way to avoid adding significant 
extra complexity.
 
> The recent work Sean Anderson did on fitimage with u-boot looked like a
> promising de-cluttering of some of the maze.

Indeed.

Paul
Rasmus Villemoes Nov. 11, 2022, 1:25 p.m. UTC | #6 
On 10/11/2022 21.32, Paul Eggleton via lists.openembedded.org wrote:
> Hi Richard
> 
> On Thursday, 10 November 2022 11:14:24 NZDT Richard Purdie wrote:

>> Adding in yet further if/else paths with magic variables to control
>> them isn't filling me with confidence.
> 
> I understand that. I was hoping to figure out a way to avoid adding significant 
> extra complexity.
>

FWIW, we've long since given up on using kernel-fitimage.bbclass, it is
way to inflexible (e.g. there's no way to build two FIT images with the
same kernel but different initramfs).

It is much simpler to just have a separate small kernel-fit.bb recipe
which pulls vmlinux or Image or whatnot, plus the .dtbs, from
DEPLOY_DIR_IMAGE along with the initramfs images, provide our own .its
file(s), and build the FIT image(s) there. That automatically avoids
rebuilding the kernel when only something in initramfs has changed.

Building the kernel and packaging the binary into some container format
really doesn't need to be done in the same recipe.

Rasmus
Richard Purdie Nov. 11, 2022, 1:35 p.m. UTC | #7 
On Fri, 2022-11-11 at 14:25 +0100, Rasmus Villemoes via
lists.openembedded.org wrote:
> On 10/11/2022 21.32, Paul Eggleton via lists.openembedded.org wrote:
> > Hi Richard
> > 
> > On Thursday, 10 November 2022 11:14:24 NZDT Richard Purdie wrote:
> 
> > > Adding in yet further if/else paths with magic variables to control
> > > them isn't filling me with confidence.
> > 
> > I understand that. I was hoping to figure out a way to avoid adding significant 
> > extra complexity.
> > 
> 
> FWIW, we've long since given up on using kernel-fitimage.bbclass, it is
> way to inflexible (e.g. there's no way to build two FIT images with the
> same kernel but different initramfs).
> 
> It is much simpler to just have a separate small kernel-fit.bb recipe
> which pulls vmlinux or Image or whatnot, plus the .dtbs, from
> DEPLOY_DIR_IMAGE along with the initramfs images, provide our own .its
> file(s), and build the FIT image(s) there. That automatically avoids
> rebuilding the kernel when only something in initramfs has changed.
> 
> Building the kernel and packaging the binary into some container format
> really doesn't need to be done in the same recipe.

I'm open to the idea of changing it to do that as that is how I'd
prefer it worked. I don't use it or know much about the needs people
have so I end up guided by others though...

Cheers,

Richard
Bruce Ashfield Nov. 11, 2022, 6:13 p.m. UTC | #8 
On Fri, Nov 11, 2022 at 8:36 AM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Fri, 2022-11-11 at 14:25 +0100, Rasmus Villemoes via
> lists.openembedded.org wrote:
> > On 10/11/2022 21.32, Paul Eggleton via lists.openembedded.org wrote:
> > > Hi Richard
> > >
> > > On Thursday, 10 November 2022 11:14:24 NZDT Richard Purdie wrote:
> >
> > > > Adding in yet further if/else paths with magic variables to control
> > > > them isn't filling me with confidence.
> > >
> > > I understand that. I was hoping to figure out a way to avoid adding significant
> > > extra complexity.
> > >
> >
> > FWIW, we've long since given up on using kernel-fitimage.bbclass, it is
> > way to inflexible (e.g. there's no way to build two FIT images with the
> > same kernel but different initramfs).
> >
> > It is much simpler to just have a separate small kernel-fit.bb recipe
> > which pulls vmlinux or Image or whatnot, plus the .dtbs, from
> > DEPLOY_DIR_IMAGE along with the initramfs images, provide our own .its
> > file(s), and build the FIT image(s) there. That automatically avoids
> > rebuilding the kernel when only something in initramfs has changed.
> >
> > Building the kernel and packaging the binary into some container format
> > really doesn't need to be done in the same recipe.
>
> I'm open to the idea of changing it to do that as that is how I'd
> prefer it worked. I don't use it or know much about the needs people
> have so I end up guided by others though...

I'm of a like mind.

We've been wanting to re-work and clean up a LOT of the kernel classes
for quite some time, but with all of the different workflows (and lack of tests)
it is hard to get it right, and not break someone's case.

I've been working through Paul's suggested patch, and I agree that it
does try to minimize the copying of the kernel build artifacts, but the
complication to the class and build is inevitable.

I've also done multiple packaging and wrapping of boot artifacts outside
of a single recipe, to avoid both complexity and extra building.

I'm wondering if your classes are something you can cleanup and share ?
That would help the discussion as we could talk about some tangible
new approaches.

Bruce

>
> Cheers,
>
> Richard
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#173129): https://lists.openembedded.org/g/openembedded-core/message/173129
> Mute This Topic: https://lists.openembedded.org/mt/94747626/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/classes-recipe/image.bbclass b/meta/classes-recipe/image.bbclass
index e387645..cedd913 100644
--- a/meta/classes-recipe/image.bbclass
+++ b/meta/classes-recipe/image.bbclass
@@ -137,13 +137,14 @@  def rootfs_variables(d):
 
 do_rootfs[vardeps] += "${@rootfs_variables(d)}"
 
+IMAGE_FITIMAGE_DEPEND = "${@'virtual/kernel:do_deploy_fitimage' if d.getVar('IMAGE_FITIMAGE') == '1' else ''}"
+
 # This is needed to have kernel image in DEPLOY_DIR.
 # This follows many common usecases and user expectations.
 # But if you are building an image which doesn't need the kernel image at all,
 # you can unset this variable manually.
 KERNEL_DEPLOY_DEPEND ?= "virtual/kernel:do_deploy"
-do_build[depends] += "${KERNEL_DEPLOY_DEPEND}"
-
+do_build[depends] += "${KERNEL_DEPLOY_DEPEND} ${IMAGE_FITIMAGE_DEPEND}"
 
 python () {
     def extraimage_getdepends(task):
diff --git a/meta/classes-recipe/kernel-fitimage.bbclass b/meta/classes-recipe/kernel-fitimage.bbclass
index 7980910..40406fc 100644
--- a/meta/classes-recipe/kernel-fitimage.bbclass
+++ b/meta/classes-recipe/kernel-fitimage.bbclass
@@ -731,10 +731,32 @@  do_install:append() {
 	fi
 }
 
+python do_preserve_kernel_build() {
+    archdir = d.getVar('KERNEL_OUTPUT_DIR')
+    src = d.getVar('B')
+    dst = d.getVar('INITRAMFSDIR')
+    oe.path.copyhardlinktree(os.path.join(src, archdir), os.path.join(dst, archdir))
+    for fn in ['vmlinux', 'fit-image.its']:
+        oe.path.copyhardlink(os.path.join(src, fn), os.path.join(dst, fn))
+}
+
+INITRAMFSDIR = "${WORKDIR}/initramfs-work"
+SSTATETASKS += "do_preserve_kernel_build"
+do_preserve_kernel_build[cleandirs] = "${INITRAMFSDIR}"
+do_preserve_kernel_build[sstate-plaindirs] = "${INITRAMFSDIR}"
+
+python do_preserve_kernel_build_setscene () {
+    sstate_setscene(d)
+}
+addtask preserve_kernel_build_setscene
+
+
+addtask preserve_kernel_build after do_bundle_initramfs
+
 do_assemble_fitimage_initramfs() {
 	if echo ${KERNEL_IMAGETYPES} | grep -wq "fitImage" && \
 		test -n "${INITRAMFS_IMAGE}" ; then
-		cd ${B}
+		cd ${INITRAMFSDIR}
 		if [ "${INITRAMFS_IMAGE_BUNDLE}" = "1" ]; then
 			fitimage_assemble fit-image-${INITRAMFS_IMAGE}.its fitImage-bundle ""
 			ln -sf fitImage-bundle ${B}/${KERNEL_OUTPUT_DIR}/fitImage
@@ -744,7 +766,9 @@  do_assemble_fitimage_initramfs() {
 	fi
 }
 
-addtask assemble_fitimage_initramfs before do_deploy after do_bundle_initramfs
+addtask assemble_fitimage_initramfs after do_preserve_kernel_build
+
+do_assemble_fitimage_initramfs[depends] += "virtual/${TARGET_PREFIX}binutils:do_populate_sysroot"
 
 do_kernel_generate_rsa_keys() {
 	if [ "${UBOOT_SIGN_ENABLE}" = "0" ] && [ "${FIT_GENERATE_KEYS}" = "1" ]; then
@@ -793,20 +817,33 @@  do_kernel_generate_rsa_keys() {
 
 addtask kernel_generate_rsa_keys before do_assemble_fitimage after do_compile
 
-kernel_do_deploy[vardepsexclude] = "DATETIME"
-kernel_do_deploy:append() {
+FITIMGDEPLOYDIR = "${WORKDIR}/fitimage-deploy"
+SSTATETASKS += "do_deploy_fitimage"
+do_deploy_fitimage[sstate-inputdirs] = "${FITIMGDEPLOYDIR}"
+do_deploy_fitimage[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}"
+
+python do_deploy_fitimage_setscene () {
+        sstate_setscene(d)
+}
+addtask do_deploy_fitimage_setscene
+
+do_deploy_fitimage[dirs] = "${FITIMGDEPLOYDIR} ${INITRAMFSDIR}"
+
+do_deploy_fitimage[vardepsexclude] = "DATETIME"
+do_deploy_fitimage() {
+	deployDir="${FITIMGDEPLOYDIR}"
 	# Update deploy directory
 	if echo ${KERNEL_IMAGETYPES} | grep -wq "fitImage"; then
 
 		if [ "${INITRAMFS_IMAGE_BUNDLE}" != "1" ]; then
 			bbnote "Copying fit-image.its source file..."
-			install -m 0644 ${B}/fit-image.its "$deployDir/fitImage-its-${KERNEL_FIT_NAME}.its"
+			install -m 0644 ${INITRAMFSDIR}/fit-image.its "$deployDir/fitImage-its-${KERNEL_FIT_NAME}.its"
 			if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
 				ln -snf fitImage-its-${KERNEL_FIT_NAME}.its "$deployDir/fitImage-its-${KERNEL_FIT_LINK_NAME}"
 			fi
 
 			bbnote "Copying linux.bin file..."
-			install -m 0644 ${B}/linux.bin $deployDir/fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}
+			install -m 0644 ${INITRAMFSDIR}/linux.bin $deployDir/fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}
 			if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
 				ln -snf fitImage-linux.bin-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT} "$deployDir/fitImage-linux.bin-${KERNEL_FIT_LINK_NAME}"
 			fi
@@ -814,14 +851,14 @@  kernel_do_deploy:append() {
 
 		if [ -n "${INITRAMFS_IMAGE}" ]; then
 			bbnote "Copying fit-image-${INITRAMFS_IMAGE}.its source file..."
-			install -m 0644 ${B}/fit-image-${INITRAMFS_IMAGE}.its "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its"
+			install -m 0644 ${INITRAMFSDIR}/fit-image-${INITRAMFS_IMAGE}.its "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its"
 			if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
 				ln -snf fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}.its "$deployDir/fitImage-its-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_LINK_NAME}"
 			fi
 
 			if [ "${INITRAMFS_IMAGE_BUNDLE}" != "1" ]; then
 				bbnote "Copying fitImage-${INITRAMFS_IMAGE} file..."
-				install -m 0644 ${B}/${KERNEL_OUTPUT_DIR}/fitImage-${INITRAMFS_IMAGE} "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}"
+				install -m 0644 ${INITRAMFSDIR}/${KERNEL_OUTPUT_DIR}/fitImage-${INITRAMFS_IMAGE} "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT}"
 				if [ -n "${KERNEL_FIT_LINK_NAME}" ] ; then
 					ln -snf fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_NAME}${KERNEL_FIT_BIN_EXT} "$deployDir/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_LINK_NAME}"
 				fi
@@ -829,3 +866,5 @@  kernel_do_deploy:append() {
 		fi
 	fi
 }
+
+addtask do_deploy_fitimage before do_build after do_deploy do_assemble_fitimage_initramfs
diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
index 7bb3449..f2efb3e 100644
--- a/meta/classes-recipe/kernel.bbclass
+++ b/meta/classes-recipe/kernel.bbclass
@@ -140,7 +140,7 @@  set -e
     # If the INTIRAMFS_IMAGE is set but the INITRAMFS_IMAGE_BUNDLE is set to 0,
     # the do_bundle_initramfs does nothing, but the INITRAMFS_IMAGE is built
     # standalone for use by wic and other tools.
-    if image:
+    if image and 'fitImage' not in d.getVar('KERNEL_IMAGETYPES'):
         if d.getVar('INITRAMFS_MULTICONFIG'):
             d.appendVarFlag('do_bundle_initramfs', 'mcdepends', ' mc::${INITRAMFS_MULTICONFIG}:${INITRAMFS_IMAGE}:do_image_complete')
         else: