On Sun, Nov 14, 2021 at 6:54 PM harshad shirwadkar <harshadshirwadkar@xxxxxxxxx> wrote: > > Hi Xin, > > Thanks for your email and steps to reproduce the issue. > > > But fast commit did change the behavior of fsync in ext4, is this as expected ? > No this is not expected. Fast commits should not change behavior of > fsync, so thanks for spotting it. > > After taking a deeper look at the issue, I think the problem is that > fast commit intentionally avoids committing directory inodes and > instead just records that "file F has been added to / deleted from > directory D". The recovery code does the actual work of updating the > directory . It saves us space in the precious fast commit area. > > While it is okay to skip "addition of dentry" or "deletion of dentry" > events on a directory, it is not okay to skip "creation of directory". To clarify and rephrase this better: While it is okay to skip recording of directory inode during addition of dentry or deletion of dentry events on a directory (which is the reason why ext4_fc_track_link() and ext4_fc_track_unlink() pass "enqueue = 0" to ext4_fc_track_template()), it is not okay to skip recording of the directory inode during creation of directory event - which is what fast commit code was unintentionally doing. > So, you're right, we should be passing "enqueue = 1" to > ext4_fc_track_template() which would tell it to also add the inode to > "the modified inodes" queue. Once the inode is in the modified inode > queue, commit routine first commits the inode and records addition of > dentry to its parent inode. > > Please feel free to send a patch to fix this. > > Thanks, > Harshad > > > On Wed, Nov 10, 2021 at 11:10 PM 尹欣 <yinxin.x@xxxxxxxxxxxxx> wrote: > > > > Hi, > > > > > > Recently, I‘m doing some testing with fast commit feature , and found > > there is a slight difference on fsync compared with the normal > > journaling scheme. > > > > Here is the 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. > > > > Refer to the description of fsync [1], fsync will not guarantee the > > parent directory to be persisted. So I think it is not an issue. > > > > But fast commit did change the behavior of fsync in ext4, is this as expected ? > > > > > > > > For the root cause of this difference, I found that fast commit will > > not add a EXT4_FC_TAG_CREAT tag for directory creation. > > > > In func ext4_fc_commit_dentry_updates(), only directories in s_fc_q > > list can be added with EXT4_FC_TAG_CREAT,but seams the newly created > > directory inode has no change to be added to s_fc_q. > > > > > > > > Shall we just change the “enqueue” param of ext4_fc_track_template() > > to 1 , which in __ext4_fc_track_create()? And make fast commit record > > all the inode creation, and do the same things as normal journaling. > > > > > > > > [1] https://man7.org/linux/man-pages/man2/fdatasync.2.html > > > > > > > > BR, > > > > Xin Yin
