On 2023/2/10 19:56, Jan Kara wrote: > Hello! > > On Fri 10-02-23 11:20:38, Ye Bin wrote: >> From: Ye Bin <yebin10@xxxxxxxxxx> >> >> Diff v2 vs v1: >> Move call 'j_replay_prepare_callback' and 'j_replay_end_callback' from >> ext4_load_journal() to jbd2_journal_recover(). >> >> When do fault injection test, got issue as follows: >> EXT4-fs (dm-5): warning: mounting fs with errors, running e2fsck is recommended >> EXT4-fs (dm-5): Errors on filesystem, clearing orphan list. >> EXT4-fs (dm-5): recovery complete >> EXT4-fs (dm-5): mounted filesystem with ordered data mode. Opts: data_err=abort,errors=remount-ro >> >> EXT4-fs (dm-5): recovery complete >> EXT4-fs (dm-5): mounted filesystem with ordered data mode. Opts: data_err=abort,errors=remount-ro >> >> Without do file system check, file system is clean when do second mount. >> Theoretically, the kernel will not clear fs error flag. In errors=remount-ro >> mode the last super block is commit directly. So super block in journal is >> not uptodate. When do jounral recovery, the uptodate super block will be >> covered by jounral data. If super block submit all failed after recover >> journal, then file system error flag is lost. When do "fsck -a" couldn't >> repair file system deeply. >> To solve above issue we need to do extra handle when do super block journal >> recovery. > > Thanks for the patches. Looking through the patches, I think this is a bit > of an overengineering for the problem at hand. The only thing that is > really worth preserving so that it is not lost after journal replay is the > error information. So in ext4_load_journal() I would just save that if > EXT4_ERROR_FS is set in es->s_state before journal replay and restore it > after journal replay. Sure if the superblock write during journal replay > succeeds but the write restoring the error information fails, we will loose > the error information but that is so unlikely in practice that I don't > think it is really worth complicating the code for it. Also the only > downside is we will loose the information there is some error in the > filesystem - we'll soon find that out again anyway :). > I think so, also add a error message if we failed to restoring the error information, it could let us know what happened. Thanks, Yi.
