From 89fc88ef2ed2518e96bd60f4be35c8987453e188 Mon Sep 17 00:00:00 2001 From: John Spray Date: Tue, 29 Nov 2016 17:11:19 +0000 Subject: [PATCH 1/2] Revert "client: trim_caps() do not dereference cap if it's removed" We will simplify this another way. This reverts commit e9fbe39f0f91b7f6551d911f1a2519f5b35175a2. Signed-off-by: John Spray --- src/client/Client.cc | 8 +++----- src/client/Client.h | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index bc246f7adf6..790c5154998 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -3858,7 +3858,7 @@ void Client::add_update_cap(Inode *in, MetaSession *mds_session, uint64_t cap_id signal_cond_list(in->waitfor_caps); } -Cap* Client::remove_cap(Cap *cap, bool queue_release) +void Client::remove_cap(Cap *cap, bool queue_release) { Inode *in = cap->inode; MetaSession *session = cap->session; @@ -3890,7 +3890,6 @@ Cap* Client::remove_cap(Cap *cap, bool queue_release) } else { cap->cap_item.remove_myself(); delete cap; - cap = nullptr; } if (!in->is_any_caps()) { @@ -3899,7 +3898,6 @@ Cap* Client::remove_cap(Cap *cap, bool queue_release) put_snap_realm(in->snaprealm); in->snaprealm = 0; } - return cap; } void Client::remove_all_caps(Inode *in) @@ -3996,7 +3994,7 @@ void Client::trim_caps(MetaSession *s, int max) // disposable non-auth cap if (!(get_caps_used(in) & ~oissued & mine)) { ldout(cct, 20) << " removing unused, unneeded non-auth cap on " << *in << dendl; - cap = remove_cap(cap, true); + remove_cap(cap, true); trimmed++; } } else { @@ -4027,7 +4025,7 @@ void Client::trim_caps(MetaSession *s, int max) } ++p; - if (cap && !cap->inode) { + if (!cap->inode) { cap->cap_item.remove_myself(); delete cap; } diff --git a/src/client/Client.h b/src/client/Client.h index 58c794aa6d5..6baa1bce626 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -621,7 +621,7 @@ protected: void add_update_cap(Inode *in, MetaSession *session, uint64_t cap_id, unsigned issued, unsigned seq, unsigned mseq, inodeno_t realm, int flags, const UserPerm& perms); - Cap* remove_cap(Cap *cap, bool queue_release); + void remove_cap(Cap *cap, bool queue_release); void remove_all_caps(Inode *in); void remove_session_caps(MetaSession *session); void mark_caps_dirty(Inode *in, int caps); From d6a6b03a0dd239a889332fd641cc6a3e99b961c1 Mon Sep 17 00:00:00 2001 From: John Spray Date: Tue, 29 Nov 2016 17:13:29 +0000 Subject: [PATCH 2/2] client: simplify remove_cap interface All this complexity only existed to handle the case where trim wanted to iterate over an xlist without getting disturbed by a deletion. We can simply increment the iterator before maybe-deleting the Cap. Signed-off-by: John Spray --- src/client/Client.cc | 21 +++++++-------------- src/client/MetaSession.h | 4 +--- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 790c5154998..b3808a2b9c8 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -3885,12 +3885,9 @@ void Client::remove_cap(Cap *cap, bool queue_release) assert(in->caps.count(mds)); in->caps.erase(mds); - if (cap == session->s_cap_iterator) { - cap->inode = NULL; - } else { - cap->cap_item.remove_myself(); - delete cap; - } + cap->cap_item.remove_myself(); + delete cap; + cap = nullptr; if (!in->is_any_caps()) { ldout(cct, 15) << "remove_cap last one, closing snaprealm " << in->snaprealm << dendl; @@ -3985,9 +3982,12 @@ void Client::trim_caps(MetaSession *s, int max) xlist::iterator p = s->caps.begin(); while ((caps_size - trimmed) > max && !p.end()) { Cap *cap = *p; - s->s_cap_iterator = cap; Inode *in = cap->inode; + // Increment p early because it will be invalidated if cap + // is deleted inside remove_cap + ++p; + if (in->caps.size() > 1 && cap != in->auth_cap) { int mine = cap->issued | cap->implemented; int oissued = in->auth_cap ? in->auth_cap->issued : 0; @@ -4023,14 +4023,7 @@ void Client::trim_caps(MetaSession *s, int max) trimmed++; } } - - ++p; - if (!cap->inode) { - cap->cap_item.remove_myself(); - delete cap; - } } - s->s_cap_iterator = NULL; if (s->caps.size() > max) _invalidate_kernel_dcache(); diff --git a/src/client/MetaSession.h b/src/client/MetaSession.h index 0d8c1526355..af84615fc1d 100644 --- a/src/client/MetaSession.h +++ b/src/client/MetaSession.h @@ -47,15 +47,13 @@ struct MetaSession { std::set flushing_caps_tids; std::set early_flushing_caps; - Cap *s_cap_iterator; - MClientCapRelease *release; MetaSession() : mds_num(-1), con(NULL), seq(0), cap_gen(0), cap_renew_seq(0), num_caps(0), state(STATE_NEW), mds_state(0), readonly(false), - s_cap_iterator(NULL), release(NULL) + release(NULL) {} ~MetaSession();