[PATCH] mount: Cleanup reference counters before build

[PATCH] mount: Cleanup reference counters before build

[Anton Mikanovich]

Reference counters are not cleared in case unsuccessful build happens.
So we need to force cleanup before the build.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/isar-events.bbclass | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index 73419b4..2f8bf6e 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -8,6 +8,8 @@ addhandler build_started
 
 python build_started() {
     bb.utils.remove(d.getVar('TMPDIR') + "/work/*/*/*/temp/once.*")
+    bb.utils.remove(d.getVar('TMPDIR') + "/work/*/*/*/rootfs.mount")
+    bb.utils.remove(d.getVar('TMPDIR') + "/deploy/buildchroot-*/*.mount")
 }
 build_started[eventmask] = "bb.event.BuildStarted"
 
-- 
2.20.1

RE: [PATCH] mount: Cleanup reference counters before build

[Moessbauer, Felix]

Hi Anton,

this patch solved the build issues at least for my use-case.
While it required me to add the buildchroot_do_mounts / buildchroot_undo_mounts to custom tasks, I guess that is the new intended behavior.

There should be a BIG warning somewhere in the documentation that this might require changes to (ill-formed) recipes.

Best regards,
Felix

> -----Original Message-----
> From: isar-users@googlegroups.com <isar-users@googlegroups.com> On
> Behalf Of Anton Mikanovich
> Sent: Wednesday, June 23, 2021 1:58 PM
> To: isar-users@googlegroups.com
> Cc: Anton Mikanovich <amikan@ilbers.de>
> Subject: [PATCH] mount: Cleanup reference counters before build
> 
> Reference counters are not cleared in case unsuccessful build happens.
> So we need to force cleanup before the build.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
>  meta/classes/isar-events.bbclass | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
> index 73419b4..2f8bf6e 100644
> --- a/meta/classes/isar-events.bbclass
> +++ b/meta/classes/isar-events.bbclass
> @@ -8,6 +8,8 @@ addhandler build_started
> 
>  python build_started() {
>      bb.utils.remove(d.getVar('TMPDIR') + "/work/*/*/*/temp/once.*")
> +    bb.utils.remove(d.getVar('TMPDIR') + "/work/*/*/*/rootfs.mount")
> +    bb.utils.remove(d.getVar('TMPDIR') + "/deploy/buildchroot-*/*.mount")
>  }
>  build_started[eventmask] = "bb.event.BuildStarted"
> 
> --
> 2.20.1
> 
> --
> You received this message because you are subscribed to the Google Groups
> "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email
> to isar-users+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.g
> oogle.com%2Fd%2Fmsgid%2Fisar-users%2F20210623115823.136514-1-
> amikan%2540ilbers.de&amp;data=04%7C01%7Cfelix.moessbauer%40siemens.c
> om%7Ccb2b9bb12a8c440d39f308d9363e3b61%7C38ae3bcd95794fd4addab42e
> 1495d55a%7C1%7C0%7C637600463175992733%7CUnknown%7CTWFpbGZsb3d
> 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> %7C1000&amp;sdata=XJvU0n32Mh0DrWQXD%2B18VEenK5t2e8lKDLLkyy1tamg
> %3D&amp;reserved=0.

Re: [PATCH] mount: Cleanup reference counters before build

[Anton Mikanovich]

[-- Attachment #1: Type: text/plain, Size: 2742 bytes --]

Thanks for informing, I hope Jan's issues will be also fixed.

Isar documentation needs some rebuilding. I will prepare appropriate 
patchset.

25.06.2021 11:19, Moessbauer, Felix wrote:
> Hi Anton,
>
> this patch solved the build issues at least for my use-case.
> While it required me to add the buildchroot_do_mounts / buildchroot_undo_mounts to custom tasks, I guess that is the new intended behavior.
>
> There should be a BIG warning somewhere in the documentation that this might require changes to (ill-formed) recipes.
>
> Best regards,
> Felix
>
>> -----Original Message-----
>> From: isar-users@googlegroups.com <isar-users@googlegroups.com> On
>> Behalf Of Anton Mikanovich
>> Sent: Wednesday, June 23, 2021 1:58 PM
>> To: isar-users@googlegroups.com
>> Cc: Anton Mikanovich <amikan@ilbers.de>
>> Subject: [PATCH] mount: Cleanup reference counters before build
>>
>> Reference counters are not cleared in case unsuccessful build happens.
>> So we need to force cleanup before the build.
>>
>> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
>> ---
>>   meta/classes/isar-events.bbclass | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
>> index 73419b4..2f8bf6e 100644
>> --- a/meta/classes/isar-events.bbclass
>> +++ b/meta/classes/isar-events.bbclass
>> @@ -8,6 +8,8 @@ addhandler build_started
>>
>>   python build_started() {
>>       bb.utils.remove(d.getVar('TMPDIR') + "/work/*/*/*/temp/once.*")
>> +    bb.utils.remove(d.getVar('TMPDIR') + "/work/*/*/*/rootfs.mount")
>> +    bb.utils.remove(d.getVar('TMPDIR') + "/deploy/buildchroot-*/*.mount")
>>   }
>>   build_started[eventmask] = "bb.event.BuildStarted"
>>
>> --
>> 2.20.1
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "isar-users" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email
>> to isar-users+unsubscribe@googlegroups.com.
>> To view this discussion on the web visit
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.g
>> oogle.com%2Fd%2Fmsgid%2Fisar-users%2F20210623115823.136514-1-
>> amikan%2540ilbers.de&amp;data=04%7C01%7Cfelix.moessbauer%40siemens.c
>> om%7Ccb2b9bb12a8c440d39f308d9363e3b61%7C38ae3bcd95794fd4addab42e
>> 1495d55a%7C1%7C0%7C637600463175992733%7CUnknown%7CTWFpbGZsb3d
>> 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
>> %7C1000&amp;sdata=XJvU0n32Mh0DrWQXD%2B18VEenK5t2e8lKDLLkyy1tamg
>> %3D&amp;reserved=0.

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov


[-- Attachment #2: Type: text/html, Size: 4028 bytes --]

Re: [PATCH] mount: Cleanup reference counters before build

[Jan Kiszka]

On 25.06.21 13:25, Anton Mikanovich wrote:
> Thanks for informing, I hope Jan's issues will be also fixed.
> 
> Isar documentation needs some rebuilding. I will prepare appropriate
> patchset.
> 

The issue might be fixed (I didn't find time to test in my setup yet),
but I'm still doubting the general usefulness of this rework.

We need a clearer picture what it fixes, what it possibly can't fix (I
doubt it can fix all error cases), and if we still want it then. Maybe
start with documenting the API changes and their reasoning.

Also describe more clearly how to reproduce the issues you were seeing
and trying to fix. That is key to understand if the new solution is
actually a solution or just shifting the problem from left to right.

Please also study the history of the current solution (master), how we
got there (was surely not straight) and why we may not have taken other
paths back then (e.g. the fact that you cannot umount when something is
still running in the background).

Meanwhile, I'm strongly voting for reverting things in next to master.

Jan

> 25.06.2021 11:19, Moessbauer, Felix wrote:
>> Hi Anton,
>>
>> this patch solved the build issues at least for my use-case.
>> While it required me to add the buildchroot_do_mounts / buildchroot_undo_mounts to custom tasks, I guess that is the new intended behavior.
>>
>> There should be a BIG warning somewhere in the documentation that this might require changes to (ill-formed) recipes.
>>
>> Best regards,
>> Felix
>>
>>> -----Original Message-----
>>> From: isar-users@googlegroups.com <isar-users@googlegroups.com> On
>>> Behalf Of Anton Mikanovich
>>> Sent: Wednesday, June 23, 2021 1:58 PM
>>> To: isar-users@googlegroups.com
>>> Cc: Anton Mikanovich <amikan@ilbers.de>
>>> Subject: [PATCH] mount: Cleanup reference counters before build
>>>
>>> Reference counters are not cleared in case unsuccessful build happens.
>>> So we need to force cleanup before the build.
>>>
>>> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
>>> ---
>>>  meta/classes/isar-events.bbclass | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
>>> index 73419b4..2f8bf6e 100644
>>> --- a/meta/classes/isar-events.bbclass
>>> +++ b/meta/classes/isar-events.bbclass
>>> @@ -8,6 +8,8 @@ addhandler build_started
>>>
>>>  python build_started() {
>>>      bb.utils.remove(d.getVar('TMPDIR') + "/work/*/*/*/temp/once.*")
>>> +    bb.utils.remove(d.getVar('TMPDIR') + "/work/*/*/*/rootfs.mount")
>>> +    bb.utils.remove(d.getVar('TMPDIR') + "/deploy/buildchroot-*/*.mount")
>>>  }
>>>  build_started[eventmask] = "bb.event.BuildStarted"
>>>
>>> --
>>> 2.20.1
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "isar-users" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email
>>> to isar-users+unsubscribe@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.g
>>> oogle.com%2Fd%2Fmsgid%2Fisar-users%2F20210623115823.136514-1-
>>> amikan%2540ilbers.de&amp;data=04%7C01%7Cfelix.moessbauer%40siemens.c
>>> om%7Ccb2b9bb12a8c440d39f308d9363e3b61%7C38ae3bcd95794fd4addab42e
>>> 1495d55a%7C1%7C0%7C637600463175992733%7CUnknown%7CTWFpbGZsb3d
>>> 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
>>> %7C1000&amp;sdata=XJvU0n32Mh0DrWQXD%2B18VEenK5t2e8lKDLLkyy1tamg
>>> %3D&amp;reserved=0.
> 
> -- 
> Anton Mikanovich
> Promwad Ltd.
> External service provider of ilbers GmbH
> Maria-Merian-Str. 8
> 85521 Ottobrunn, Germany
> +49 (89) 122 67 24-0
> Commercial register Munich, HRB 214197
> General Manager: Baurzhan Ismagulov
> 

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

Re: [PATCH] mount: Cleanup reference counters before build

[Anton Mikanovich]

29.06.2021 16:49, Jan Kiszka wrote:
> We need a clearer picture what it fixes, what it possibly can't fix (I
> doubt it can fix all error cases), and if we still want it then. Maybe
> start with documenting the API changes and their reasoning.
>
> Also describe more clearly how to reproduce the issues you were seeing
> and trying to fix. That is key to understand if the new solution is
> actually a solution or just shifting the problem from left to right.

To reproduce the original issue (fixed by `Rebuild mount logic` 
patchset), a task running twice for two targets (linux-mainline in our 
case) should fail, the second task run should start and succeed:

To have the original issue fixed by `Rebuild mount logic` patchset be 
reproduced you need one recipe to be included into two targets, and this 
recipe should also failed on the first build attempt, but succeed on 
second one. Moreover, this issue will be reproduced only in case the 
second run will start directly after the fail:

 >bitbake mc:de0-nano-soc-buster:isar-image-base 
mc:stm32mp15x-buster:isar-image-base
 >...
 >ERROR: Logfile of failure stored in: 
/opt/isar/build/tmp/work/debian-buster-armhf/linux-mainline/5.4.70-r0/temp/log.do_dpkg_build.12459
 >NOTE: recipe linux-mainline-5.4.70-r0: task do_dpkg_build: Failed
 >ERROR: Task 
(mc:stm32mp15x-buster:/opt/isar/meta-isar/recipes-kernel/linux/linux-mainline_5.4.70.bb:do_dpkg_build) 
failed with exit code '1'
 >NOTE: Running task 304 of 350 
(mc:de0-nano-soc-buster:/opt/isar/meta-isar/recipes-kernel/linux/linux-mainline_5.4.70.bb:do_dpkg_build)
 >NOTE: recipe linux-mainline-5.4.70-r0: task do_dpkg_build: Started
 >WARNING: mc:de0-nano-soc-buster:linux-mainline-5.4.70-r0 
do_dpkg_build: 
/opt/isar/build/tmp/deploy/buildchroot-target/debian-buster-armhf//home/builder/linux-mainline: 
Couldn't unmount, retrying...

In most cases second one does not start and bitbake exit due to the 
error, but when it start the build is went to the infinite loop.

In that scenario:
1) mc:stm32mp15x-buster:linux-mainline:do_dpkg_build perform mounting of 
WORKDIR to buildchroot-target/WORKDIR, then failed and leave WORKDIR to 
be mounted.
2) mc:de0-nano-soc-buster:linux-mainline:do_dpkg_build perform the same 
mount again which lead to circullar mount 
WORKDIR->WORKDIR->buildchroot-target/WORKDIR, then package build is 
performed without errors.
3) mc:de0-nano-soc-buster:linux-mainline:do_dpkg_build stuck inside 
dpkg_undo_mounts. When package build finish it will put 
run.dpkg_undo_mounts.PID shell function into WORKDIR/temp and execute. 
But when run.dpkg_undo_mounts.PID try to unmount 
buildchroot-target/WORKDIR the kernel will also unmount circullar mount 
to WORKDIR which is locked by run.dpkg_undo_mounts.PID itself.

This scenario can be demonstrated with the following steps:

$ mkdir dirA
$ mkdir dirB
$ echo "sudo umount /home/user/dirB" > dirA/um
$ sudo mount --bind dirA dirB
$ mount | grep dir
/dev/sdb1 on /home/user/dirB type ext4 (rw,relatime)
$ sudo mount --bind dirA dirB
$ mount | grep dir
/dev/sdb1 on /home/user/dirB type ext4 (rw,relatime)
/dev/sdb1 on /home/user/dirB type ext4 (rw,relatime)
/dev/sdb1 on /home/user/dirA type ext4 (rw,relatime)
$ bash ~/dirA/um
umount: /home/user/dirB: target is busy.

So mount rebuild patchset actually make dpkg_undo_mounts to be always 
run even after build fail and not allow double mounting.
It was done only for dpkg_do_mounts/dpkg_undo_mounts because only for 
this scenario double mount is critical and can lead to dead lock. 
Implementation can be completed for all the mounts if needed.

We understand that the API change is quite significant. I'm checking 
whether the final unmounting could be solved without change the API.


P.S. in case anyone will try to reproduce the original issue this patch 
can help:

diff --git a/meta-isar/recipes-kernel/linux/linux-mainline_5.4.70.bb 
b/meta-isar/recipes-kernel/linux/linux-mainline_5.4.70.bb
index 209ad9c..49b9739 100644
--- a/meta-isar/recipes-kernel/linux/linux-mainline_5.4.70.bb
+++ b/meta-isar/recipes-kernel/linux/linux-mainline_5.4.70.bb
@@ -32,3 +32,11 @@ dpkg_configure_kernel_append() {
      grep "CONFIG_ROOT_NFS=y" ${S}/${KERNEL_BUILD_DIR}/.config || \
          bbfatal "Self-check failed: CONFIG_ROOT_NFS not enabled"
  }
+
+dpkg_runbuild_prepend() {
+    if [ ! -e "${WORKDIR}/stampfile" ]; then
+        touch "${WORKDIR}/stampfile"
+        sleep 5
+        exit 1
+    fi
+}

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov

Re: [PATCH] mount: Cleanup reference counters before build

[Anton Mikanovich]

23.06.2021 14:58, Anton Mikanovich wrote:
> Reference counters are not cleared in case unsuccessful build happens.
> So we need to force cleanup before the build.
>
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>

Applied to next.

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov