Messages in this thread | | | Date | Wed, 16 Nov 2016 18:59:11 -0800 | From | Jaegeuk Kim <> | Subject | Re: [PATCH] f2fs: fix fdatasync |
| |
On Thu, Nov 17, 2016 at 10:51:37AM +0800, Chao Yu wrote: > 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?
Do we need that? That means inode page was already up-to-date?
> > 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; > > } > >
| |