Re: [PATCH 09/20] ceph_d_revalidate(): use stable parent inode passed by caller

Viacheslav Dubeyko
2025-01-10T19:45:55Z

On Fri, 2025-01-10 at 02:42 +0000, Al Viro wrote:
> No need to mess with the boilerplate for obtaining what we already
> have.  Note that ceph is one of the "will want a path from filesystem
> root if we want to talk to server" cases, so the name of the last
> component is of little use - it is passed to fscrypt_d_revalidate()
> and it's used to deal with (also crypt-related) case in request
> marshalling, when encrypted name turns out to be too long.  The
> former
> is not a problem, but the latter is racy; that part will be handled
> in the next commit.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/ceph/dir.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index c4c71c24221b..dc5f55bebad7 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1940,30 +1940,19 @@ static int dir_lease_is_valid(struct inode
> *dir, struct dentry *dentry,
>  /*
>   * Check if cached dentry can be trusted.
>   */
> -static int ceph_d_revalidate(struct inode *parent_dir, const struct
> qstr *name,
> +static int ceph_d_revalidate(struct inode *dir, const struct qstr
> *name,
>        struct dentry *dentry, unsigned int
> flags)
>  {
>   struct ceph_mds_client *mdsc = ceph_sb_to_fs_client(dentry-
> >d_sb)->mdsc;
>   struct ceph_client *cl = mdsc->fsc->client;
>   int valid = 0;
> - struct dentry *parent;
> - struct inode *dir, *inode;
> + struct inode *inode;
>
> - valid = fscrypt_d_revalidate(parent_dir, name, dentry,
> flags);
> + valid = fscrypt_d_revalidate(dir, name, dentry, flags);
>   if (valid <= 0)
>   return valid;
>
> - if (flags & LOOKUP_RCU) {
> - parent = READ_ONCE(dentry->d_parent);
> - dir = d_inode_rcu(parent);
> - if (!dir)
> - return -ECHILD;
> - inode = d_inode_rcu(dentry);
> - } else {
> - parent = dget_parent(dentry);
> - dir = d_inode(parent);
> - inode = d_inode(dentry);
> - }
> + inode = d_inode_rcu(dentry);
>
>   doutc(cl, "%p '%pd' inode %p offset 0x%llx nokey %d\n",
>         dentry, dentry, inode, ceph_dentry(dentry)->offset,
> @@ -2039,9 +2028,6 @@ static int ceph_d_revalidate(struct inode
> *parent_dir, const struct qstr *name,
>   doutc(cl, "%p '%pd' %s\n", dentry, dentry, valid ? "valid" :
> "invalid");
>   if (!valid)
>   ceph_dir_clear_complete(dir);
> -
> - if (!(flags & LOOKUP_RCU))
> - dput(parent);
>   return valid;
>  }
>

Looks much better now.

Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>

Thanks,
Slava.