lkml.org 
[lkml]   [2022]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [External] Re: [PATCH RESEND] ext4:fix different behavior of fsync when use fast commit
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@mit.edu> 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

\
 
 \ /
  Last update: 2022-01-07 09:49    [W:0.117 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site