linux-ext4 - Re: [Question] ext4: different behavior of fsync when use fast commit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD+ocbyQQym54ZkY9Jdidju_bw9Pj32sGM3qigABCu-Uj3n4Hw@mail.gmail.com>
Date:   Sun, 14 Nov 2021 18:54:00 -0800
From:   harshad shirwadkar <harshadshirwadkar@...il.com>
To:     尹欣 <yinxin.x@...edance.com>
Cc:     "Theodore Y. Ts'o" <tytso@....edu>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [Question] ext4: different behavior of fsync when use fast commit

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".
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@...edance.com> 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