ceph: don't take s_mutex or snap_rwsem in ceph_check_caps

These locks appear to be completely unnecessary. Almost all of this
function is done under the inode->i_ceph_lock, aside from the actual
sending of the message. Don't take either lock in this function.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Luis Henriques <lhenriques@suse.de>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 919eada..95f56b3 100644 (file)
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -1912,7 +1912,6 @@ void ceph_check_caps(struct ceph_inode_info *ci, int flags,
        struct ceph_cap *cap;
        u64 flush_tid, oldest_flush_tid;
        int file_wanted, used, cap_used;
-       int took_snap_rwsem = 0;             /* true if mdsc->snap_rwsem held */
        int issued, implemented, want, retain, revoking, flushing = 0;
        int mds = -1;   /* keep track of how far we've gone through i_caps list
                           to avoid an infinite loop on retry */
@@ -1920,14 +1919,13 @@ void ceph_check_caps(struct ceph_inode_info *ci, int flags,
        bool queue_invalidate = false;
        bool tried_invalidate = false;
 
+       if (session)
+               ceph_get_mds_session(session);
+
        spin_lock(&ci->i_ceph_lock);
        if (ci->i_ceph_flags & CEPH_I_FLUSH)
                flags |= CHECK_CAPS_FLUSH;
-
-       goto retry_locked;
 retry:
-       spin_lock(&ci->i_ceph_lock);
-retry_locked:
        /* Caps wanted by virtue of active open files. */
        file_wanted = __ceph_caps_file_wanted(ci);
 
@@ -2007,7 +2005,7 @@ retry_locked:
                        ci->i_rdcache_revoking = ci->i_rdcache_gen;
                }
                tried_invalidate = true;
-               goto retry_locked;
+               goto retry;
        }
 
        for (p = rb_first(&ci->i_caps); p; p = rb_next(p)) {
@@ -2021,8 +2019,6 @@ retry_locked:
                    ((flags & CHECK_CAPS_AUTHONLY) && cap != ci->i_auth_cap))
                        continue;
 
-               /* NOTE: no side-effects allowed, until we take s_mutex */
-
                /*
                 * If we have an auth cap, we don't need to consider any
                 * overlapping caps as used.
@@ -2085,37 +2081,8 @@ retry_locked:
                        continue;     /* nope, all good */
 
 ack:
-               if (session && session != cap->session) {
-                       dout("oops, wrong session %p mutex\n", session);
-                       mutex_unlock(&session->s_mutex);
-                       session = NULL;
-               }
-               if (!session) {
-                       session = cap->session;
-                       if (mutex_trylock(&session->s_mutex) == 0) {
-                               dout("inverting session/ino locks on %p\n",
-                                    session);
-                               session = ceph_get_mds_session(session);
-                               spin_unlock(&ci->i_ceph_lock);
-                               if (took_snap_rwsem) {
-                                       up_read(&mdsc->snap_rwsem);
-                                       took_snap_rwsem = 0;
-                               }
-                               if (session) {
-                                       mutex_lock(&session->s_mutex);
-                                       ceph_put_mds_session(session);
-                               } else {
-                                       /*
-                                        * Because we take the reference while
-                                        * holding the i_ceph_lock, it should
-                                        * never be NULL. Throw a warning if it
-                                        * ever is.
-                                        */
-                                       WARN_ON_ONCE(true);
-                               }
-                               goto retry;
-                       }
-               }
+               ceph_put_mds_session(session);
+               session = ceph_get_mds_session(cap->session);
 
                /* kick flushing and flush snaps before sending normal
                 * cap message */
@@ -2127,20 +2094,7 @@ ack:
                        if (ci->i_ceph_flags & CEPH_I_FLUSH_SNAPS)
                                __ceph_flush_snaps(ci, session);
 
-                       goto retry_locked;
-               }
-
-               /* take snap_rwsem after session mutex */
-               if (!took_snap_rwsem) {
-                       if (down_read_trylock(&mdsc->snap_rwsem) == 0) {
-                               dout("inverting snap/in locks on %p\n",
-                                    inode);
-                               spin_unlock(&ci->i_ceph_lock);
-                               down_read(&mdsc->snap_rwsem);
-                               took_snap_rwsem = 1;
-                               goto retry;
-                       }
-                       took_snap_rwsem = 1;
+                       goto retry;
                }
 
                if (cap == ci->i_auth_cap && ci->i_dirty_caps) {
@@ -2162,9 +2116,10 @@ ack:
 
                __prep_cap(&arg, cap, CEPH_CAP_OP_UPDATE, mflags, cap_used,
                           want, retain, flushing, flush_tid, oldest_flush_tid);
-               spin_unlock(&ci->i_ceph_lock);
 
+               spin_unlock(&ci->i_ceph_lock);
                __send_cap(&arg, ci);
+               spin_lock(&ci->i_ceph_lock);
 
                goto retry; /* retake i_ceph_lock and restart our cap scan. */
        }
@@ -2179,13 +2134,9 @@ ack:
 
        spin_unlock(&ci->i_ceph_lock);
 
+       ceph_put_mds_session(session);
        if (queue_invalidate)
                ceph_queue_invalidate(inode);
-
-       if (session)
-               mutex_unlock(&session->s_mutex);
-       if (took_snap_rwsem)
-               up_read(&mdsc->snap_rwsem);
 }
 
 /*
@@ -3550,13 +3501,12 @@ static void handle_cap_grant(struct inode *inode,
        if (wake)
                wake_up_all(&ci->i_cap_wq);
 
+       mutex_unlock(&session->s_mutex);
        if (check_caps == 1)
                ceph_check_caps(ci, CHECK_CAPS_AUTHONLY | CHECK_CAPS_NOINVAL,
                                session);
        else if (check_caps == 2)
                ceph_check_caps(ci, CHECK_CAPS_NOINVAL, session);
-       else
-               mutex_unlock(&session->s_mutex);
 }
 
 /*