Re: [RFC 1/1] delay creation of initrd until end of rootfs install

[Gylstorff Quirin <quirin.gylstorff@siemens.com>]

On 1/18/24 6:06 PM, 'Jan Kiszka' via isar-users wrote:
> On 18.01.24 17:44, Moessbauer, Felix (T CED OES-DE) wrote:
>> On Fri, 2023-12-08 at 10:46 +0000, 'MOESSBAUER, Felix' via isar-users
>> wrote:
>>> On Wed, 2023-12-06 at 09:04 +0900, Jan Kiszka wrote:
>>>> On 04.12.23 10:36, Felix Moessbauer wrote:
>>>>> This patch solves major performance issues around the initramfs
>>>>> creation by ensuring that the initrd is only created once. This
>>>>> is
>>>>> implemented by stubbing the update-initramfs call during the
>>>>> package
>>>>> installing. After all apt operations are completed, we manually
>>>>> trigger the initrd creation. In case a custom initramfs is used,
>>>>> the
>>>>> creation is completely skipped in the image rootfs, as this would
>>>>> anyways not be used.
>>>>>
>>>>> Before that, each package install that made a initrd relevant
>>>>> change
>>>>> triggered the update of the initrd. As we have multiple apt calls
>>>>> during
>>>>> the build, this step was sometimes executed multiple times. In
>>>>> addition,
>>>>> the apt install step is emulated, further slowing down the initrd
>>>>> generation. On some layers on non native architecutes, this
>>>>> summed
>>>>> up to
>>>>> over 10 minutes of initrd generation time.
>>>>>
>>>>> Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
>>>>> ---
>>>>>   meta/classes/image.bbclass                    |  4 +++
>>>>>   meta/classes/rootfs.bbclass                   | 32
>>>>> +++++++++++++++++++
>>>>>   .../isar-bootstrap/isar-bootstrap.inc         |  2 ++
>>>>>   3 files changed, 38 insertions(+)
>>>>>
>>>>> diff --git a/meta/classes/image.bbclass
>>>>> b/meta/classes/image.bbclass
>>>>> index 73f1d52c..39addc59 100644
>>>>> --- a/meta/classes/image.bbclass
>>>>> +++ b/meta/classes/image.bbclass
>>>>> @@ -72,6 +72,8 @@ inherit essential
>>>>>   
>>>>>   ROOTFSDIR = "${IMAGE_ROOTFS}"
>>>>>   ROOTFS_FEATURES += "clean-package-cache clean-pycache generate-
>>>>> manifest export-dpkg-status clean-log-files clean-debconf-cache"
>>>>> +# when using a custom initrd, do not generate one as part of the
>>>>> image rootfs
>>>>> +ROOTFS_FEATURES += "${@ '' if d.getVar('INITRD_IMAGE') == ''
>>>>> else
>>>>> 'no-generate-initrd'}"
>>>>>   ROOTFS_PACKAGES += "${IMAGE_PREINSTALL}
>>>>> ${@isar_multiarch_packages('IMAGE_INSTALL', d)}"
>>>>>   ROOTFS_MANIFEST_DEPLOY_DIR ?= "${DEPLOY_DIR_IMAGE}"
>>>>>   ROOTFS_DPKGSTATUS_DEPLOY_DIR ?= "${DEPLOY_DIR_IMAGE}"
>>>>> @@ -488,6 +490,8 @@ do_rootfs_quality_check() {
>>>>>       args="${ROOTFS_QA_FIND_ARGS}"
>>>>>       # rootfs_finalize chroot-setup.sh
>>>>>       args="${args} ! -path ${ROOTFSDIR}/var/lib/dpkg/diversions"
>>>>> +    # initramfs is generated outside of the image rootfs
>>>>> +    args="${args} ! -path ${ROOTFSDIR}/boot/initrd.img*"
>>>>>       for cmd in ${ROOTFS_POSTPROCESS_COMMAND}; do
>>>>>           case "${cmd}" in
>>>>>              image_postprocess_mark)
>>>>> diff --git a/meta/classes/rootfs.bbclass
>>>>> b/meta/classes/rootfs.bbclass
>>>>> index 1b95115a..69e38dac 100644
>>>>> --- a/meta/classes/rootfs.bbclass
>>>>> +++ b/meta/classes/rootfs.bbclass
>>>>> @@ -14,6 +14,7 @@ ROOTFS_BASE_DISTRO ?= "${BASE_DISTRO}"
>>>>>   # 'generate-manifest' - generate a package manifest of the
>>>>> rootfs
>>>>> into ${ROOTFS_MANIFEST_DEPLOY_DIR}
>>>>>   # 'export-dpkg-status' - exports /var/lib/dpkg/status file to
>>>>> ${ROOTFS_DPKGSTATUS_DEPLOY_DIR}
>>>>>   # 'clean-log-files' - delete log files that are not owned by
>>>>> packages
>>>>> +# 'no-generate-initrd' - do not generate debian default initrd
>>>>>   ROOTFS_FEATURES ?= ""
>>>>>   
>>>>>   ROOTFS_APT_ARGS="install --yes -o Debug::pkgProblemResolver=yes"
>>>>> @@ -117,6 +118,16 @@ rootfs_configure_apt() {
>>>>>   EOSUDO
>>>>>   }
>>>>>   
>>>>> +ROOTFS_CONFIGURE_COMMAND += "rootfs_disable_initrd_generation"
>>>>> +rootfs_disable_initrd_generation[weight] = "1"
>>>>> +rootfs_disable_initrd_generation() {
>>>>> +    # fully disable initrd generation
>>>>> +    echo "replace update-initramfs with stub"
>>>>> +    sudo mv "${ROOTFSDIR}/usr/sbin/update-initramfs" \
>>>>> +            "${ROOTFSDIR}/usr/sbin/update-initramfs.isar"
>>>>> +    sudo chroot "${ROOTFSDIR}" ln -s "/usr/bin/true"
>>>>> "/usr/sbin/update-initramfs"
>>>>> +}
>>>>> +
>>>>>   
>>>>>   ROOTFS_INSTALL_COMMAND += "rootfs_install_pkgs_update"
>>>>>   rootfs_install_pkgs_update[weight] = "5"
>>>>> @@ -310,6 +321,27 @@ rootfs_cleanup_isar_apt() {
>>>>>   EOSUDO
>>>>>   }
>>>>>   
>>>>> +ROOTFS_POSTPROCESS_COMMAND += "rootfs_restore_initrd_tooling"
>>>>> +rootfs_generate_initrd[weight] = "1"
>>>>> +rootfs_restore_initrd_tooling() {
>>>>> +    if [ -e "${ROOTFSDIR}/usr/sbin/update-initramfs.isar" ];
>>>>> then
>>>>> +        sudo mv -f "${ROOTFSDIR}/usr/sbin/update-initramfs.isar"
>>>>> \
>>>>> +            "${ROOTFSDIR}/usr/sbin/update-initramfs"
>>>>> +    fi
>>>>> +}
>>>>> +
>>>>> +ROOTFS_POSTPROCESS_COMMAND +=
>>>>> "${@bb.utils.contains('ROOTFS_FEATURES', 'no-generate-initrd',
>>>>> '',
>>>>> 'rootfs_generate_initrd', d)}"
>>>>> +rootfs_generate_initrd[weight] = "10"
>>>>> +rootfs_generate_initrd() {
>>>>> +    if [ -n "$(sudo find '${ROOTFSDIR}/boot' -type f -name
>>>>> 'vmlinu[xz]*')" ]; then
>>>>> +        sudo -E chroot "${ROOTFSDIR}" sh -c '\
>>>>> +            export kernel_version=$(basename /boot/vmlinu[xz]* |
>>>>> cut -d'-' -f2-); \
>>>>> +            update-initramfs -u -v -k "$kernel_version";'
>>>>> +    else
>>>>> +        echo "no kernel in this rootfs, do not generate initrd"
>>>>> +    fi
>>>>> +}
>>>>> +
>>>>>   do_rootfs_postprocess[vardeps] = "${ROOTFS_POSTPROCESS_COMMAND}"
>>>>>   do_rootfs_postprocess[network] = "${TASK_USE_SUDO}"
>>>>>   python do_rootfs_postprocess() {
>>>>> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>>>>> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>>>>> index 3477c2fb..4c6011bc 100644
>>>>> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>>>>> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>>>>> @@ -24,6 +24,8 @@ DISTRO_BOOTSTRAP_KEYFILES = ""
>>>>>   THIRD_PARTY_APT_KEYFILES = ""
>>>>>   DEPLOY_ISAR_BOOTSTRAP ?= ""
>>>>>   DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
>>>>> +# install early, so we can stub the update-initramfs script
>>>>> before
>>>>> rootfs install
>>>>> +DISTRO_BOOTSTRAP_BASE_PACKAGES:append = ",initramfs-tools"
>>>>>   DISTRO_BOOTSTRAP_BASE_PACKAGES:append:gnupg = ",gnupg"
>>>>>   DISTRO_BOOTSTRAP_BASE_PACKAGES:append:https-support = ",ca-
>>>>> certificates"
>>>>>   DISTRO_VARS_PREFIX ?= "${@'HOST_' if
>>>>> d.getVar('BOOTSTRAP_FOR_HOST') == '1' else ''}"
>>>>
>>>> The idea is quite nice for us in Isar, though I'd also eventually
>>>> like
>>>> to improve Debian itself in this regard. The building blocks are
>>>> there
>>>> in upstream but they do not fully work.
>>>
>>> Yes, definitely. I reported this on the debian-kernel ML as well:
>>> https://lists.debian.org/debian-kernel/2023/12/msg00097.html
>>>
>>> Anyways, are there more objections against this patch?
>>
>> Any news on this? This is on the ML for quite some time without
>> objections. If required, I can send a rebased version.
>>
> 
> I assume this is held back now by the current -rc cycle. Likely an
> improvement after 0.10.
> 
> Jan
> 

As 0.10 was released,  can we merge this?

Quirin