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-de...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
