From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaegeuk Kim Subject: Re: [PATCH] f2fs: fix fdatasync Date: Wed, 16 Nov 2016 18:59:11 -0800 Message-ID: <20161117025911.GA67852@jaegeuk> References: <20161116121211.11790-1-yuchao0@huawei.com> <20161116191312.GA57609@jaegeuk> <25a9f225-ba22-d0cd-54ff-4a75031bb161@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from sog-mx-2.v43.ch3.sourceforge.com ([172.29.43.192] helo=mx.sourceforge.net) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1c7Cv2-0006Fi-Jp for linux-f2fs-devel@lists.sourceforge.net; Thu, 17 Nov 2016 02:59:20 +0000 Received: from mail.kernel.org ([198.145.29.136]) by sog-mx-2.v43.ch3.sourceforge.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.76) id 1c7Cv1-00077Q-HZ for linux-f2fs-devel@lists.sourceforge.net; Thu, 17 Nov 2016 02:59:20 +0000 Content-Disposition: inline In-Reply-To: <25a9f225-ba22-d0cd-54ff-4a75031bb161@huawei.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net To: Chao Yu Cc: chao@kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net 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; > > } > > ------------------------------------------------------------------------------