public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Baokun Li <libaokun1@huawei.com>
Cc: Theodore Ts'o <tytso@mit.edu>, Jan Kara <jack@suse.cz>,
	"linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
	Christian Brauner <brauner@kernel.org>,
	sunyongjian1@huawei.com, Yang Erkun <yangerkun@huawei.com>,
	linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [BUG REPORT] ext4: “errors=remount-ro” has become “errors=shutdown”?
Date: Mon, 6 Jan 2025 15:49:56 -0800	[thread overview]
Message-ID: <20250106234956.GM6174@frogsfrogsfrogs> (raw)
In-Reply-To: <5eb2ad64-c6ea-45f8-9ba1-7de5c68d59aa@huawei.com>

On Sat, Jan 04, 2025 at 10:41:28AM +0800, Baokun Li wrote:
> Hi Ted,
> 
> On 2025/1/3 23:54, Theodore Ts'o wrote:
> > On Fri, Jan 03, 2025 at 10:35:17AM -0500, Theodore Ts'o wrote:
> > > I don't see how setting the shutdown flag causes reads to fail.  That
> > > was true in an early version of the ext4 patch which implemented
> > > shutdown support, but one of the XFS developers (I don't remember if
> > > it was Dave or Cristoph) objected because XFS did not cause the
> > > read_pages function to fail.  Are you seeing this with an upstream
> > > kernel, or with a patched kernel?  The upstream kernel does *not* have
> > > the check in ext4_readpages() or ext4_read_folio() (post folio
> > > conversion).
> > OK, that's weird.  Testing on 6.13-rc4, I don't see the problem simulating an ext4 error:
> > 
> > root@kvm-xfstests:~# mke2fs -t ext4 -Fq /dev/vdc
> > /dev/vdc contains a ext4 file system
> > 	last mounted on /vdc on Fri Jan  3 10:38:21 2025
> > root@kvm-xfstests:~# mount -t ext4 -o errors=continue /dev/vdc /vdc
> We are discussing "errors=remount-ro," as the title states, not the
> continue mode. The key code leading to the behavior change is as follows,
> therefore the continue mode is not affected.

Hmm.  On the one hand, XFS has generally returned EIO (or ESHUTDOWN in a
couple of specialty cases) when the fs has been shut down.

OTOH XFS also doesn't have errors=remount-ro; it just dies, which I
think has been its behavior for a long time.

To me, it doesn't sound unreasonable for ext* to allow reads after a
shutdown when errors=remount-ro since it's always had that behavior.

Bonus Q: do you want an errors=fail variant to shut things down fast?

--D

> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -657,7 +657,7 @@ static void ext4_handle_error(struct super_block *sb,
> bool force_ro, int error,
>                 WARN_ON_ONCE(1);
> 
>         if (!continue_fs && !sb_rdonly(sb)) {
> -               ext4_set_mount_flag(sb, EXT4_MF_FS_ABORTED);
> +               set_bit(EXT4_FLAGS_SHUTDOWN, &EXT4_SB(sb)->s_ext4_flags);
>                 if (journal)
>                         jbd2_journal_abort(journal, -EIO);
>         }
> 
> See the end for problem reproduction.
> 
> > [   24.780982] EXT4-fs (vdc): mounted filesystem f8595206-fe57-486c-80dd-48b03d41ebdb r/w with ordered data mode. Quota mode: none.
> > root@kvm-xfstests:~# cp /etc/motd /vdc/motd
> > root@kvm-xfstests:~# echo testing > /sys/fs/ext4/vdc/trigger_fs_error
> > [   42.943141] EXT4-fs error (device vdc): trigger_test_error:129: comm bash: testing
> > root@kvm-xfstests:~# cat /vdc/motd
> > 
> > The programs included with the Debian GNU/Linux system are free software;
> > the exact distribution terms for each program are described in the
> > individual files in /usr/share/doc/*/copyright.
> > 
> > Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
> > permitted by applicable law.
> > root@kvm-xfstests:~#
> > 
> > 
> > HOWEVER, testing with shutdown ioctl, both ext4 and xfs are failing with EIO:
> Yes, this is as expected.
> > root@kvm-xfstests:~# mount /dev/vdc /vdc
> > [    7.969168] XFS (vdc): Mounting V5 Filesystem 7834ea96-eab0-46c5-9b18-c8f054fa9cf4
> > [    7.978539] XFS (vdc): Ending clean mount
> > root@kvm-xfstests:~# cp /etc/motd /vdc
> > root@kvm-xfstests:~# /root/xfstests/src/godown -v /vdc
> > Opening "/vdc"
> > Calling XFS_IOC_GOINGDOWN
> > [   29.354609] XFS (vdc): User initiated shutdown received.
> > [   29.356123] XFS (vdc): Log I/O Error (0x6) detected at xfs_fs_goingdown+0x55/0xb0 (fs/xfs/xfs_fsops.c:452).  Shutting down filesystem.
> > [   29.357092] XFS (vdc): Please unmount the filesystem and rectify the problem(s)
> > root@kvm-xfstests:~# cat /vdc/motd
> > cat: /vdc/motd: Input/output error
> > root@kvm-xfstests:~#
> > 
> > So I take back what I said earlier, but I am a bit confused why it
> > worked after simulating an file system error using "echo testing >
> > /sys/fs/ext4/vdc/trigger_fs_error".
> > 
> It's because "errors=remount-ro" wasn't used when mounting...
> 
> Here's a replication:
> 
> root@kvm-xfstests:~# mount -o errors=remount-ro /dev/vdc /mnt/test
> [  115.731007] EXT4-fs (vdc): mounted filesystem
> 0838f08f-c04e-440c-a9a5-417677efb03e r/w with ordered data mode. Quota mode:
> none.
> root@kvm-xfstests:~# echo test > /mnt/test/file
> root@kvm-xfstests:~# cat /mnt/test/file
> test
> root@kvm-xfstests:~# echo 1 > /sys/fs/ext4/vdc/trigger_fs_error
> [  131.537649] EXT4-fs error (device vdc): trigger_test_error:129: comm
> bash: 1
> [  131.538226] Aborting journal on device vdc-8.
> [  131.538844] EXT4-fs (vdc): Remounting filesystem read-only
> root@kvm-xfstests:~# cat /mnt/test/file
> cat: /mnt/test/file: Input/output error
> root@kvm-xfstests:~# uname -a
> Linux kvm-xfstests 6.13.0-rc4-xfstests-g6cfe3548f8f5-dirty #284 SMP
> PREEMPT_DYNAMIC Fri Dec 27 10:39:02 CST 2024 x86_64 GNU/Linux
> 
> 
> Regards,
> Baokun
> 
> 

  reply	other threads:[~2025-01-06 23:49 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <22d652f6-cb3c-43f5-b2fe-0a4bb6516a04@huawei.com>
     [not found] ` <z52ea53du2k66du24ju4yetqm72e6pvtcbwkrjf4oomw2feffq@355vymdndrxn>
     [not found]   ` <17108cad-efa8-46b4-a320-70d7b696f75b@huawei.com>
2025-01-03 10:42     ` [BUG REPORT] ext4: “errors=remount-ro” has become “errors=shutdown”? Jan Kara
2025-01-03 13:19       ` Baokun Li
2025-01-03 14:34         ` Jan Kara
2025-01-04  3:15           ` Baokun Li
2025-01-03 15:35       ` Theodore Ts'o
2025-01-03 15:54         ` Theodore Ts'o
2025-01-04  2:41           ` Baokun Li
2025-01-06 23:49             ` Darrick J. Wong [this message]
2025-01-07  2:01               ` Baokun Li
2025-01-07  7:08                 ` Darrick J. Wong
2025-01-08  2:08                   ` Baokun Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250106234956.GM6174@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=libaokun1@huawei.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sunyongjian1@huawei.com \
    --cc=tytso@mit.edu \
    --cc=yangerkun@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox