On Thu, Apr 01, 2021 at 04:19:03PM +0800, Ye Bin wrote: > Test steps: > 1. mount /dev/sda -o errors=panic test > 2. mount /dev/sda -o remount,ro test > 3. mount /dev/sda -o remount,abort test > > Before 014c9caa29d3 not been merged there will trigger panic. But > 014c9caa29d3 change this behavior. This makes sense, but I'll note this behavior has changed over time. root@kvm-xfstests:~# mount -o errors=panic /dev/vdc /vdc [ 20.252713] EXT4-fs (vdc): mounted filesystem with ordered data mode. Opts: errors=panic root@kvm-xfstests:~# mount -o remount,ro /dev/vdc [ 24.832448] EXT4-fs (vdc): re-mounted. Opts: (null) root@kvm-xfstests:~# mount -o remount,abort /dev/vdc [ 30.833543] EXT4-fs error (device vdc): ext4_remount:5340: Abort forced by user mount: /vdc: cannot remount /dev/vdc read-write, is write-protected. root@kvm-xfstests:~# mount -o remount,abort,ro /dev/vdc [ 34.545549] EXT4-fs error (device vdc): ext4_remount:5340: Abort forced by user [ 34.555475] EXT4-fs (vdc): re-mounted. Opts: abort root@kvm-xfstests:~# uname -a Linux kvm-xfstests 4.19.163-xfstests #1 SMP Sat Dec 19 23:55:11 EST 2020 x86_64 GNU/Linux root@kvm-xfstests:~# The same is true for the 5.4 kernel. I do agree that it *should* force a panic, and the fact that superblock is read-only shouldn't make a difference as to how errors=panic is handled. So I think the patch is correct, but I'll note that it also changes this case: 1) mount -o /dev/sda -o ro,errors=panic test 2) echo test > /sys/fs/ext4/sda/trigger_fs_error With this patch, this will also now panic, whereas before, an ext4_error() would not trigger a panic. I think that's better because it makes things more consistent --- but it is a change in behavior which could potentially surprise some people. But since they can easily get the previous behavior with an explicit "mount -o ro,errors=continue", I think that's acceptable. I'll apply the patch with a modified commit description to warn of this particular change in behavior. - Ted
