Commit d8fb02abdc39f92a1066313e2b17047876afa8f9
Committed by
Sage Weil
1 parent
32852a81bc
Exists in
smarc-l5.0.0_1.0.0-ga
and in
5 other branches
ceph: create a new session lock to avoid lock inversion
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>
Showing 4 changed files with 14 additions and 9 deletions Side-by-side Diff
fs/ceph/caps.c
... | ... | @@ -641,10 +641,10 @@ |
641 | 641 | unsigned long ttl; |
642 | 642 | u32 gen; |
643 | 643 | |
644 | - spin_lock(&cap->session->s_cap_lock); | |
644 | + spin_lock(&cap->session->s_gen_ttl_lock); | |
645 | 645 | gen = cap->session->s_cap_gen; |
646 | 646 | ttl = cap->session->s_cap_ttl; |
647 | - spin_unlock(&cap->session->s_cap_lock); | |
647 | + spin_unlock(&cap->session->s_gen_ttl_lock); | |
648 | 648 | |
649 | 649 | if (cap->cap_gen < gen || time_after_eq(jiffies, ttl)) { |
650 | 650 | dout("__cap_is_valid %p cap %p issued %s " |
fs/ceph/dir.c
... | ... | @@ -975,10 +975,10 @@ |
975 | 975 | di = ceph_dentry(dentry); |
976 | 976 | if (di && di->lease_session) { |
977 | 977 | s = di->lease_session; |
978 | - spin_lock(&s->s_cap_lock); | |
978 | + spin_lock(&s->s_gen_ttl_lock); | |
979 | 979 | gen = s->s_cap_gen; |
980 | 980 | ttl = s->s_cap_ttl; |
981 | - spin_unlock(&s->s_cap_lock); | |
981 | + spin_unlock(&s->s_gen_ttl_lock); | |
982 | 982 | |
983 | 983 | if (di->lease_gen == gen && |
984 | 984 | time_before(jiffies, dentry->d_time) && |
fs/ceph/mds_client.c
... | ... | @@ -400,9 +400,11 @@ |
400 | 400 | s->s_con.peer_name.type = CEPH_ENTITY_TYPE_MDS; |
401 | 401 | s->s_con.peer_name.num = cpu_to_le64(mds); |
402 | 402 | |
403 | - spin_lock_init(&s->s_cap_lock); | |
403 | + spin_lock_init(&s->s_gen_ttl_lock); | |
404 | 404 | s->s_cap_gen = 0; |
405 | 405 | s->s_cap_ttl = 0; |
406 | + | |
407 | + spin_lock_init(&s->s_cap_lock); | |
406 | 408 | s->s_renew_requested = 0; |
407 | 409 | s->s_renew_seq = 0; |
408 | 410 | INIT_LIST_HEAD(&s->s_caps); |
409 | 411 | |
... | ... | @@ -2328,10 +2330,10 @@ |
2328 | 2330 | case CEPH_SESSION_STALE: |
2329 | 2331 | pr_info("mds%d caps went stale, renewing\n", |
2330 | 2332 | session->s_mds); |
2331 | - spin_lock(&session->s_cap_lock); | |
2333 | + spin_lock(&session->s_gen_ttl_lock); | |
2332 | 2334 | session->s_cap_gen++; |
2333 | 2335 | session->s_cap_ttl = 0; |
2334 | - spin_unlock(&session->s_cap_lock); | |
2336 | + spin_unlock(&session->s_gen_ttl_lock); | |
2335 | 2337 | send_renew_caps(mdsc, session); |
2336 | 2338 | break; |
2337 | 2339 |
fs/ceph/mds_client.h
... | ... | @@ -117,10 +117,13 @@ |
117 | 117 | void *s_authorizer_buf, *s_authorizer_reply_buf; |
118 | 118 | size_t s_authorizer_buf_len, s_authorizer_reply_buf_len; |
119 | 119 | |
120 | - /* protected by s_cap_lock */ | |
121 | - spinlock_t s_cap_lock; | |
120 | + /* protected by s_gen_ttl_lock */ | |
121 | + spinlock_t s_gen_ttl_lock; | |
122 | 122 | u32 s_cap_gen; /* inc each time we get mds stale msg */ |
123 | 123 | unsigned long s_cap_ttl; /* when session caps expire */ |
124 | + | |
125 | + /* protected by s_cap_lock */ | |
126 | + spinlock_t s_cap_lock; | |
124 | 127 | struct list_head s_caps; /* all caps issued by this session */ |
125 | 128 | int s_nr_caps, s_trim_caps; |
126 | 129 | int s_num_cap_releases; |