On 2025/1/3 22:34, Jan Kara wrote:
On Fri 03-01-25 21:19:27, Baokun Li wrote:
On 2025/1/3 18:42, Jan Kara wrote:
What's worse is that after commit
95257987a638 ("ext4: drop EXT4_MF_FS_ABORTED flag")
was merged in v6.6-rc1, the EXT4_FLAGS_SHUTDOWN bit is set in
ext4_handle_error(). This causes the file system to not be read-only
when an error is triggered in "errors=remount-ro" mode, because
EXT4_FLAGS_SHUTDOWN prevents both writing and reading.
Here I don't understand what is really the problem with EXT4_MF_FS_ABORTED
removal. What do you exactly mean by "causes the file system to not be
read-only"? We still return EROFS where we used to, we disallow writing as
we used to. Can you perhaps give an example what changed with this commit?
Sorry for the lack of clarity in my previous explanation. The key point
is not about removing EXT4_MF_FS_ABORTED, but rather we will set
EXT4_FLAGS_SHUTDOWN bit, which not only prevents writes but also prevents
reads. Therefore, saying it's not read-only actually means it's completely
unreadable.
Ah, I see. I didn't think about that. Is it that you really want reading to
work from a filesystem after error? Can you share why (I'm just trying to
understand the usecase)? Or is this mostly a theoretical usecase?
Switching to read-only mode after an error is a common practice for most
file systems (ext4/btrfs/affs/fat/jfs/nilfs/nilfs2/ocfs2/ubifs/ufs, etc.).
There are two main benefits to doing this:
* Read-only processes can continue to run unaffected after the error.
* Shutting down after an error would lose some data in memory that has
not been written to disk. If the file system is read-only, we can back
up these data to another location in time and then exit gracefully.
I think we could introduce "shutdown modifications" state which would still
allow pure reads to succeed if there's a usecase for such functionality.
I agree that maintaining a flag like EXT4_FLAGS_RDONLY within ext4 seems
to be a good solution at this point. It avoids both introducing mechanism
changes and VFS coupling. If no one has a better idea, I will implement it.
Yeah, let's go with a separate "emergency RO" ext4 flag then. I think we
could just enhance the ext4_forced_shutdown() checks to take a flag whether
the operation is a modification or not and when it is a modification, it
would additionally trigger also when EMERGENCY_RO flag is set (which would
get set by ext4_handle_error()).
Thanks for having a look into this.
Honza
Sounds very nice, this way the changes will be minimal and the code won't
look messy.Thank you for your suggestion!
Cheers,
Baokun
So how does your framework detect that the filesystem has failed with
errors=remount-ro? By parsing /proc/mounts or otherwise querying current
filesystem mount options?
In most cases, run the mount command and filter related options.
Would it be acceptable for you to look at some
other mount option (e.g. "shutdown") to detect that state? We could easily
implement that.
We do need to add a shutdown hint, but that's not the point.
We've discussed this internally, and now if the logs are flushed,
we have no way of knowing if the current filesystem is shutdown. We don't
know if the -EIO from the filesystem is a hardware problem or if the
filesystem is already shutdown. So even if there is no current problem,
we should add some kind of hint to let the user know that the current
filesystem is shutdown.
The changes to display shutdown are as follows, so that we can see if the
current filesystem has been shutdown in the mount command.
Yes, I think this would be a good addition regardless of other changes we
might need to do. It would be preferable to be able to come up with
something that's acceptable for querying of shutdown state also for other
filesystems - I've CCed fsdevel and XFS in particular since it has much
longer history of fs shutdown implementation.
Honza
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3955bec9245d..ba28ef0f662e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3157,6 +3157,9 @@ static int _ext4_show_options(struct seq_file *seq,
struct super_block *sb,
if (nodefs && !test_opt(sb, NO_PREFETCH_BLOCK_BITMAPS))
SEQ_OPTS_PUTS("prefetch_block_bitmaps");
+ if (!nodefs && ext4_forced_shutdown(sb))
+ SEQ_OPTS_PUTS("shutdown");
+
ext4_show_quota_options(seq, sb);
return 0;
}
I'm sorry again for causing you trouble.
Never mind, thank you for your reply!
