[1/2] ceph: use ceph_mdsc_build_path instead of clone_dentry_name
diff mbox series

Message ID 20190429192554.30833-1-jlayton@kernel.org
State New
Headers show
Series
  • [1/2] ceph: use ceph_mdsc_build_path instead of clone_dentry_name
Related show

Commit Message

Jeff Layton April 29, 2019, 7:25 p.m. UTC
While it may be slightly more efficient, it's probably not worthwhile to
optimize for the case that clone_dentry_name handles. We can get the
same result by just calling ceph_mdsc_build_path when the parent isn't
locked, with less code duplication.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/mds_client.c | 41 +++--------------------------------------
 1 file changed, 3 insertions(+), 38 deletions(-)

Comments

Yan, Zheng April 30, 2019, 9:32 a.m. UTC | #1
On Tue, Apr 30, 2019 at 3:27 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> While it may be slightly more efficient, it's probably not worthwhile to
> optimize for the case that clone_dentry_name handles. We can get the
> same result by just calling ceph_mdsc_build_path when the parent isn't
> locked, with less code duplication.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/mds_client.c | 41 +++--------------------------------------
>  1 file changed, 3 insertions(+), 38 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 74cb3078ea63..e8245df09691 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2170,55 +2170,20 @@ char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
>         return path;
>  }
>
> -/* Duplicate the dentry->d_name.name safely */
> -static int clone_dentry_name(struct dentry *dentry, const char **ppath,
> -                            int *ppathlen)
> -{
> -       u32 len;
> -       char *name;
> -retry:
> -       len = READ_ONCE(dentry->d_name.len);
> -       name = kmalloc(len + 1, GFP_NOFS);
> -       if (!name)
> -               return -ENOMEM;
> -
> -       spin_lock(&dentry->d_lock);
> -       if (dentry->d_name.len != len) {
> -               spin_unlock(&dentry->d_lock);
> -               kfree(name);
> -               goto retry;
> -       }
> -       memcpy(name, dentry->d_name.name, len);
> -       spin_unlock(&dentry->d_lock);
> -
> -       name[len] = '\0';
> -       *ppath = name;
> -       *ppathlen = len;
> -       return 0;
> -}
> -
>  static int build_dentry_path(struct dentry *dentry, struct inode *dir,
>                              const char **ppath, int *ppathlen, u64 *pino,
>                              bool *pfreepath, bool parent_locked)
>  {
> -       int ret;
>         char *path;
>
>         rcu_read_lock();
>         if (!dir)
>                 dir = d_inode_rcu(dentry->d_parent);
> -       if (dir && ceph_snap(dir) == CEPH_NOSNAP) {
> +       if (dir && parent_locked && ceph_snap(dir) == CEPH_NOSNAP) {
>                 *pino = ceph_ino(dir);
>                 rcu_read_unlock();
> -               if (parent_locked) {
> -                       *ppath = dentry->d_name.name;
> -                       *ppathlen = dentry->d_name.len;
> -               } else {
> -                       ret = clone_dentry_name(dentry, ppath, ppathlen);
> -                       if (ret)
> -                               return ret;
> -                       *pfreepath = true;
> -               }
> +               *ppath = dentry->d_name.name;
> +               *ppathlen = dentry->d_name.len;
>                 return 0;
>         }
>         rcu_read_unlock();


Reviewed-by: "Yan, Zheng" <zyan@redhat.com>

> --
> 2.20.1
>

Patch
diff mbox series

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 74cb3078ea63..e8245df09691 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2170,55 +2170,20 @@  char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
 	return path;
 }
 
-/* Duplicate the dentry->d_name.name safely */
-static int clone_dentry_name(struct dentry *dentry, const char **ppath,
-			     int *ppathlen)
-{
-	u32 len;
-	char *name;
-retry:
-	len = READ_ONCE(dentry->d_name.len);
-	name = kmalloc(len + 1, GFP_NOFS);
-	if (!name)
-		return -ENOMEM;
-
-	spin_lock(&dentry->d_lock);
-	if (dentry->d_name.len != len) {
-		spin_unlock(&dentry->d_lock);
-		kfree(name);
-		goto retry;
-	}
-	memcpy(name, dentry->d_name.name, len);
-	spin_unlock(&dentry->d_lock);
-
-	name[len] = '\0';
-	*ppath = name;
-	*ppathlen = len;
-	return 0;
-}
-
 static int build_dentry_path(struct dentry *dentry, struct inode *dir,
 			     const char **ppath, int *ppathlen, u64 *pino,
 			     bool *pfreepath, bool parent_locked)
 {
-	int ret;
 	char *path;
 
 	rcu_read_lock();
 	if (!dir)
 		dir = d_inode_rcu(dentry->d_parent);
-	if (dir && ceph_snap(dir) == CEPH_NOSNAP) {
+	if (dir && parent_locked && ceph_snap(dir) == CEPH_NOSNAP) {
 		*pino = ceph_ino(dir);
 		rcu_read_unlock();
-		if (parent_locked) {
-			*ppath = dentry->d_name.name;
-			*ppathlen = dentry->d_name.len;
-		} else {
-			ret = clone_dentry_name(dentry, ppath, ppathlen);
-			if (ret)
-				return ret;
-			*pfreepath = true;
-		}
+		*ppath = dentry->d_name.name;
+		*ppathlen = dentry->d_name.len;
 		return 0;
 	}
 	rcu_read_unlock();