ceph: create a new session lock to avoid lock inversion
authorAlex Elder <elder@dreamhost.com>
Fri, 13 Jan 2012 01:48:10 +0000 (17:48 -0800)
committerSage Weil <sage@newdream.net>
Thu, 2 Feb 2012 20:49:19 +0000 (12:49 -0800)
Lockdep was reporting a possible circular lock dependency in
dentry_lease_is_valid().  That function needs to sample the
session's s_cap_gen and and s_cap_ttl fields coherently, but needs
to do so while holding a dentry lock.  The s_cap_lock field was
being used to protect the two fields, but that can't be taken while
holding a lock on a dentry within the session.

In most cases, the s_cap_gen and s_cap_ttl fields only get operated
on separately.  But in three cases they need to be updated together.
Implement a new lock to protect the spots updating both fields
atomically is required.

Signed-off-by: Alex Elder <elder@dreamhost.com>
Reviewed-by: Sage Weil <sage@newdream.net>
fs/ceph/caps.c
fs/ceph/dir.c
fs/ceph/mds_client.c
fs/ceph/mds_client.h

index 8b53193e4f7ca67e3011c8501c8f1fee62357f9d..90d789df9ce0a559e61e1e36465494a149cf41e5 100644 (file)
@@ -641,10 +641,10 @@ static int __cap_is_valid(struct ceph_cap *cap)
        unsigned long ttl;
        u32 gen;
 
-       spin_lock(&cap->session->s_cap_lock);
+       spin_lock(&cap->session->s_gen_ttl_lock);
        gen = cap->session->s_cap_gen;
        ttl = cap->session->s_cap_ttl;
-       spin_unlock(&cap->session->s_cap_lock);
+       spin_unlock(&cap->session->s_gen_ttl_lock);
 
        if (cap->cap_gen < gen || time_after_eq(jiffies, ttl)) {
                dout("__cap_is_valid %p cap %p issued %s "
index 98954003a8d313007386d4cfc214c5dba9296647..63c52f33361bce397cd963af7ef6ef587b4bc06e 100644 (file)
@@ -975,10 +975,10 @@ static int dentry_lease_is_valid(struct dentry *dentry)
        di = ceph_dentry(dentry);
        if (di && di->lease_session) {
                s = di->lease_session;
-               spin_lock(&s->s_cap_lock);
+               spin_lock(&s->s_gen_ttl_lock);
                gen = s->s_cap_gen;
                ttl = s->s_cap_ttl;
-               spin_unlock(&s->s_cap_lock);
+               spin_unlock(&s->s_gen_ttl_lock);
 
                if (di->lease_gen == gen &&
                    time_before(jiffies, dentry->d_time) &&
index be1415fcaac82bd0bcf67f8bd6e8130fe17adb4e..a4fdf9397a90646f5a309a0d6bf1449dc2009493 100644 (file)
@@ -400,9 +400,11 @@ static struct ceph_mds_session *register_session(struct ceph_mds_client *mdsc,
        s->s_con.peer_name.type = CEPH_ENTITY_TYPE_MDS;
        s->s_con.peer_name.num = cpu_to_le64(mds);
 
-       spin_lock_init(&s->s_cap_lock);
+       spin_lock_init(&s->s_gen_ttl_lock);
        s->s_cap_gen = 0;
        s->s_cap_ttl = 0;
+
+       spin_lock_init(&s->s_cap_lock);
        s->s_renew_requested = 0;
        s->s_renew_seq = 0;
        INIT_LIST_HEAD(&s->s_caps);
@@ -2328,10 +2330,10 @@ static void handle_session(struct ceph_mds_session *session,
        case CEPH_SESSION_STALE:
                pr_info("mds%d caps went stale, renewing\n",
                        session->s_mds);
-               spin_lock(&session->s_cap_lock);
+               spin_lock(&session->s_gen_ttl_lock);
                session->s_cap_gen++;
                session->s_cap_ttl = 0;
-               spin_unlock(&session->s_cap_lock);
+               spin_unlock(&session->s_gen_ttl_lock);
                send_renew_caps(mdsc, session);
                break;
 
index a50ca0e39475794018c2350570547bff4f6a7df8..8c7c04ebb595a1a8bd2e9c1b177890f8ba234b9a 100644 (file)
@@ -117,10 +117,13 @@ struct ceph_mds_session {
        void             *s_authorizer_buf, *s_authorizer_reply_buf;
        size_t            s_authorizer_buf_len, s_authorizer_reply_buf_len;
 
-       /* protected by s_cap_lock */
-       spinlock_t        s_cap_lock;
+       /* protected by s_gen_ttl_lock */
+       spinlock_t        s_gen_ttl_lock;
        u32               s_cap_gen;  /* inc each time we get mds stale msg */
        unsigned long     s_cap_ttl;  /* when session caps expire */
+
+       /* protected by s_cap_lock */
+       spinlock_t        s_cap_lock;
        struct list_head  s_caps;     /* all caps issued by this session */
        int               s_nr_caps, s_trim_caps;
        int               s_num_cap_releases;