Thanks, you are right , the EXT4_MF_FC_COMMITTING is not needed after this change , I will do the cleanup for it , and send a v2 patches set. Thanks, Xin Yin On Sat, Jan 8, 2022 at 4:14 AM harshad shirwadkar <harshadshirwadkar@xxxxxxxxx> wrote: > > Makes sense. With this change, we don't really need > EXT4_MF_FC_COMMITTING flag anymore. So, we can drop it. But other than > that, this patch looks good. > > Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@xxxxxxxxx> > > On Fri, Jan 7, 2022 at 4:13 AM Xin Yin <yinxin.x@xxxxxxxxxxxxx> wrote: > > > > in the follow scenario: > > 1. jbd start transaction n > > 2. task A get new handle for transaction n+1 > > 3. task A do some actions and add inode to FC_Q_MAIN fc_q > > 4. jbd complete transaction n and clear FC_Q_MAIN fc_q > > 5. task A call fsync > > > > fast commit will lost the file actions during a full commit. > > > > we should also add updates to staging queue during a full commit. > > and in ext4_fc_cleanup(), when reset a inode's fc track range, check > > it's i_sync_tid, if it bigger than current transaction tid, do not > > rest it, or we will lost the track range. > > > > Signed-off-by: Xin Yin <yinxin.x@xxxxxxxxxxxxx> > > --- > > fs/ext4/fast_commit.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > > index 3673d4798af3..4cea92aec7c4 100644 > > --- a/fs/ext4/fast_commit.c > > +++ b/fs/ext4/fast_commit.c > > @@ -365,7 +365,8 @@ static int ext4_fc_track_template( > > spin_lock(&sbi->s_fc_lock); > > if (list_empty(&EXT4_I(inode)->i_fc_list)) > > list_add_tail(&EXT4_I(inode)->i_fc_list, > > - (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_COMMITTING)) ? > > + (sbi->s_journal->j_flags & JBD2_FULL_COMMIT_ONGOING || > > + sbi->s_journal->j_flags & JBD2_FAST_COMMIT_ONGOING) ? > > &sbi->s_fc_q[FC_Q_STAGING] : > > &sbi->s_fc_q[FC_Q_MAIN]); > > spin_unlock(&sbi->s_fc_lock); > > @@ -418,7 +419,8 @@ static int __track_dentry_update(struct inode *inode, void *arg, bool update) > > node->fcd_name.len = dentry->d_name.len; > > > > spin_lock(&sbi->s_fc_lock); > > - if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_COMMITTING)) > > + if (sbi->s_journal->j_flags & JBD2_FULL_COMMIT_ONGOING || > > + sbi->s_journal->j_flags & JBD2_FAST_COMMIT_ONGOING) > > list_add_tail(&node->fcd_list, > > &sbi->s_fc_dentry_q[FC_Q_STAGING]); > > else > > @@ -1202,7 +1204,8 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) > > list_del_init(&iter->i_fc_list); > > ext4_clear_inode_state(&iter->vfs_inode, > > EXT4_STATE_FC_COMMITTING); > > - ext4_fc_reset_inode(&iter->vfs_inode); > > + if (iter->i_sync_tid <= tid) > > + ext4_fc_reset_inode(&iter->vfs_inode); > > /* Make sure EXT4_STATE_FC_COMMITTING bit is clear */ > > smp_mb(); > > #if (BITS_PER_LONG < 64) > > -- > > 2.20.1 > >
