Messages in this thread | | | Subject | Re: [PATCH] f2fs: fix fdatasync | From | Chao Yu <> | Date | Thu, 17 Nov 2016 10:51:37 +0800 |
| |
Hi Jaegeuk,
On 2016/11/17 3:13, Jaegeuk Kim wrote: > Hi Chao, > > On Wed, Nov 16, 2016 at 08:12:11PM +0800, Chao Yu wrote: >> For below two cases, we can't guarantee data consistence: >> >> a) >> 1. xfs_io "pwrite 0 4195328" "fsync" >> 2. xfs_io "pwrite 4195328 1024" "fdatasync" >> 3. godown >> 4. umount & mount >> --> isize we updated before fdatasync won't be recovered >> >> b) >> 1. xfs_io "pwrite -S 0xcc 0 4202496" "fsync" >> 2. xfs_io "fpunch 4194304 4096" "fdatasync" >> 3. godown >> 4. umount & mount >> --> dnode we punched before fdatasync won't be recovered >> >> The reason is that normally fdatasync won't be aware of modification >> of metadata in file, e.g. isize changing, dnode updating, so in ->fsync >> we will skip flushing node pages for above cases, result in making >> fdatasynced file being lost during recovery. >> >> Introduce FDATASYNC_INO global ino cache for tracking node changing, >> later fdatasync choose to flush nodes depend on ino cache state. > > We don't need to add this additionally, and would be better to consider other > major metadata as well. > > How about this?
Seems it can't track file after evict?
Thanks,
> > --- > fs/f2fs/f2fs.h | 11 ++++++++++- > fs/f2fs/file.c | 2 +- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index c9672a3..50ffa4f 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -1727,8 +1727,17 @@ static inline void f2fs_i_size_write(struct inode *inode, loff_t i_size) > set_inode_flag(inode, FI_AUTO_RECOVER); > } > > -static inline bool f2fs_skip_inode_update(struct inode *inode) > +static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > { > + if (dsync) { > + struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > + bool ret; > + > + spin_lock(&sbi->inode_lock[DIRTY_META]); > + ret = list_empty(&F2FS_I(inode)->gdirty_list); > + spin_unlock(&sbi->inode_lock[DIRTY_META]); > + return ret; > + } > if (!is_inode_flag_set(inode, FI_AUTO_RECOVER)) > return false; > return F2FS_I(inode)->last_disk_size == i_size_read(inode); > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index d48c120..50123c6 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -212,7 +212,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end, > } > > /* if the inode is dirty, let's recover all the time */ > - if (!datasync && !f2fs_skip_inode_update(inode)) { > + if (!f2fs_skip_inode_update(inode, datasync)) { > f2fs_write_inode(inode, NULL); > goto go_write; > } >
| |