Messages in this thread | | | Date | Wed, 6 May 2020 08:14:07 +0800 | From | Gao Xiang <> | Subject | Re: [f2fs-dev] [PATCH] f2fs: get parent inode when recovering pino |
| |
Hi Eric,
On Tue, May 05, 2020 at 11:19:41AM -0700, Eric Biggers wrote: > On Tue, May 05, 2020 at 11:13:23AM -0700, Jaegeuk Kim wrote:
...
> > > > -static int get_parent_ino(struct inode *inode, nid_t *pino) > > -{ > > - struct dentry *dentry; > > - > > - inode = igrab(inode); > > - dentry = d_find_any_alias(inode); > > - iput(inode); > > - if (!dentry) > > - return 0; > > - > > - *pino = parent_ino(dentry); > > - dput(dentry); > > - return 1; > > -} > > - > > static inline enum cp_reason_type need_do_checkpoint(struct inode *inode) > > { > > struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > > @@ -223,14 +208,15 @@ static bool need_inode_page_update(struct f2fs_sb_info *sbi, nid_t ino) > > return ret; > > } > > > > -static void try_to_fix_pino(struct inode *inode) > > +static void try_to_fix_pino(struct dentry *dentry) > > { > > + struct inode *inode = d_inode(dentry); > > struct f2fs_inode_info *fi = F2FS_I(inode); > > - nid_t pino; > > > > down_write(&fi->i_sem); > > - if (file_wrong_pino(inode) && inode->i_nlink == 1 && > > - get_parent_ino(inode, &pino)) { > > + if (file_wrong_pino(inode) && inode->i_nlink == 1) { > > + nid_t pino = parent_ino(dentry); > > + > > f2fs_i_pino_write(inode, pino); > > file_got_pino(inode); > > } > > @@ -310,7 +296,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end, > > * We've secured consistency through sync_fs. Following pino > > * will be used only for fsynced inodes after checkpoint. > > */ > > - try_to_fix_pino(inode); > > + try_to_fix_pino(file_dentry(file)); > > clear_inode_flag(inode, FI_APPEND_WRITE); > > clear_inode_flag(inode, FI_UPDATE_WRITE); > > goto out; > > Actually, I think this is wrong because the fsync can be done via a file > descriptor that was opened to a now-deleted link to the file.
I'm still confused about this...
I don't know what's wrong with this version from my limited knowledge? inode itself is locked when fsyncing, so
if the fsync inode->i_nlink == 1, this inode has only one hard link (not deleted yet) and should belong to a single directory; and
the only one parent directory would not go away (not deleted as well) since there are some dirents in it (not empty).
Could kindly explain more so I would learn more about this scenario? Thanks a lot!
> > We need to find the dentry whose parent directory is still exists, i.e. the > parent directory that is counting towards 'inode->i_nlink == 1'.
directory counting towards 'inode->i_nlink == 1', what's happening?
> > I think d_find_alias() is what we're looking for.
It may be simply dentry->d_parent (stable/positive as you said before, and it's not empty). why need to d_find_alias()?
And what is the original problem? I could not get some clue from the original patch description (I only saw some extra igrab/iput because of some unknown reasons), it there some backtrace related to the problem?
Thanks, Gao Xiang
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 6ab8f621a3c5..855f27468baa 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -165,13 +165,13 @@ static int get_parent_ino(struct inode *inode, nid_t *pino) > { > struct dentry *dentry; > > - inode = igrab(inode); > - dentry = d_find_any_alias(inode); > - iput(inode); > + dentry = d_find_alias(inode); > if (!dentry) > return 0; > > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
| |