On Wed 15-03-23 20:37:32, Zhang Yi wrote: > On 2023/3/15 17:48, Jan Kara wrote: > > On Tue 14-03-23 22:05:21, Zhang Yi wrote: > >> From: Zhang Yi <yi.zhang@xxxxxxxxxx> > >> > >> For a newly mounted file system, the journal committing thread always > >> record new transactions from the start of the journal area, no matter > >> whether the journal was clean or just has been recovered. So the logdump > >> code in debugfs cannot dump continuous logs between each mount, it is > >> disadvantageous to analysis corrupted file system image and locate the > >> file system inconsistency bugs. > >> > >> If we get a corrupted file system in the running products and want to > >> find out what has happened, besides lookup the system log, one effective > >> way is to backtrack the journal log. But we may not always run e2fsck > >> before each mount and the default fsck -a mode also cannot always > >> checkout all inconsistencies, so it could left over some inconsistencies > >> into the next mount until we detect it. Finally, transactions in the > >> journal may probably discontinuous and some relatively new transactions > >> has been covered, it becomes hard to analyse. If we could record > >> transactions continuously between each mount, we could acquire more > >> useful info from the journal. Like this: > >> > >> |Previous mount checkpointed/recovered logs|Current mount logs | > >> |{------}{---}{--------} ... {------}| ... |{======}{========}...000000| > >> > >> And yes the journal area is limited and cannot record everything, the > >> problematic transaction may also be covered even if we do this, but > >> this is still useful for fuzzy tests and short-running products. > >> > >> This patch save the head blocknr in the superblock after flushing the > >> journal or unmounting the file system, let the next mount could continue > >> to record new transaction behind it. This change is backward compatible > >> because the old kernel does not care about the head blocknr of the > >> journal. It is also fine if we mount a clean old image without valid > >> head blocknr, we fail back to set it to s_first just like before. > >> Finally, for the case of mount an unclean file system, we could also get > >> the journal head easily after scanning/replaying the journal, it will > >> continue to record new transaction after the recovered transactions. > >> > >> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx> > > > > I like this implementation! I even think we could perhaps make ext4 always > > behave this way to not increase size of the test matrix. Or do you see any > > downside to this option? > > > > Thanks for your suggestion. Indeed, I don't find any side effect on this > option both in theory and in the actual use tests on ext4, I added a new > option was just from the safe point of view and let user could disable it if > they don't want it. I also prefer to make ext4 always behave this way.:) > > I would like to keep the JBD2_CYCLE_RECORD flag(ocfs2 also use jbd2, I don't > want to disturb it until it needs), remove EXT4_MOUNT2_JOURNAL_CYCLE_RECORD > and always set JBD2_CYCLE_RECORD on ext4 in patch 2 in the next iteration. Yes, that makes sense. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR