Message ID | 20240813165250.2717650-1-peter.maydell@linaro.org (mailing list archive) |
---|---|
Headers | show |
Series | s390: Convert virtio-ccw, cpu to three-phase reset, and followup cleanup | expand |
Am 13.08.24 um 18:52 schrieb Peter Maydell: > The main aim of this patchseries is to remove the two remaining uses > of device_class_set_parent_reset() in the tree, which are virtio-ccw > and the s390 CPU class. Doing that lets us do some followup cleanup. > (The diffstat looks alarming but is almost all coccinelle automated > changes.) > > virtio-ccw is a simple conversion, because it's a leaf class and no > other code interacts with the reset function. So we convert to > three-phase and resettable_class_set_parent_phases(). > > The s390 CPU is trickier, because s390 has its own set of different > kinds of reset (the S390_CPU_RESET_* enum values). At the moment we > have a single function implementing the reset, which always chains to > the parent DeviceClass::reset, and which can be called both from the > CPU object's own DeviceClass::reset method and also from other s390 > code. > > I opted to handle this by adding two new S390-CPU-specific reset types > to the Resettable ResetType enum. Now instead of having an underlying > implementation of reset that is s390-specific and which might be > called either directly or via the DeviceClass::reset method, we can > implement only the Resettable hold phase method, and have the places > that need to trigger an s390-specific reset type do so by calling > resettable_reset(). > > The other option would have been to smuggle in the s390 reset type > via, for instance, a field in the CPU state that we set in > s390_do_cpu_initial_reset() etc and then examined in the reset method, > but doing it this way seems cleaner. > > The rest of the series is largely cleanup. Some small stuff: > * remove the now-unused device_class_set_parent_reset() function > * remove some CPU state struct fields in alpha and hppa that > were simply unused > * fix an accidental comma in xilinx_axidma that confused a > Coccinelle script > * hw/remote/message.c should not be directly invoking the > DeviceClass::reset method > > There are also patches here which convert devices from directly > setting: > dc->reset = my_reset_function; > to using a function call: > device_class_set_legacy_reset(dc, my_reset_function); > > The conversion is entirely scripted using a coccinelle patch, and it > is responsible for the large diffstat here. > > If people feel this is unnecessary churn I'm open to that argument, > but my motivation here is: > * it means that people writing device code see the "legacy" word > and get a hint that a new QEMU device probably should be using > something else (i.e. Resettable) > * it makes it easier to find the places in the code that are > still using legacy-reset with a simple grep > > In particular, having done that makes it easier to rename the > DeviceState::reset field to DeviceState::legacy_reset, and that in > turn is how I found that hw/remote/ was still directly invoking it. > > The last patch removes device_phases_reset(), which was the part of > the reset transition glue that supported "device is reset via the > legacy reset method but it is a three-phase device". Now we know > there is no way to directly invoke legacy-reset any more, we can drop > that bit of glue. (The opposite direction, "device is reset via a > three-phase-aware method but only implements the legacy reset method", > is still supported.) > > I am pondering the idea of a further Coccinelle-driven conversion > of device_class_set_legacy_reset() calls into three-phase setting > of the Resettable hold phase method. This would be a zero behaviour > change, but the difficult part will be where we have devices > which make direct function calls to their reset function as well > as registering it as the reset method; I'm not sure if Coccinelle > can do "match functions which are not referenced elsewhere in the > file" checks. > > Note that my testing here has only been "make check" and > "make check-avocado", which I know doesn't really exercise reset > very heavily. Nina, can you have a look at those patches?
On Wed, 14 Aug 2024 at 15:22, Christian Borntraeger <borntraeger@linux.ibm.com> wrote: > > Am 13.08.24 um 18:52 schrieb Peter Maydell: > > The main aim of this patchseries is to remove the two remaining uses > > of device_class_set_parent_reset() in the tree, which are virtio-ccw > > and the s390 CPU class. Doing that lets us do some followup cleanup. > > (The diffstat looks alarming but is almost all coccinelle automated > > changes.) > > > > Note that my testing here has only been "make check" and > > "make check-avocado", which I know doesn't really exercise reset > > very heavily. > Nina, can you have a look at those patches? If you plan to do any testing you'll want to locally fix the silly mistake I made in patch 2 (a RESET_TYPE_COLD where it should say RESET_TYPE_S390_CPU_NORMAL) -- see the review comments on that patch. Sorry about that one... thanks -- PMM
On Wed, 2024-08-14 at 21:06 +0100, Peter Maydell wrote: > On Wed, 14 Aug 2024 at 15:22, Christian Borntraeger > <borntraeger@linux.ibm.com> wrote: > > > > Am 13.08.24 um 18:52 schrieb Peter Maydell: > > > The main aim of this patchseries is to remove the two remaining uses > > > of device_class_set_parent_reset() in the tree, which are virtio-ccw > > > and the s390 CPU class. Doing that lets us do some followup cleanup. > > > (The diffstat looks alarming but is almost all coccinelle automated > > > changes.) > > > > > > Note that my testing here has only been "make check" and > > > "make check-avocado", which I know doesn't really exercise reset > > > very heavily. > > > Nina, can you have a look at those patches? > > If you plan to do any testing you'll want to locally fix the > silly mistake I made in patch 2 (a RESET_TYPE_COLD where > it should say RESET_TYPE_S390_CPU_NORMAL) -- see the review > comments on that patch. Sorry about that one... > > thanks > -- PMM > I'll run it through our CI and see if anything pops up.
Quoting Nina Schoetterl-Glausch (2024-08-22 12:34:14)
> I'll run it through our CI and see if anything pops up.
Nina is on holiday, she asked me to quickly report back.
There was a little hickup without the fixup to patch 2, but after Nina
pushed the fixup, we did not observe any failures related to your
changes in our CI. Thanks!
Quoting Nico Boehr (2024-08-26 14:08:20) > There was a little hickup without the fixup to patch 2, but after Nina > pushed the fixup, we did not observe any failures related to your > changes in our CI. Thanks! Peter, after a few CI runs, we unfortunately did find some issues with your patch :-( Rebooting a guest in a loop sometimes fails. Michael was able to bisect it to your series. The problem is intermittent. The guest is unable to load its initramfs: [ 0.560674] rootfs image is not initramfs (no cpio magic); looks like an initrd [ 0.588605] Freeing initrd memory: 95680K [ 0.593143] md: Waiting for all devices to be available before autodetect [ 0.593144] md: If you don't use raid, use raid=noautodetect [ 0.593145] md: Autodetecting RAID arrays. [ 0.593146] md: autorun ... [ 0.593147] md: ... autorun DONE. [ 0.593156] RAMDISK: gzip image found at block 0 [ 0.609110] RAMDISK: incomplete write (29120 != 32768) [ 0.609113] write error ...and then a panic because the kernel doesn't find a rootfs. It seems like the compressed initramfs is corrupted somehow, since "rootfs image is not initramfs" doesn't appear on a successful boot. initramfs and kernel are loaded via direct kernel boot. Running under KVM. Some vhost error messages do appear before the guest panics, but it is not entirely clear to me whether they are related: [...] 2024-08-28T06:56:29.765324Z qemu-system-s390x: vhost vring error in virtqueue 0: Invalid argument (22) 2024-08-28T06:56:32.210982Z qemu-system-s390x: vhost vring error in virtqueue 0: Invalid argument (22) 2024-08-28 06:56:35.430+0000: panic s390: core='0' psw-mask='0x0002000180000000' psw-addr='0x00000387b028c67e' reason='disabled-wait' Any idea?
On Wed, 28 Aug 2024 at 09:13, Nico Boehr <nrb@linux.ibm.com> wrote: > > Quoting Nico Boehr (2024-08-26 14:08:20) > > There was a little hickup without the fixup to patch 2, but after Nina > > pushed the fixup, we did not observe any failures related to your > > changes in our CI. Thanks! > > Peter, after a few CI runs, we unfortunately did find some issues with your > patch :-( > > Rebooting a guest in a loop sometimes fails. Michael was able to bisect it > to your series. > > The problem is intermittent. The guest is unable to load its initramfs: > > [ 0.560674] rootfs image is not initramfs (no cpio magic); looks like an initrd > [ 0.588605] Freeing initrd memory: 95680K > [ 0.593143] md: Waiting for all devices to be available before autodetect > [ 0.593144] md: If you don't use raid, use raid=noautodetect > [ 0.593145] md: Autodetecting RAID arrays. > [ 0.593146] md: autorun ... > [ 0.593147] md: ... autorun DONE. > [ 0.593156] RAMDISK: gzip image found at block 0 > [ 0.609110] RAMDISK: incomplete write (29120 != 32768) > [ 0.609113] write error > > ...and then a panic because the kernel doesn't find a rootfs. > > It seems like the compressed initramfs is corrupted somehow, since "rootfs > image is not initramfs" doesn't appear on a successful boot. > > initramfs and kernel are loaded via direct kernel boot. Running under KVM. > > Some vhost error messages do appear before the guest panics, but it is not > entirely clear to me whether they are related: > > [...] > 2024-08-28T06:56:29.765324Z qemu-system-s390x: vhost vring error in virtqueue 0: Invalid argument (22) > 2024-08-28T06:56:32.210982Z qemu-system-s390x: vhost vring error in virtqueue 0: Invalid argument (22) > 2024-08-28 06:56:35.430+0000: panic s390: core='0' psw-mask='0x0002000180000000' psw-addr='0x00000387b028c67e' reason='disabled-wait' > > Any idea? Well, the series is *supposed* to be just a refactoring, not a change of behaviour, so I'm not sure. I don't suppose you have a reproduce case that I can run? (I do have access to an s390 machine if that helps.) thanks -- PMM
Quoting Peter Maydell (2024-08-28 17:46:42) [...] > Well, the series is *supposed* to be just a refactoring, not a change of > behaviour, so I'm not sure. I don't suppose you have a reproduce case > that I can run? (I do have access to an s390 machine if that helps.) Well, it's on an internal testing framework which we do not release publicly. :-( Luckily, it's not that difficult to reproduce. It seems like this only happens when a reboot is initiated over SSH connection via vsock. Here are some instructions on how to reproduce (with mkosi and Fedora): 1. Craft an mkosi.conf like this: [Distribution] Distribution=fedora Architecture=s390x [Output] Format=cpio CompressOutput=xz [Content] Packages=openssh-server Packages=kernel-modules-core-6.8.5-301.fc40.s390x Bootloader=none MakeInitrd=no Ssh=yes Autologin=yes RootPassword=hunter Timezone=Etc/UTC Locale=C.UTF-8 2. Generate SSH host key: mkdir -p mkosi.extra/etc/ssh && ssh-keygen -t ed25519 -f mkosi.extra/etc/ssh/ssh_host_ed25519_key 3. Build image: mkosi 4. Boot with QEMU: qemu-system-s390x -machine s390-ccw-virtio,accel=kvm -nodefaults -nographic -chardev stdio,id=con0 -device sclpconsole,chardev=con0 -m 2048 -kernel image.vmlinuz -initrd image.cpio.xz -device vhost-vsock-ccw,guest-cid=3 5. In a different terminal, run a reboot loop: i=0; while true; do ssh -o ProxyCommand='socat VSOCK-CONNECT:3:22 -' localhost -l root reboot; echo $i; let i=i+1; done After a while (10 minutes, sometimes longer) you should see the quest crash either with "Attempted to kill init" or "Unable to mount rootfs" and a message about corrupted initramfs: [ 1.599082] Initramfs unpacking failed: XZ-compressed data is corrupt I had this run without your series for 45 minutes, while with your series this crashed within ~5-10 minutes. Thanks!
On Thu, 29 Aug 2024 at 13:20, Nico Boehr <nrb@linux.ibm.com> wrote: > > Quoting Peter Maydell (2024-08-28 17:46:42) > [...] > > Well, the series is *supposed* to be just a refactoring, not a change of > > behaviour, so I'm not sure. I don't suppose you have a reproduce case > > that I can run? (I do have access to an s390 machine if that helps.) > > Well, it's on an internal testing framework which we do not release > publicly. :-( > > Luckily, it's not that difficult to reproduce. It seems like this only > happens when a reboot is initiated over SSH connection via vsock. Here are > some instructions on how to reproduce (with mkosi and Fedora): > > 1. Craft an mkosi.conf like this: > [Distribution] > Distribution=fedora > Architecture=s390x > > [Output] > Format=cpio > CompressOutput=xz > > [Content] > Packages=openssh-server > Packages=kernel-modules-core-6.8.5-301.fc40.s390x > Bootloader=none > MakeInitrd=no > Ssh=yes > Autologin=yes > RootPassword=hunter > Timezone=Etc/UTC > Locale=C.UTF-8 > 2. Generate SSH host key: > mkdir -p mkosi.extra/etc/ssh && ssh-keygen -t ed25519 -f mkosi.extra/etc/ssh/ssh_host_ed25519_key > 3. Build image: > mkosi > 4. Boot with QEMU: > qemu-system-s390x -machine s390-ccw-virtio,accel=kvm -nodefaults -nographic -chardev stdio,id=con0 -device sclpconsole,chardev=con0 -m 2048 -kernel image.vmlinuz -initrd image.cpio.xz -device vhost-vsock-ccw,guest-cid=3 > 5. In a different terminal, run a reboot loop: > i=0; while true; do ssh -o ProxyCommand='socat VSOCK-CONNECT:3:22 -' localhost -l root reboot; echo $i; let i=i+1; done Thanks. I tried this repro, but mkosi falls over almost immediately: ‣ Detaching namespace ‣ Setting up package cache… ‣ Setting up package cache /home/linux1/s390-failure/.mkosi-htsau2ot complete ‣ Setting up temporary workspace. ‣ Temporary workspace set up in /var/tmp/mkosi-tjc0nror ‣ Running second (final) stage… ‣ Creating image with partition table… Disk /home/linux1/s390-failure/.mkosi-ddkx5xpb: 3 GiB, 3221266432 bytes, 6291536 sectors Units: sectors of 1 * 512 = 512 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes >>> Script header accepted. >>> Script header accepted. >>> Script header accepted. >>> Created a new GPT disklabel (GUID: 14CF3B05-D072-0A45-8EE4-3219112ACB2E). /home/linux1/s390-failure/.mkosi-ddkx5xpb1: Created a new partition 1 of type 'unknown' and of size 3 GiB. /home/linux1/s390-failure/.mkosi-ddkx5xpb2: Done. New situation: Disklabel type: gpt Disk identifier: 14CF3B05-D072-0A45-8EE4-3219112ACB2E Device Start End Sectors Size Type /home/linux1/s390-failure/.mkosi-ddkx5xpb1 40 6291495 6291456 3G unknown The partition table has been altered. ‣ Created image with partition table as /home/linux1/s390-failure/.mkosi-ddkx5xpb ‣ Attaching /home/linux1/s390-failure/.mkosi-ddkx5xpb as loopback… ‣ Attached /dev/loop13 ‣ Formatting root partition… mke2fs 1.46.5 (30-Dec-2021) The file /dev/loop13p1 does not exist and no size was specified. ‣ (Detaching /dev/loop13) Traceback (most recent call last): File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main return _run_code(code, main_globals, None, File "/usr/lib/python3.10/runpy.py", line 86, in _run_code exec(code, run_globals) File "/usr/lib/python3/dist-packages/mkosi/__main__.py", line 32, in <module> main() File "/usr/lib/python3/dist-packages/mkosi/__main__.py", line 26, in main run_verb(a) File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 7809, in run_verb manifest = build_stuff(args) File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 7227, in build_stuff image = build_image(args, root, manifest=manifest, do_run_build_script=False, cleanup=True) File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 6941, in build_image prepare_root(args, encrypted.root, cached) File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 1308, in prepare_root mkfs_generic(args, label, path, dev) File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 1031, in mkfs_generic run([*cmdline, dev]) File "/usr/lib/python3/dist-packages/mkosi/backend.py", line 699, in run return subprocess.run(cmdline, check=check, stdout=stdout, stderr=stderr, **kwargs) File "/usr/lib/python3.10/subprocess.py", line 526, in run raise CalledProcessError(retcode, process.args, subprocess.CalledProcessError: Command '['mkfs.ext4', '-I', '256', '-L', 'root', '-M', '/', '/dev/loop13p1']' returned non-zero exit status 1. (My s390 box is running Ubuntu, in case that makes a difference.) Maybe you could put the kernel and initrd somewhere I can get them from? -- PMM
> > (My s390 box is running Ubuntu, in case that makes a difference.) > > Maybe you could put the kernel and initrd somewhere I can > get them from? I am also trying. See : https://www.kaod.org/qemu/s390x/ These were generated on my x86/f40 laptop. C.
Quoting Peter Maydell (2024-08-29 15:09:44) > Thanks. I tried this repro, but mkosi falls over almost > immediately: > > ‣ Detaching namespace > ‣ Setting up package cache… > ‣ Setting up package cache /home/linux1/s390-failure/.mkosi-htsau2ot complete > ‣ Setting up temporary workspace. > ‣ Temporary workspace set up in /var/tmp/mkosi-tjc0nror > ‣ Running second (final) stage… > ‣ Creating image with partition table… > Disk /home/linux1/s390-failure/.mkosi-ddkx5xpb: 3 GiB, 3221266432 > bytes, 6291536 sectors > Units: sectors of 1 * 512 = 512 bytes > Sector size (logical/physical): 512 bytes / 512 bytes > I/O size (minimum/optimal): 512 bytes / 512 bytes > > >>> Script header accepted. > >>> Script header accepted. > >>> Script header accepted. > >>> Created a new GPT disklabel (GUID: 14CF3B05-D072-0A45-8EE4-3219112ACB2E). > /home/linux1/s390-failure/.mkosi-ddkx5xpb1: Created a new partition 1 > of type 'unknown' and of size 3 GiB. > /home/linux1/s390-failure/.mkosi-ddkx5xpb2: Done. > > New situation: > Disklabel type: gpt > Disk identifier: 14CF3B05-D072-0A45-8EE4-3219112ACB2E > > Device Start End Sectors Size Type > /home/linux1/s390-failure/.mkosi-ddkx5xpb1 40 6291495 6291456 3G unknown > > The partition table has been altered. > ‣ Created image with partition table as > /home/linux1/s390-failure/.mkosi-ddkx5xpb > ‣ Attaching /home/linux1/s390-failure/.mkosi-ddkx5xpb as loopback… > ‣ Attached /dev/loop13 > ‣ Formatting root partition… > mke2fs 1.46.5 (30-Dec-2021) > The file /dev/loop13p1 does not exist and no size was specified. > ‣ (Detaching /dev/loop13) > Traceback (most recent call last): > File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main > return _run_code(code, main_globals, None, > File "/usr/lib/python3.10/runpy.py", line 86, in _run_code > exec(code, run_globals) > File "/usr/lib/python3/dist-packages/mkosi/__main__.py", line 32, in <module> > main() > File "/usr/lib/python3/dist-packages/mkosi/__main__.py", line 26, in main > run_verb(a) > File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 7809, > in run_verb > manifest = build_stuff(args) > File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 7227, > in build_stuff > image = build_image(args, root, manifest=manifest, > do_run_build_script=False, cleanup=True) > File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 6941, > in build_image > prepare_root(args, encrypted.root, cached) > File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 1308, > in prepare_root > mkfs_generic(args, label, path, dev) > File "/usr/lib/python3/dist-packages/mkosi/__init__.py", line 1031, > in mkfs_generic > run([*cmdline, dev]) > File "/usr/lib/python3/dist-packages/mkosi/backend.py", line 699, in run > return subprocess.run(cmdline, check=check, stdout=stdout, > stderr=stderr, **kwargs) > File "/usr/lib/python3.10/subprocess.py", line 526, in run > raise CalledProcessError(retcode, process.args, > subprocess.CalledProcessError: Command '['mkfs.ext4', '-I', '256', > '-L', 'root', '-M', '/', '/dev/loop13p1']' returned non-zero exit > status 1. > > (My s390 box is running Ubuntu, in case that makes a difference.) > > Maybe you could put the kernel and initrd somewhere I can > get them from? Sigh. Yeah, I'll try to find a way (may take a while). In the meantime, looks like mkosi is trying to create an block image, but that's not what it's configured to do; are you sure mkosi.conf is in the same directory you're calling it from?
On Thu, 29 Aug 2024 at 14:26, Nico Boehr <nrb@linux.ibm.com> wrote: > > Quoting Peter Maydell (2024-08-29 15:09:44) > > Thanks. I tried this repro, but mkosi falls over almost > > immediately: > In the meantime, looks like mkosi is trying to create an block image, but > that's not what it's configured to do; are you sure mkosi.conf is in the > same directory you're calling it from? It is. I notice however that the manpage for mkosi says that it looks for "mkosi.default", not "mkosi.conf". Maybe it needs a newer mkosi than Ubuntu ships? (mkosi --version says "mkosi 12".) I'll use the images Cédric has kindly generated. -- PMM
Quoting Peter Maydell (2024-08-29 15:35:30) > On Thu, 29 Aug 2024 at 14:26, Nico Boehr <nrb@linux.ibm.com> wrote: > > > > Quoting Peter Maydell (2024-08-29 15:09:44) > > > Thanks. I tried this repro, but mkosi falls over almost > > > immediately: > > > In the meantime, looks like mkosi is trying to create an block image, but > > that's not what it's configured to do; are you sure mkosi.conf is in the > > same directory you're calling it from? > > It is. I notice however that the manpage for mkosi > says that it looks for "mkosi.default", not "mkosi.conf". > Maybe it needs a newer mkosi than Ubuntu ships? > (mkosi --version says "mkosi 12".) Likely. I have mkosi 22 here. > I'll use the images Cédric has kindly generated. Thanks, images by Cédric look good, but I forgot to tell you that you need a SSH key for login too :) You could unpack image.cpio.gz, add your key to root/.ssh and repack the whole thing to image.new.xz: - mkdir xtract && cd xtract - unxz < ../image.cpio.xz | cpio -H newc -iv - cp ~/.ssh/authorized_keys root/.ssh/ - find . 2>/dev/null | cpio -o -c -R root:root | xz -9 --format=lzma > ../image.new.xz
On Thu, 29 Aug 2024 at 15:44, Nico Boehr <nrb@linux.ibm.com> wrote: > > Quoting Peter Maydell (2024-08-29 15:35:30) > > On Thu, 29 Aug 2024 at 14:26, Nico Boehr <nrb@linux.ibm.com> wrote: > > > > > > Quoting Peter Maydell (2024-08-29 15:09:44) > > > > Thanks. I tried this repro, but mkosi falls over almost > > > > immediately: > > > > > In the meantime, looks like mkosi is trying to create an block image, but > > > that's not what it's configured to do; are you sure mkosi.conf is in the > > > same directory you're calling it from? > > > > It is. I notice however that the manpage for mkosi > > says that it looks for "mkosi.default", not "mkosi.conf". > > Maybe it needs a newer mkosi than Ubuntu ships? > > (mkosi --version says "mkosi 12".) > > Likely. I have mkosi 22 here. > > > I'll use the images Cédric has kindly generated. > > Thanks, images by Cédric look good, but I forgot to tell you that > you need a SSH key for login too :) > > You could unpack image.cpio.gz, add your key to root/.ssh and repack the > whole thing to image.new.xz: > - mkdir xtract && cd xtract > - unxz < ../image.cpio.xz | cpio -H newc -iv > - cp ~/.ssh/authorized_keys root/.ssh/ > - find . 2>/dev/null | cpio -o -c -R root:root | xz -9 --format=lzma > ../image.new.xz Thanks, I was just wondering how to get the ssh key into there. I found I needed ... cpio -H newc -o -R root:root | xz -9 -C crc32 when creating the new image, otherwise the kernel doesn't like the format. (Also since I'm not root I needed to sprinkle in some sudo invocations.) -- PMM
On Wed, 28 Aug 2024 at 09:13, Nico Boehr <nrb@linux.ibm.com> wrote: > > Quoting Nico Boehr (2024-08-26 14:08:20) > > There was a little hickup without the fixup to patch 2, but after Nina > > pushed the fixup, we did not observe any failures related to your > > changes in our CI. Thanks! > > Peter, after a few CI runs, we unfortunately did find some issues with your > patch :-( > > Rebooting a guest in a loop sometimes fails. Michael was able to bisect it > to your series. > > The problem is intermittent. The guest is unable to load its initramfs: > > [ 0.560674] rootfs image is not initramfs (no cpio magic); looks like an initrd > [ 0.588605] Freeing initrd memory: 95680K > [ 0.593143] md: Waiting for all devices to be available before autodetect > [ 0.593144] md: If you don't use raid, use raid=noautodetect > [ 0.593145] md: Autodetecting RAID arrays. > [ 0.593146] md: autorun ... > [ 0.593147] md: ... autorun DONE. > [ 0.593156] RAMDISK: gzip image found at block 0 > [ 0.609110] RAMDISK: incomplete write (29120 != 32768) > [ 0.609113] write error > > ...and then a panic because the kernel doesn't find a rootfs. I repro'd *something*, but it wasn't quite this. I got: [ 4.691853] clk: Disabling unused clocks [ 4.695419] Freeing unused kernel image (initmem) memory: 6520K [ 4.695430] Write protected read-only-after-init data: 144k [ 4.695834] Checked W+X mappings: passed, no unexpected W+X pages found [ 4.695849] Run /init as init process /init: error while loading shared libraries: libgcc_s.so.1: cannot open shared object file: No such file or directory [ 4.697009] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00007f00 [ 4.697030] CPU: 0 PID: 1 Comm: init Not tainted 6.8.5-301.fc40.s390x #1 [ 4.697035] Hardware name: IBM 8561 LT1 400 (KVM/Linux) [ 4.697040] Call Trace: [ 4.697047] [<000000007ab6ae36>] dump_stack_lvl+0x66/0x88 [ 4.697081] [<0000000079e17c2a>] panic+0x312/0x328 [ 4.697096] [<0000000079e1de84>] do_exit+0x8a4/0xae8 [ 4.697101] [<0000000079e1e2e0>] do_group_exit+0x40/0xb8 [ 4.697103] [<0000000079e1e386>] __s390x_sys_exit_group+0x2e/0x30 [ 4.697105] [<000000007ab9526a>] __do_syscall+0x252/0x2c0 [ 4.697113] [<000000007aba8840>] system_call+0x70/0x98 Which I guess could be caused by a different corruption of the initramfs ? thanks -- PMM
Quoting Peter Maydell (2024-08-29 17:53:02) > On Wed, 28 Aug 2024 at 09:13, Nico Boehr <nrb@linux.ibm.com> wrote: > > > > Quoting Nico Boehr (2024-08-26 14:08:20) > > > There was a little hickup without the fixup to patch 2, but after Nina > > > pushed the fixup, we did not observe any failures related to your > > > changes in our CI. Thanks! > > > > Peter, after a few CI runs, we unfortunately did find some issues with your > > patch :-( > > > > Rebooting a guest in a loop sometimes fails. Michael was able to bisect it > > to your series. > > > > The problem is intermittent. The guest is unable to load its initramfs: > > > > [ 0.560674] rootfs image is not initramfs (no cpio magic); looks like an initrd > > [ 0.588605] Freeing initrd memory: 95680K > > [ 0.593143] md: Waiting for all devices to be available before autodetect > > [ 0.593144] md: If you don't use raid, use raid=noautodetect > > [ 0.593145] md: Autodetecting RAID arrays. > > [ 0.593146] md: autorun ... > > [ 0.593147] md: ... autorun DONE. > > [ 0.593156] RAMDISK: gzip image found at block 0 > > [ 0.609110] RAMDISK: incomplete write (29120 != 32768) > > [ 0.609113] write error > > > > ...and then a panic because the kernel doesn't find a rootfs. > > I repro'd *something*, but it wasn't quite this. I got: > > > [ 4.691853] clk: Disabling unused clocks > [ 4.695419] Freeing unused kernel image (initmem) memory: 6520K > [ 4.695430] Write protected read-only-after-init data: 144k > [ 4.695834] Checked W+X mappings: passed, no unexpected W+X pages found > [ 4.695849] Run /init as init process > /init: error while loading shared libraries: libgcc_s.so.1: cannot > open shared object file: No such file or directory > [ 4.697009] Kernel panic - not syncing: Attempted to kill init! > exitcode=0x00007f00 > [ 4.697030] CPU: 0 PID: 1 Comm: init Not tainted 6.8.5-301.fc40.s390x #1 > [ 4.697035] Hardware name: IBM 8561 LT1 400 (KVM/Linux) > [ 4.697040] Call Trace: > [ 4.697047] [<000000007ab6ae36>] dump_stack_lvl+0x66/0x88 > [ 4.697081] [<0000000079e17c2a>] panic+0x312/0x328 > [ 4.697096] [<0000000079e1de84>] do_exit+0x8a4/0xae8 > [ 4.697101] [<0000000079e1e2e0>] do_group_exit+0x40/0xb8 > [ 4.697103] [<0000000079e1e386>] __s390x_sys_exit_group+0x2e/0x30 > [ 4.697105] [<000000007ab9526a>] __do_syscall+0x252/0x2c0 > [ 4.697113] [<000000007aba8840>] system_call+0x70/0x98 > > Which I guess could be caused by a different corruption > of the initramfs ? I think that is the problem, just a different symptom. I also got this sometimes (in random libraries all over the place). And yes, agree, it is a corruption of the initramfs: [ 1.597465] Initramfs unpacking failed: XZ-compressed data is corrupt
On Fri, 30 Aug 2024 at 06:48, Nico Boehr <nrb@linux.ibm.com> wrote: > > Quoting Peter Maydell (2024-08-29 17:53:02) > > I repro'd *something*, but it wasn't quite this. I got: > > > > > > [ 4.691853] clk: Disabling unused clocks > > [ 4.695419] Freeing unused kernel image (initmem) memory: 6520K > > [ 4.695430] Write protected read-only-after-init data: 144k > > [ 4.695834] Checked W+X mappings: passed, no unexpected W+X pages found > > [ 4.695849] Run /init as init process > > /init: error while loading shared libraries: libgcc_s.so.1: cannot > > open shared object file: No such file or directory > > [ 4.697009] Kernel panic - not syncing: Attempted to kill init! > > exitcode=0x00007f00 > > [ 4.697030] CPU: 0 PID: 1 Comm: init Not tainted 6.8.5-301.fc40.s390x #1 > > [ 4.697035] Hardware name: IBM 8561 LT1 400 (KVM/Linux) > > [ 4.697040] Call Trace: > > [ 4.697047] [<000000007ab6ae36>] dump_stack_lvl+0x66/0x88 > > [ 4.697081] [<0000000079e17c2a>] panic+0x312/0x328 > > [ 4.697096] [<0000000079e1de84>] do_exit+0x8a4/0xae8 > > [ 4.697101] [<0000000079e1e2e0>] do_group_exit+0x40/0xb8 > > [ 4.697103] [<0000000079e1e386>] __s390x_sys_exit_group+0x2e/0x30 > > [ 4.697105] [<000000007ab9526a>] __do_syscall+0x252/0x2c0 > > [ 4.697113] [<000000007aba8840>] system_call+0x70/0x98 > > > > Which I guess could be caused by a different corruption > > of the initramfs ? > > I think that is the problem, just a different symptom. I also got this > sometimes (in random libraries all over the place). I ran overnight with none of the patchset applied, and it never failed (as expected). Running with just the first virtio-ccw patch does fall over fairly quickly. So something's up with that patch, which is curious because that's the one I thought was a straightforward conversion without any complications :-) I'll investigate further today, I have the beginnings of a theory about what might be happening... -- PMM
Quoting Peter Maydell (2024-08-30 12:01:47) > On Fri, 30 Aug 2024 at 06:48, Nico Boehr <nrb@linux.ibm.com> wrote: > > > > Quoting Peter Maydell (2024-08-29 17:53:02) > > > I repro'd *something*, but it wasn't quite this. I got: > > > > > > > > > [ 4.691853] clk: Disabling unused clocks > > > [ 4.695419] Freeing unused kernel image (initmem) memory: 6520K > > > [ 4.695430] Write protected read-only-after-init data: 144k > > > [ 4.695834] Checked W+X mappings: passed, no unexpected W+X pages found > > > [ 4.695849] Run /init as init process > > > /init: error while loading shared libraries: libgcc_s.so.1: cannot > > > open shared object file: No such file or directory > > > [ 4.697009] Kernel panic - not syncing: Attempted to kill init! > > > exitcode=0x00007f00 > > > [ 4.697030] CPU: 0 PID: 1 Comm: init Not tainted 6.8.5-301.fc40.s390x #1 > > > [ 4.697035] Hardware name: IBM 8561 LT1 400 (KVM/Linux) > > > [ 4.697040] Call Trace: > > > [ 4.697047] [<000000007ab6ae36>] dump_stack_lvl+0x66/0x88 > > > [ 4.697081] [<0000000079e17c2a>] panic+0x312/0x328 > > > [ 4.697096] [<0000000079e1de84>] do_exit+0x8a4/0xae8 > > > [ 4.697101] [<0000000079e1e2e0>] do_group_exit+0x40/0xb8 > > > [ 4.697103] [<0000000079e1e386>] __s390x_sys_exit_group+0x2e/0x30 > > > [ 4.697105] [<000000007ab9526a>] __do_syscall+0x252/0x2c0 > > > [ 4.697113] [<000000007aba8840>] system_call+0x70/0x98 > > > > > > Which I guess could be caused by a different corruption > > > of the initramfs ? > > > > I think that is the problem, just a different symptom. I also got this > > sometimes (in random libraries all over the place). > > I ran overnight with none of the patchset applied, and it never > failed (as expected). Running with just the first virtio-ccw > patch does fall over fairly quickly. So something's up with > that patch, which is curious because that's the one I thought > was a straightforward conversion without any complications :-) > > I'll investigate further today, I have the beginnings of a > theory about what might be happening... Thanks for taking the time, Peter! Let me know when you have insights.
On Fri, 30 Aug 2024 at 12:56, Nico Boehr <nrb@linux.ibm.com> wrote: > > Quoting Peter Maydell (2024-08-30 12:01:47) > > I ran overnight with none of the patchset applied, and it never > > failed (as expected). Running with just the first virtio-ccw > > patch does fall over fairly quickly. So something's up with > > that patch, which is curious because that's the one I thought > > was a straightforward conversion without any complications :-) > > > > I'll investigate further today, I have the beginnings of a > > theory about what might be happening... > > Thanks for taking the time, Peter! Let me know when you have insights. I think I've found the problem, I'm just testing it to see if it does properly fix the intermittent. The issue is that before we can convert a class to three-phase reset we need to have already converted all its parent classes. TYPE_VIRTIO_CCW_DEVICE is a subclass of TYPE_CCW_DEVICE, but I forgot to convert TYPE_CCW_DEVICE to three-phase reset. The result is that the reset in the parent class was not happening at all (presumably leaving the TYPE_CCW_DEVICE in an invalid/unexpected state on reboot). The fix is to add an extra patch at the start of the series. Once I've tested this I'll send out a v2 of the series, maybe also adding the cleanup RTH suggested in one of the later patches. (Ironically, if we do that cleanup then the bug would also go away, because old-style DeviceClass:reset methods will get called in exactly the same way as if they were registered as phases.hold methods, rather than being specially handled...) commit ff9bc4c8910f636f20e9b6e91d63dd003408ce10 Author: Peter Maydell <peter.maydell@linaro.org> Date: Fri Aug 30 12:50:03 2024 +0100 hw/s390/ccw-device: Convert to three-phase reset Convert the TYPE_CCW_DEVICE to three-phase reset. This is a device class which is subclassed, so it needs to be three-phase before we can convert the subclass. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c index a7d682e5af9..14c24e38904 100644 --- a/hw/s390x/ccw-device.c +++ b/hw/s390x/ccw-device.c @@ -44,9 +44,9 @@ static Property ccw_device_properties[] = { DEFINE_PROP_END_OF_LIST(), }; -static void ccw_device_reset(DeviceState *d) +static void ccw_device_reset_hold(Object *obj, ResetType type) { - CcwDevice *ccw_dev = CCW_DEVICE(d); + CcwDevice *ccw_dev = CCW_DEVICE(obj); css_reset_sch(ccw_dev->sch); } @@ -55,11 +55,12 @@ static void ccw_device_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); CCWDeviceClass *k = CCW_DEVICE_CLASS(klass); + ResettableClass *rc = RESETTABLE_CLASS(klass); k->realize = ccw_device_realize; k->refill_ids = ccw_device_refill_ids; device_class_set_props(dc, ccw_device_properties); - dc->reset = ccw_device_reset; + rc->phases.hold = ccw_device_reset_hold; dc->bus_type = TYPE_VIRTUAL_CSS_BUS; } -- PMM
On Fri, 30 Aug 2024 at 13:04, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 30 Aug 2024 at 12:56, Nico Boehr <nrb@linux.ibm.com> wrote: > > > > Quoting Peter Maydell (2024-08-30 12:01:47) > > > I ran overnight with none of the patchset applied, and it never > > > failed (as expected). Running with just the first virtio-ccw > > > patch does fall over fairly quickly. So something's up with > > > that patch, which is curious because that's the one I thought > > > was a straightforward conversion without any complications :-) > > > > > > I'll investigate further today, I have the beginnings of a > > > theory about what might be happening... > > > > Thanks for taking the time, Peter! Let me know when you have insights. > > I think I've found the problem, I'm just testing it to see if it > does properly fix the intermittent. > The fix is to add an extra patch at the start of the > series. Once I've tested this I'll send out a v2 of the series, > maybe also adding the cleanup RTH suggested in one of the later > patches. By the way, how would you like this series to go upstream? I can think of three options: * you take the whole thing through the s390 tree * I take the whole thing through my tree * you take the s390 specific patches, and I take the remaining reset-cleanup patches once those have landed Any preference? thanks -- PMM
On 30/08/2024 14.17, Peter Maydell wrote: > On Fri, 30 Aug 2024 at 13:04, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On Fri, 30 Aug 2024 at 12:56, Nico Boehr <nrb@linux.ibm.com> wrote: >>> >>> Quoting Peter Maydell (2024-08-30 12:01:47) >>>> I ran overnight with none of the patchset applied, and it never >>>> failed (as expected). Running with just the first virtio-ccw >>>> patch does fall over fairly quickly. So something's up with >>>> that patch, which is curious because that's the one I thought >>>> was a straightforward conversion without any complications :-) >>>> >>>> I'll investigate further today, I have the beginnings of a >>>> theory about what might be happening... >>> >>> Thanks for taking the time, Peter! Let me know when you have insights. >> >> I think I've found the problem, I'm just testing it to see if it >> does properly fix the intermittent. > >> The fix is to add an extra patch at the start of the >> series. Once I've tested this I'll send out a v2 of the series, >> maybe also adding the cleanup RTH suggested in one of the later >> patches. > > By the way, how would you like this series to go upstream? > I can think of three options: > * you take the whole thing through the s390 tree > * I take the whole thing through my tree > * you take the s390 specific patches, and I take the > remaining reset-cleanup patches once those have landed > > Any preference? I'm fine if you take the patches through your tree along with the other reset-related patches. Thomas