On Wed, Oct 16, 2019 at 10:30 AM Theodore Y. Ts'o <tytso@xxxxxxx> wrote: > > On Tue, Oct 01, 2019 at 12:40:54AM -0700, Harshad Shirwadkar wrote: > > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c > > index 14d549445418..e0684212384d 100644 > > --- a/fs/jbd2/journal.c > > +++ b/fs/jbd2/journal.c > > > > jbd2_write_superblock(journal, write_op); > > > > + if (had_fast_commit) > > + jbd2_set_feature_fast_commit(journal); > > + > > Why the logic with had_fast_commit and (re-)setting the fast commit > feature flag? > > This ties back to how we handle the logic around setting the fast > commit flag if requested by the file system.... Fast commit feature flag serves 2 purposes: 1) If the flag is turned on in on-disk superblock, it means that the superblock contains fast commit blocks that should be replayed. 2) If the flag is turned on in the in-memory representation of the superblock, it serves as an indicator for the rest of the JBD2 code that fast commit feature is enabled. Based on that flag, for example, the journal thread decides to try fast commits. In this particular case, since the journal is empty we don't want to commit fast commit feature flag on-disk but we want to retain that flag in in-memory structure. > > > @@ -768,6 +816,8 @@ static int do_one_pass(journal_t *journal, > > if (err) > > goto failed; > > continue; > > + case JBD2_FC_BLOCK: > > + continue; > > Why should a Fast Commit block ever show up in the primary part of the > journal? It should never happen, right? That's right, I'll fix this in next version. > > In which case, we should probably at least issue a warning, and not > just skip the block. > > - Ted
