MIME-Version: 1.0
References: <CAK896s5szH8qbDGBDo74hyz5iM4QrjbMJkMjh-f1sZh6K0ozTg@mail.gmail.com>
In-Reply-To: <CAK896s5szH8qbDGBDo74hyz5iM4QrjbMJkMjh-f1sZh6K0ozTg@mail.gmail.com>
From:   harshad shirwadkar <harshadshirwadkar@gmail.com>
Date:   Sun, 14 Nov 2021 18:54:00 -0800
Message-ID: <CAD+ocbyQQym54ZkY9Jdidju_bw9Pj32sGM3qigABCu-Uj3n4Hw@mail.gmail.com>
Subject: Re: [Question] ext4: different behavior of fsync when use fast commit
To:     =?UTF-8?B?5bC55qyj?= <yinxin.x@bytedance.com>
Cc:     "Theodore Y. Ts'o" <tytso@mit.edu>,
        Ext4 Developers List <linux-ext4@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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 exp=
ected ?
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 =3D 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 =E5=B0=B9=E6=AC=A3 <yinxin.x@bytedance.com=
> wrote:
>
> Hi,
>
>
> Recently, I=E2=80=98m doing some testing with fast commit feature , and f=
ound
> 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 =E2=80=9Ca.txt=E2=80=9D 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 exp=
ected ?
>
>
>
> 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 =E2=80=9Cenqueue=E2=80=9D 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