| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 09 Jul 2014 10:59:18 +0800
From: Chao Yu <chao2.yu@...sung.com>
To: 'Jaegeuk Kim' <jaegeuk@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-f2fs-devel@...ts.sourceforge.net
Subject: RE: [f2fs-dev] [PATCH] f2fs: do checkpoint for the renamed inode
Hi Jaegeuk,
> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@...nel.org]
> Sent: Tuesday, July 08, 2014 2:37 PM
> To: Chao Yu
> Cc: linux-kernel@...r.kernel.org; linux-fsdevel@...r.kernel.org;
> linux-f2fs-devel@...ts.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH] f2fs: do checkpoint for the renamed inode
>
> On Mon, Jul 07, 2014 at 11:17:35AM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> > > -----Original Message-----
> > > From: Jaegeuk Kim [mailto:jaegeuk@...nel.org]
> > > Sent: Wednesday, July 02, 2014 6:35 PM
> > > To: linux-kernel@...r.kernel.org; linux-fsdevel@...r.kernel.org;
> > > linux-f2fs-devel@...ts.sourceforge.net
> > > Cc: Jaegeuk Kim
> > > Subject: [f2fs-dev] [PATCH] f2fs: do checkpoint for the renamed inode
> > >
> > > If an inode is renamed, it should be registered as file_lost_pino to conduct
> > > checkpoint at f2fs_sync_file.
> > > Otherwise, the inode cannot be recovered due to no dent_mark in the following
> > > scenario.
> > >
> > > Note that, this scenario is from xfstests/322.
> > >
> > > 1. create "a"
> > > 2. fsync "a"
> > > 3. rename "a" to "b"
> > > 4. fsync "b"
> > > 5. Sudden power-cut
> > >
> > > After recovery is done, "b" should be seen.
> > > However, the result shows "a", since the recovery procedure does not enter
> > > recover_dentry due to no dent_mark.
> >
> > As we know CP is expensive, how about modifying checkpointed value directly
> > in nat cache at step #3? So "b" could be fsynced with dent_mark at step #4.
>
> Yes, I also thought about that.
> But, when handling the nat cache, one of my concerns was the lock granularity.
> Basically each nat cache entry should be covered by its node page lock, so it
> is a little bit complicate to do something easily in f2fs_rename.
>
> So, this patch was to fix the bug with a naive approach.
> And we can write additional patch to enhance this too.
Thanks for your explanation! :)
Regards,
Yu
>
> Thanks,
>
> >
> > >
> > > The reason is like below.
> > > - The nid of "a" is checkpointed during #2, f2fs_sync_file.
> > > - The inode page for "b" produced by #3 is written without dent_mark by
> > > sync_node_pages.
> > >
> > > So, this patch fixes this bug by assinging file_lost_pino to the "a"'s inode.
> > > If the pino is lost, f2fs_sync_file conducts checkpoint, and then recovers
> > > the latest pino and its dentry information for further recovery.
> > >
> > > Signed-off-by: Jaegeuk Kim <jaegeuk@...nel.org>
> > > ---
> > > fs/f2fs/namei.c | 10 ++++------
> > > 1 file changed, 4 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> > > index 96f1dd1..1b3cae0 100644
> > > --- a/fs/f2fs/namei.c
> > > +++ b/fs/f2fs/namei.c
> > > @@ -411,9 +411,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> > > }
> > >
> > > f2fs_set_link(new_dir, new_entry, new_page, old_inode);
> > > - down_write(&F2FS_I(old_inode)->i_sem);
> > > - F2FS_I(old_inode)->i_pino = new_dir->i_ino;
> > > - up_write(&F2FS_I(old_inode)->i_sem);
> > >
> > > new_inode->i_ctime = CURRENT_TIME;
> > > down_write(&F2FS_I(new_inode)->i_sem);
> > > @@ -446,6 +443,10 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> > > }
> > > }
> > >
> > > + down_write(&F2FS_I(old_inode)->i_sem);
> > > + file_lost_pino(old_inode);
> > > + up_write(&F2FS_I(old_inode)->i_sem);
> > > +
> > > old_inode->i_ctime = CURRENT_TIME;
> > > mark_inode_dirty(old_inode);
> > >
> > > @@ -455,9 +456,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> > > if (old_dir != new_dir) {
> > > f2fs_set_link(old_inode, old_dir_entry,
> > > old_dir_page, new_dir);
> > > - down_write(&F2FS_I(old_inode)->i_sem);
> > > - F2FS_I(old_inode)->i_pino = new_dir->i_ino;
> > > - up_write(&F2FS_I(old_inode)->i_sem);
> > > update_inode_page(old_inode);
> > > } else {
> > > kunmap(old_dir_page);
> > > --
> > > 1.8.5.2 (Apple Git-48)
> > >
> > >
> > > ------------------------------------------------------------------------------
> > > Open source business process management suite built on Java and Eclipse
> > > Turn processes into business applications with Bonita BPM Community Edition
> > > Quickly connect people, data, and systems into organized workflows
> > > Winner of BOSSIE, CODIE, OW2 and Gartner awards
> > > http://p.sf.net/sfu/Bonitasoft
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > Linux-f2fs-devel@...ts.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
> --
> Jaegeuk Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists