Re: [PATCH v2 3/5] rootfs: Unmount rootfs mounts if not needed

[Jan Kiszka <jan.kiszka@siemens.com>]

On 21.04.21 16:58, Anton Mikanovich wrote:
> Count the usage of rootfs mounts and trigger unmount only after all
> corresponded tasks are finished.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
>  meta/classes/image.bbclass                    | 15 ----
>  meta/classes/initramfs.bbclass                |  2 +
>  meta/classes/rootfs.bbclass                   | 68 +++++++++++++++++--
>  .../buildchroot/buildchroot.inc               |  7 ++
>  4 files changed, 70 insertions(+), 22 deletions(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 9f9b3f8..12d1616 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -199,21 +199,6 @@ do_rootfs_finalize() {
>              find "${ROOTFSDIR}/usr/bin" \
>                  -maxdepth 1 -name 'qemu-*-static' -type f -delete
>  
> -        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
> -            umount -l ${ROOTFSDIR}/isar-apt
> -        rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
> -
> -        mountpoint -q '${ROOTFSDIR}/base-apt' && \
> -            umount -l ${ROOTFSDIR}/base-apt
> -        rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
> -
> -        mountpoint -q '${ROOTFSDIR}/dev' && \
> -            umount -l ${ROOTFSDIR}/dev
> -        mountpoint -q '${ROOTFSDIR}/sys' && \
> -            umount -l ${ROOTFSDIR}/proc
> -        mountpoint -q '${ROOTFSDIR}/sys' && \
> -            umount -l ${ROOTFSDIR}/sys
> -
>          rm -f "${ROOTFSDIR}/etc/apt/apt.conf.d/55isar-fallback.conf"
>  
>          rm -f "${ROOTFSDIR}/etc/apt/sources.list.d/isar-apt.list"
> diff --git a/meta/classes/initramfs.bbclass b/meta/classes/initramfs.bbclass
> index 10a642b..e895afa 100644
> --- a/meta/classes/initramfs.bbclass
> +++ b/meta/classes/initramfs.bbclass
> @@ -38,5 +38,7 @@ do_generate_initramfs() {
>  
>      rm -rf "${INITRAMFS_IMAGE_FILE}"
>      cp "${INITRAMFS_ROOTFS}/initrd.img" "${INITRAMFS_IMAGE_FILE}"
> +
> +    rootfs_undo_mounts
>  }
>  addtask generate_initramfs after do_rootfs before do_build
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index f9151c5..7bb04e5 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -26,8 +26,41 @@ export LANG = "C"
>  export LANGUAGE = "C"
>  export LC_ALL = "C"
>  
> +ROOTFS_CNT_LOCKFILE = "${ROOTFSDIR}.mount_lock"
> +
> +rootfs_mount_cnt_inc() {
> +    sudo -s <<'EOSUDO'
> +        ( flock 9
> +        set -e
> +        count="1"
> +        if [ -f '${ROOTFSDIR}/.mount' ]; then
> +            count=$(($(< '${ROOTFSDIR}/.mount') + 1))
> +        fi
> +        echo $count | tee '${ROOTFSDIR}/.mount'
> +        ) 9>'${ROOTFS_CNT_LOCKFILE}'
> +EOSUDO
> +}
> +
> +rootfs_mount_cnt_dec() {
> +    sudo -s <<'EOSUDO'
> +        ( flock 9
> +        set -e
> +        if [ -f '${ROOTFSDIR}/.mount' ]; then
> +            count=$(($(< '${ROOTFSDIR}/.mount') - 1))
> +            echo $count | tee '${ROOTFSDIR}/.mount'
> +        else
> +            exit 1
> +        fi
> +        ) 9>'${ROOTFS_CNT_LOCKFILE}'
> +EOSUDO
> +}
> +
> +
>  rootfs_do_mounts[weight] = "3"
>  rootfs_do_mounts() {
> +    if [ $(rootfs_mount_cnt_inc) -gt 1 ]; then
> +        return
> +    fi
>      sudo -s <<'EOSUDO'
>          mountpoint -q '${ROOTFSDIR}/dev' || \
>              mount --rbind /dev '${ROOTFSDIR}/dev'
> @@ -59,6 +92,26 @@ rootfs_do_mounts() {
>  EOSUDO
>  }
>  
> +rootfs_undo_mounts() {
> +    if [ $(rootfs_mount_cnt_dec) -gt 0 ]; then
> +        return
> +    fi
> +    sudo -s <<'EOSUDO'
> +        mountpoint -q '${ROOTFSDIR}/base-apt' && \
> +            umount ${ROOTFSDIR}/base-apt && \
> +            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
> +        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
> +            umount ${ROOTFSDIR}/isar-apt && \
> +            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
> +        mountpoint -q '${ROOTFSDIR}/sys' && \
> +            umount -R ${ROOTFSDIR}/sys
> +        mountpoint -q '${ROOTFSDIR}/proc' && \
> +            umount ${ROOTFSDIR}/proc
> +        mountpoint -q '${ROOTFSDIR}/dev' && \
> +            umount -R ${ROOTFSDIR}/dev
> +EOSUDO
> +}
> +
>  rootfs_do_qemu() {
>      if [ '${@repr(d.getVar('ROOTFS_ARCH') == d.getVar('HOST_ARCH'))}' = 'False' ]
>      then
> @@ -159,7 +212,7 @@ python do_rootfs_install() {
>  
>      # Mount after configure commands, so that they have time to copy
>      # 'isar-apt' (sdkchroot):
> -    cmds = ['rootfs_prepare'] + configure_cmds + ['rootfs_do_mounts'] + install_cmds
> +    cmds = ['rootfs_prepare'] + configure_cmds + ['rootfs_do_mounts'] + install_cmds  + ['rootfs_undo_mounts']
>  
>      # NOTE: The weights specify how long each task takes in seconds and are used
>      # by the MultiStageProgressReporter to render a progress bar for this task.
> @@ -241,12 +294,13 @@ python do_rootfs_postprocess() {
>      progress_reporter.update(0)
>  
>      cmds = d.getVar("ROOTFS_POSTPROCESS_COMMAND")
> -    if cmds is None or not cmds.strip():
> -        return
> -    cmds = cmds.split()
> -    for i, cmd in enumerate(cmds):
> -        bb.build.exec_func(cmd, d)
> -        progress_reporter.update(int(i / len(cmds) * 100))
> +    if cmds is not None and cmds.strip():
> +        cmds = cmds.split()
> +        for i, cmd in enumerate(cmds):
> +            bb.build.exec_func(cmd, d)
> +            progress_reporter.update(int(i / len(cmds) * 100))
> +
> +    bb.build.exec_func('rootfs_undo_mounts', d)
>  }
>  addtask rootfs_postprocess before do_rootfs
>  
> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
> index 5a2befb..b80e703 100644
> --- a/meta/recipes-devtools/buildchroot/buildchroot.inc
> +++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
> @@ -49,6 +49,13 @@ rootfs_do_mounts_append() {
>  EOSUDO
>  }
>  
> +rootfs_undo_mounts_append() {
> +    sudo -s <<'EOSUDO'
> +    mountpoint -q '${BUILDCHROOT_DIR}/downloads' && \
> +        umount ${BUILDCHROOT_DIR}/downloads
> +EOSUDO
> +}
> +
>  ROOTFS_POSTPROCESS_COMMAND =+ "buildchroot_install_files"
>  buildchroot_install_files() {
>      sudo mkdir -p "${BUILDCHROOT_DIR}/home/builder"
> 

This is simply from running next in CI:

ERROR: mc:de0-nano-soc-buster:buildchroot-host-1.0-r0 do_rootfs_postprocess: Error executing a python function in exec_python_func() autogenerated:
The stack trace of python calls that resulted in this exception/failure was:

[cut useless backtrace]

buster-armhf/buildchroot-host/1.0-r0/temp/run.rootfs_undo_mounts.65491' failed with exit code 32:
umount: /builds/BLKE9aZ_/0/ebsy/debian/isar/build/tmp/work/debian-buster-armhf/buildchroot-host/1.0-r0/rootfs/sys/fs/cgroup: target is busy.
WARNING: exit code 32 from a shell command.
ERROR: Logfile of failure stored in: /builds/BLKE9aZ_/0/ebsy/debian/isar/build/tmp/work/debian-buster-armhf/buildchroot-host/1.0-r0/temp/log.do_rootfs_postprocess.65491
NOTE: recipe buildchroot-host-1.0-r0: task do_rootfs_postprocess: Failed
ERROR: Task (mc:de0-nano-soc-buster:/builds/BLKE9aZ_/0/ebsy/debian/isar/meta/recipes-devtools/buildchroot/buildchroot-host.bb:do_rootfs_postprocess) failed with exit code '1'

It's analogous to https://groups.google.com/d/msgid/isar-users/87cef6ee-411c-b611-4fb4-3957573d9455%40siemens.com, just clearly pointing to this series.

What is the status of fixing all this? I don't see patches on the list, 
while the umounting rework is negatively impacting next since April. 
With all theses regressions, we cannot properly test unrelated fixes and 
new features, not to speak of updating downstream layer to newer Isar.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux