[PATCH 1/2] ceph: remove ceph_lookup_inode()

[PATCH 1/2] ceph: remove ceph_lookup_inode()

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

commit 6f60f889 (ceph: fix freeing inode vs removing session caps race)
introduced ceph_lookup_inode(). But there is already a ceph_find_inode()
which provides similar function. So remove ceph_lookup_inode(), use
ceph_find_inode() instead.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/inode.c      | 8 --------
 fs/ceph/mds_client.c | 2 +-
 fs/ceph/super.h      | 2 --
 3 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index eae41cd..8549a48 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -62,14 +62,6 @@ struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino)
 	return inode;
 }
 
-struct inode *ceph_lookup_inode(struct super_block *sb, struct ceph_vino vino)
-{
-	struct inode *inode;
-	ino_t t = ceph_vino_to_ino(vino);
-	inode = ilookup5_nowait(sb, t, ceph_ino_compare, &vino);
-	return inode;
-}
-
 /*
  * get/constuct snapdir inode for a given directory
  */
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 603786b..b7bda5d 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1054,7 +1054,7 @@ static void remove_session_caps(struct ceph_mds_session *session)
 			vino = cap->ci->i_vino;
 			spin_unlock(&session->s_cap_lock);
 
-			inode = ceph_lookup_inode(sb, vino);
+			inode = ceph_find_inode(sb, vino);
 			iput(inode);
 
 			spin_lock(&session->s_cap_lock);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index bb23ef6..6014b0a 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -694,8 +694,6 @@ extern void ceph_destroy_inode(struct inode *inode);
 
 extern struct inode *ceph_get_inode(struct super_block *sb,
 				    struct ceph_vino vino);
-extern struct inode *ceph_lookup_inode(struct super_block *sb,
-				       struct ceph_vino vino);
 extern struct inode *ceph_get_snapdir(struct inode *parent);
 extern int ceph_fill_file_size(struct inode *inode, int issued,
 			       u32 truncate_seq, u64 truncate_size, u64 size);
-- 
1.8.1.4

[PATCH 2/2] ceph: use d_invalidate() to invalidate aliases

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

d_invalidate() is the standard VFS method to invalidate dentry.
compare to d_delete(), it also try shrinking children dentries.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/caps.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 7b451eb..13976c3 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -2346,10 +2346,10 @@ static void invalidate_aliases(struct inode *inode)
 	d_prune_aliases(inode);
 	/*
 	 * For non-directory inode, d_find_alias() only returns
-	 * connected dentry. After calling d_delete(), the dentry
-	 * become disconnected.
+	 * connected dentry. After calling d_invalidate(), the
+	 * dentry become disconnected.
 	 *
-	 * For directory inode, d_find_alias() only can return
+	 * For directory inode, d_find_alias() can return
 	 * disconnected dentry. But directory inode should have
 	 * one alias at most.
 	 */
@@ -2358,7 +2358,7 @@ static void invalidate_aliases(struct inode *inode)
 			dput(dn);
 			break;
 		}
-		d_delete(dn);
+		d_invalidate(dn);
 		if (prev)
 			dput(prev);
 		prev = dn;
-- 
1.8.1.4

Re: [PATCH 2/2] ceph: use d_invalidate() to invalidate aliases

[Alex Elder]

On 09/02/2013 02:19 AM, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 
> d_invalidate() is the standard VFS method to invalidate dentry.
> compare to d_delete(), it also try shrinking children dentries.

I'm less familiar with this code than rbd and the osd client,
so you probably want another reviewer.
I have one comment though, below.

> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> ---
>  fs/ceph/caps.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 7b451eb..13976c3 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2346,10 +2346,10 @@ static void invalidate_aliases(struct inode *inode)
>  	d_prune_aliases(inode);
>  	/*
>  	 * For non-directory inode, d_find_alias() only returns
> -	 * connected dentry. After calling d_delete(), the dentry
> -	 * become disconnected.
> +	 * connected dentry. After calling d_invalidate(), the
> +	 * dentry become disconnected.
>  	 *
> -	 * For directory inode, d_find_alias() only can return
> +	 * For directory inode, d_find_alias() can return
>  	 * disconnected dentry. But directory inode should have
>  	 * one alias at most.
>  	 */
> @@ -2358,7 +2358,7 @@ static void invalidate_aliases(struct inode *inode)
>  			dput(dn);
>  			break;
>  		}
> -		d_delete(dn);
> +		d_invalidate(dn);

Unlike d_delete(), d_invalidate() can return an error, and in
particular will return an error if there are other dentries
reachable from the one being invalidated.  Do you need to check
for and handle an error here?

					-Alex

>  		if (prev)
>  			dput(prev);
>  		prev = dn;
> 

Re: [PATCH 1/2] ceph: remove ceph_lookup_inode()

[Alex Elder]

On 09/02/2013 02:19 AM, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 
> commit 6f60f889 (ceph: fix freeing inode vs removing session caps race)
> introduced ceph_lookup_inode(). But there is already a ceph_find_inode()
> which provides similar function. So remove ceph_lookup_inode(), use
> ceph_find_inode() instead.
> 
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>

Looks good.

Reviewed-by: Alex Elder <elder@linaro.org>

> ---
>  fs/ceph/inode.c      | 8 --------
>  fs/ceph/mds_client.c | 2 +-
>  fs/ceph/super.h      | 2 --
>  3 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index eae41cd..8549a48 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -62,14 +62,6 @@ struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino)
>  	return inode;
>  }
>  
> -struct inode *ceph_lookup_inode(struct super_block *sb, struct ceph_vino vino)
> -{
> -	struct inode *inode;
> -	ino_t t = ceph_vino_to_ino(vino);
> -	inode = ilookup5_nowait(sb, t, ceph_ino_compare, &vino);
> -	return inode;
> -}
> -
>  /*
>   * get/constuct snapdir inode for a given directory
>   */
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 603786b..b7bda5d 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -1054,7 +1054,7 @@ static void remove_session_caps(struct ceph_mds_session *session)
>  			vino = cap->ci->i_vino;
>  			spin_unlock(&session->s_cap_lock);
>  
> -			inode = ceph_lookup_inode(sb, vino);
> +			inode = ceph_find_inode(sb, vino);
>  			iput(inode);
>  
>  			spin_lock(&session->s_cap_lock);
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index bb23ef6..6014b0a 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -694,8 +694,6 @@ extern void ceph_destroy_inode(struct inode *inode);
>  
>  extern struct inode *ceph_get_inode(struct super_block *sb,
>  				    struct ceph_vino vino);
> -extern struct inode *ceph_lookup_inode(struct super_block *sb,
> -				       struct ceph_vino vino);
>  extern struct inode *ceph_get_snapdir(struct inode *parent);
>  extern int ceph_fill_file_size(struct inode *inode, int issued,
>  			       u32 truncate_seq, u64 truncate_size, u64 size);
>