Skip to content

Commit

Permalink
ceph: create a new session lock to avoid lock inversion
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Alex Elder authored and Sage Weil committed Feb 2, 2012
1 parent 32852a8 commit d8fb02a
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 9 deletions.
4 changes: 2 additions & 2 deletions fs/ceph/caps.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
4 changes: 2 additions & 2 deletions fs/ceph/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) &&
Expand Down
8 changes: 5 additions & 3 deletions fs/ceph/mds_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down
7 changes: 5 additions & 2 deletions fs/ceph/mds_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit d8fb02a

Please sign in to comment.