Thanks for the explanation and direction , I will do another fix for this issue. Best Regards, Xin Yin On Thu, Jan 6, 2022 at 11:22 AM Theodore Ts'o <tytso@xxxxxxx> wrote: > > On Fri, Dec 24, 2021 at 02:57:28PM +0800, Xin Yin wrote: > > For the follow test example: > > -mkdir test/ > > -create&write test/a.txt > > -fsync test/a.txt > > -crash (before a full commit) > > > > If fast commit is used then "a.txt" will lost, while the normal > > journaling can recover it. > > The problem is that your proposed fix: > > > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > > index 3deb97b22ca4..4b843648ffe5 100644 > > --- a/fs/ext4/fast_commit.c > > +++ b/fs/ext4/fast_commit.c > > @@ -423,7 +423,7 @@ void __ext4_fc_track_create(handle_t *handle, struct inode *inode, > > args.op = EXT4_FC_TAG_CREAT; > > > > ret = ext4_fc_track_template(handle, inode, __track_dentry_update, > > - (void *)&args, 0); > > + (void *)&args, 1); > > trace_ext4_fc_track_create(inode, dentry, ret); > > } > > affects both file creations as well as directory creations (mkdir). > Putting the inode on the fast commit list is something that is meant > for files, and means that on a fast commit we need to force the data > blocks out. So it seems that isn't the right fix for the problem. > > Why do something really simple? Look at the parent directory's inode, > and check its i_sync_tid. If it's not equal to > handle->h_transaction->t_tid, then it's safe to do the fast commit. > If it's equal to the current transaction, we can either force a full > commit. > > Optionally, in the case where i_sync_tid == current tid, since there's > a chance that the parent directory's inode could have been freshly > fetched from disk (see __ext4_iget() in fs/ext4/inode.c), we could > compare its i_crtime against ktime_get_real_seconds(), and if the > inode was created in the last 2*journal->j_commit_interval/HZ seconds, > it's safe to do a fast commit; otherwise do a full commit. > > Cheers, > > - Ted
