diff mbox series

[RFC,5/6] ceph: don't take s_mutex in ceph_flush_snaps

Message ID 20210615145730.21952-6-jlayton@kernel.org (mailing list archive)
State New, archived
Headers show
Series ceph: remove excess mutex locking from cap/snap flushing codepaths | expand

Commit Message

Jeff Layton June 15, 2021, 2:57 p.m. UTC
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/caps.c | 8 +-------
 fs/ceph/snap.c | 4 +---
 2 files changed, 2 insertions(+), 10 deletions(-)

Comments

Luis Henriques June 16, 2021, 3:25 p.m. UTC | #1
On Tue, Jun 15, 2021 at 10:57:29AM -0400, Jeff Layton wrote:
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/caps.c | 8 +-------
>  fs/ceph/snap.c | 4 +---
>  2 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index d21b1fa36875..5864d5088e27 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -1531,7 +1531,7 @@ static inline int __send_flush_snap(struct inode *inode,
>   * asynchronously back to the MDS once sync writes complete and dirty
>   * data is written out.
>   *
> - * Called under i_ceph_lock.  Takes s_mutex as needed.
> + * Called under i_ceph_lock.
>   */
>  static void __ceph_flush_snaps(struct ceph_inode_info *ci,
>  			       struct ceph_mds_session *session)
> @@ -1653,7 +1653,6 @@ void ceph_flush_snaps(struct ceph_inode_info *ci,
>  	mds = ci->i_auth_cap->session->s_mds;
>  	if (session && session->s_mds != mds) {
>  		dout(" oops, wrong session %p mutex\n", session);
> -		mutex_unlock(&session->s_mutex);
>  		ceph_put_mds_session(session);
>  		session = NULL;
>  	}
> @@ -1662,10 +1661,6 @@ void ceph_flush_snaps(struct ceph_inode_info *ci,
>  		mutex_lock(&mdsc->mutex);
>  		session = __ceph_lookup_mds_session(mdsc, mds);
>  		mutex_unlock(&mdsc->mutex);
> -		if (session) {
> -			dout(" inverting session/ino locks on %p\n", session);
> -			mutex_lock(&session->s_mutex);
> -		}
>  		goto retry;
>  	}
>  
> @@ -1680,7 +1675,6 @@ void ceph_flush_snaps(struct ceph_inode_info *ci,
>  	if (psession) {
>  		*psession = session;
>  	} else if (session) {
> -		mutex_unlock(&session->s_mutex);
>  		ceph_put_mds_session(session);

nit: since ceph_put_mds_session() now checks for NULL, the 'else' doesn't
really need the condition.

>  	}
>  	/* we flushed them all; remove this inode from the queue */
> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> index f8cac2abab3f..afc7f4c32364 100644
> --- a/fs/ceph/snap.c
> +++ b/fs/ceph/snap.c
> @@ -846,10 +846,8 @@ static void flush_snaps(struct ceph_mds_client *mdsc)
>  	}
>  	spin_unlock(&mdsc->snap_flush_lock);
>  
> -	if (session) {
> -		mutex_unlock(&session->s_mutex);
> +	if (session)
>  		ceph_put_mds_session(session);
> -	}

Same here: the 'if (session)' can be dropped.

>  	dout("flush_snaps done\n");
>  }
>  
> -- 
> 2.31.1
> 

Reviewed-by: Luis Henriques <lhenriques@suse.de>

Cheers,
--
Luís
diff mbox series

Patch

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index d21b1fa36875..5864d5088e27 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -1531,7 +1531,7 @@  static inline int __send_flush_snap(struct inode *inode,
  * asynchronously back to the MDS once sync writes complete and dirty
  * data is written out.
  *
- * Called under i_ceph_lock.  Takes s_mutex as needed.
+ * Called under i_ceph_lock.
  */
 static void __ceph_flush_snaps(struct ceph_inode_info *ci,
 			       struct ceph_mds_session *session)
@@ -1653,7 +1653,6 @@  void ceph_flush_snaps(struct ceph_inode_info *ci,
 	mds = ci->i_auth_cap->session->s_mds;
 	if (session && session->s_mds != mds) {
 		dout(" oops, wrong session %p mutex\n", session);
-		mutex_unlock(&session->s_mutex);
 		ceph_put_mds_session(session);
 		session = NULL;
 	}
@@ -1662,10 +1661,6 @@  void ceph_flush_snaps(struct ceph_inode_info *ci,
 		mutex_lock(&mdsc->mutex);
 		session = __ceph_lookup_mds_session(mdsc, mds);
 		mutex_unlock(&mdsc->mutex);
-		if (session) {
-			dout(" inverting session/ino locks on %p\n", session);
-			mutex_lock(&session->s_mutex);
-		}
 		goto retry;
 	}
 
@@ -1680,7 +1675,6 @@  void ceph_flush_snaps(struct ceph_inode_info *ci,
 	if (psession) {
 		*psession = session;
 	} else if (session) {
-		mutex_unlock(&session->s_mutex);
 		ceph_put_mds_session(session);
 	}
 	/* we flushed them all; remove this inode from the queue */
diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
index f8cac2abab3f..afc7f4c32364 100644
--- a/fs/ceph/snap.c
+++ b/fs/ceph/snap.c
@@ -846,10 +846,8 @@  static void flush_snaps(struct ceph_mds_client *mdsc)
 	}
 	spin_unlock(&mdsc->snap_flush_lock);
 
-	if (session) {
-		mutex_unlock(&session->s_mutex);
+	if (session)
 		ceph_put_mds_session(session);
-	}
 	dout("flush_snaps done\n");
 }