Re: [PATCH v4 05/10] signal: Introduce TRAP_PERF si_code and si_perf to siginfo

[Marco Elver]

+Cc linux-arm-kernel

On Wed, 21 Apr 2021 at 15:19, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
>
> Hi Marco,
>
> On 21.04.2021 13:03, Marco Elver wrote:
> > On Wed, 21 Apr 2021 at 12:57, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
> >> On 21.04.2021 11:35, Marek Szyprowski wrote:
> >>> On 21.04.2021 10:11, Marco Elver wrote:
> >>>> On Wed, 21 Apr 2021 at 09:35, Marek Szyprowski
> >>>> <m.szyprowski@xxxxxxxxxxx> wrote:
> >>>>> On 21.04.2021 08:21, Marek Szyprowski wrote:
> >>>>>> On 21.04.2021 00:42, Marco Elver wrote:
> >>>>>>> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski
> >>>>>>> <m.szyprowski@xxxxxxxxxxx> wrote:
> >>>>>>>> On 08.04.2021 12:36, Marco Elver wrote:
> >>>>>>>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
> >>>>>>>>> si_perf. These will be used by the perf event subsystem to send
> >>>>>>>>> signals
> >>>>>>>>> (if requested) to the task where an event occurred.
> >>>>>>>>>
> >>>>>>>>> Acked-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> # m68k
> >>>>>>>>> Acked-by: Arnd Bergmann <arnd@xxxxxxxx> # asm-generic
> >>>>>>>>> Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
> >>>>>>>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
> >>>>>>>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
> >>>>>>>> regression on my test systems (arm 32bit and 64bit). Most systems
> >>>>>>>> fails
> >>>>>>>> to boot in the given time frame. I've observed that there is a
> >>>>>>>> timeout
> >>>>>>>> waiting for udev to populate /dev and then also during the network
> >>>>>>>> interfaces configuration. Reverting this commit, together with
> >>>>>>>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to
> >>>>>>>> let it
> >>>>>>>> compile, on top of next-20210420 fixes the issue.
> >>>>>>> Thanks, this is weird for sure and nothing in particular stands out.
> >>>>>>>
> >>>>>>> I have questions:
> >>>>>>> -- Can you please share your config?
> >>>>>> This happens with standard multi_v7_defconfig (arm) or just defconfig
> >>>>>> for arm64.
> >>>>>>
> >>>>>>> -- Also, can you share how you run this? Can it be reproduced in
> >>>>>>> qemu?
> >>>>>> Nothing special. I just boot my test systems and see that they are
> >>>>>> waiting lots of time during the udev populating /dev and network
> >>>>>> interfaces configuration. I didn't try with qemu yet.
> >>>>>>> -- How did you derive this patch to be at fault? Why not just
> >>>>>>> 97ba62b27867, given you also need to revert it?
> >>>>>> Well, I've just run my boot tests with automated 'git bisect' and that
> >>>>>> was its result. It was a bit late in the evening, so I didn't analyze
> >>>>>> it further, I've just posted a report about the issue I've found. It
> >>>>>> looks that bisecting pointed to a wrong commit somehow.
> >>>>>>> If you are unsure which patch exactly it is, can you try just
> >>>>>>> reverting 97ba62b27867 and see what happens?
> >>>>>> Indeed, this is a real faulty commit. Initially I've decided to revert
> >>>>>> it to let kernel compile (it uses some symbols introduced by this
> >>>>>> commit). Reverting only it on top of linux-next 20210420 also fixes
> >>>>>> the issue. I'm sorry for the noise in this thread. I hope we will find
> >>>>>> what really causes the issue.
> >>>>> This was a premature conclusion. It looks that during the test I've did
> >>>>> while writing that reply, the modules were not deployed properly and a
> >>>>> test board (RPi4) booted without modules. In that case the board booted
> >>>>> fine and there was no udev timeout. After deploying kernel modules, the
> >>>>> udev timeout is back.
> >>>> I'm confused now. Can you confirm that the problem is due to your
> >>>> kernel modules, or do you think it's still due to 97ba62b27867? Or
> >>>> fb6cc127e0b6 (this patch)?
> >>> I don't use any custom kernel modules. I just deploy all modules that
> >>> are being built from the given kernel defconfig (arm
> >>> multi_v7_defconfig or arm64 default) and they are automatically loaded
> >>> during the boot by udev. I've checked again and bisect was right. The
> >>> kernel built from fb6cc127e0b6 suffers from the described issue, while
> >>> the one build from the previous commit (2e498d0a74e5) works fine.
> >> I've managed to reproduce this issue with qemu. I've compiled the kernel
> >> for arm 32bit with multi_v7_defconfig and used some older Debian rootfs
> >> image. The log and qemu parameters are here:
> >> https://protect2.fireeye.com/v1/url?k=7cfc23a2-23671aa9-7cfda8ed-002590f5b904-dab7e2ec39dae1f9&q=1&e=36a5ed13-6ad5-430c-8f44-e95c4f0af5c3&u=https%3A%2F%2Fpaste.debian.net%2F1194526%2F
> >>
> >> Check the timestamp for the 'EXT4-fs (vda): re-mounted' message and
> >> 'done (timeout)' status for the 'Waiting for /dev to be fully populated'
> >> message. This happens only when kernel modules build from the
> >> multi_v7_defconfig are deployed on the rootfs.
> > Still hard to say what is going on and what is at fault. But being
> > able to repro this in qemu helps debug quicker -- would you also be
> > able to share the precise rootfs.img, i.e. upload it somewhere I can
> > fetch it? And just to be sure, please also share your .config, as it
> > might have compiler-version dependent configuration that might help
> > repro (unlikely, but you never know).
>
> I've managed to reproduce this issue with a public Raspberry Pi OS Lite
> rootfs image, even without deploying kernel modules:
>
> https://downloads.raspberrypi.org/raspios_lite_armhf/images/raspios_lite_armhf-2021-03-25/2021-03-04-raspios-buster-armhf-lite.zip
>
> # qemu-system-arm -M virt -smp 2 -m 512 -kernel zImage -append "earlycon
> console=ttyAMA0 root=/dev/vda2 rw rootwait" -serial stdio -display none
> -monitor null -device virtio-blk-device,drive=virtio-blk -drive
> file=/tmp/2021-03-04-raspios-buster-armhf-lite.img,id=virtio-blk,if=none,format=raw
> -netdev user,id=user -device virtio-net-device,netdev=user
>
> The above one doesn't boot if zImage z compiled from commit fb6cc127e0b6
> and boots if compiled from 2e498d0a74e5. In both cases I've used default
> arm/multi_v7_defconfig and
> gcc-linaro-6.4.1-2017.11-x86_64_arm-linux-gnueabi toolchain.

Yup, I've narrowed it down to the addition of "__u64 _perf" to
siginfo_t. My guess is the __u64 causes a different alignment for a
bunch of adjacent fields. It seems that x86 and m68k are the only ones
that have compile-time tests for the offsets. Arm should probably add
those -- I have added a bucket of static_assert() in
arch/arm/kernel/signal.c and see that something's off.

I'll hopefully have a fix in a day or so.

Thanks,
-- Marco