From a8d0158d0df141598098c2c89fb8f0b3ba0c5915 Mon Sep 17 00:00:00 2001 From: "Frank S. Filz" Date: Tue, 6 Sep 2022 11:44:43 -0700 Subject: [PATCH] Client/Inode: wait_for_caps fixups The non-blocking flush requires us to be able to re-add to wait_for_caps but if we simply add to the list, we get stuck in an infinite loop. Add a wait_for_caps_pending list to add to, and then when done signalling, we move the wait_for_caps_pending items onto the wait_for_caps list. Also in handle_cap_flush_ack(), we need to complete the caps flushing before signalling since with non-blocking flush, we will be actually examining the caps from the completion rather than signalling a condition variable in the completion. Signed-off-by: Frank S. Filz --- src/client/Client.cc | 69 ++++++++++++++++++++++++++++++++------------ src/client/Client.h | 1 + src/client/Inode.h | 1 + 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 96c4396b66e..953a6dd5397 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -3586,7 +3586,8 @@ void Client::put_cap_ref(Inode *in, int cap) ldout(cct, 10) << __func__ << " finishing pending cap_snap on " << *in << dendl; in->cap_snaps.rbegin()->second.writing = 0; finish_cap_snap(in, in->cap_snaps.rbegin()->second, get_caps_used(in)); - signal_context_list(in->waitfor_caps); // wake up blocked sync writers + ldout(cct, 10) << __func__ << " calling signal_caps_inode" << dendl; + signal_caps_inode(in); // wake up blocked sync writers } if (last & CEPH_CAP_FILE_BUFFER) { for (auto &p : in->cap_snaps) @@ -4236,6 +4237,24 @@ void Client::signal_context_list(list& ls) } } +void Client::signal_caps_inode(Inode *in) +{ + // Process the waitfor_caps list + while (!in->waitfor_caps.empty()) { + in->waitfor_caps.front()->complete(0); + in->waitfor_caps.pop_front(); + } + + // New items may have been added to the pending list, move them onto the + // waitfor_caps list + while (!in->waitfor_caps_pending.empty()) { + Context *ctx = in->waitfor_caps_pending.front(); + + in->waitfor_caps_pending.pop_front(); + in->waitfor_caps.push_back(ctx); + } +} + void Client::wake_up_session_caps(MetaSession *s, bool reconnect) { for (const auto &cap : s->caps) { @@ -4252,7 +4271,8 @@ void Client::wake_up_session_caps(MetaSession *s, bool reconnect) in.flags |= I_CAP_DROPPED; } } - signal_context_list(in.waitfor_caps); + ldout(cct, 10) << __func__ << " calling signal_caps_inode" << dendl; + signal_caps_inode(&in); } } @@ -4506,8 +4526,10 @@ void Client::add_update_cap(Inode *in, MetaSession *mds_session, uint64_t cap_id } } - if (issued & ~old_caps) - signal_context_list(in->waitfor_caps); + if (issued & ~old_caps) { + ldout(cct, 10) << __func__ << " calling signal_caps_inode" << dendl; + signal_caps_inode(in); + } } void Client::remove_cap(Cap *cap, bool queue_release) @@ -4599,7 +4621,8 @@ void Client::remove_session_caps(MetaSession *s, int err) _schedule_invalidate_callback(in.get(), 0, 0); } - signal_context_list(in->waitfor_caps); + ldout(cct, 10) << __func__ << " calling signal_caps_inode" << dendl; + signal_caps_inode(in.get()); } s->flushing_caps_tids.clear(); sync_cond.notify_all(); @@ -4810,8 +4833,10 @@ void Client::force_session_readonly(MetaSession *s) s->readonly = true; for (xlist::iterator p = s->caps.begin(); !p.end(); ++p) { auto &in = (*p)->inode; - if (in.caps_wanted() & CEPH_CAP_FILE_WR) - signal_context_list(in.waitfor_caps); + if (in.caps_wanted() & CEPH_CAP_FILE_WR) { + ldout(cct, 10) << __func__ << " calling signal_caps_inode" << dendl; + signal_caps_inode(&in); + } } } @@ -5532,13 +5557,6 @@ void Client::handle_cap_flush_ack(MetaSession *session, Inode *in, Cap *cap, con << " cleaned " << ccap_string(cleaned) << " on " << *in << " with " << ccap_string(dirty) << dendl; - if (flushed) { - signal_context_list(in->waitfor_caps); - if (session->flushing_caps_tids.empty() || - *session->flushing_caps_tids.begin() > flush_ack_tid) - sync_cond.notify_all(); - } - if (!dirty) { in->cap_dirtier_uid = -1; in->cap_dirtier_gid = -1; @@ -5557,10 +5575,20 @@ void Client::handle_cap_flush_ack(MetaSession *session, Inode *in, Cap *cap, con if (in->flushing_cap_tids.empty()) in->flushing_cap_item.remove_myself(); } - if (!in->caps_dirty()) - put_inode(in); } } + + if (flushed) { + ldout(cct, 10) << __func__ << " calling signal_caps_inode" << dendl; + signal_caps_inode(in); + if (session->flushing_caps_tids.empty() || + *session->flushing_caps_tids.begin() > flush_ack_tid) + sync_cond.notify_all(); + } + + if (cleaned && !in->caps_dirty()) { + put_inode(in); + } } @@ -5585,7 +5613,8 @@ void Client::handle_cap_flushsnap_ack(MetaSession *session, Inode *in, const MCo in->flushing_cap_item.remove_myself(); in->cap_snaps.erase(it); - signal_context_list(in->waitfor_caps); + ldout(cct, 10) << __func__ << " calling signal_caps_inode" << dendl; + signal_caps_inode(in); if (session->flushing_caps_tids.empty() || *session->flushing_caps_tids.begin() > flush_ack_tid) sync_cond.notify_all(); @@ -5847,8 +5876,10 @@ void Client::handle_cap_grant(MetaSession *session, Inode *in, Cap *cap, const M check_caps(in, flags); // wake up waiters - if (new_caps) - signal_context_list(in->waitfor_caps); + if (new_caps) { + ldout(cct, 10) << __func__ << " calling signal_caps_inode" << dendl; + signal_caps_inode(in); + } // may drop inode's last ref if (deleted_inode) diff --git a/src/client/Client.h b/src/client/Client.h index b950d563962..8371bb5547b 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -1028,6 +1028,7 @@ protected: } void wait_on_context_list(std::list& ls); void signal_context_list(std::list& ls); + void signal_caps_inode(Inode *in); // -- metadata cache stuff diff --git a/src/client/Inode.h b/src/client/Inode.h index aa07e7edc5d..18cadf194ff 100644 --- a/src/client/Inode.h +++ b/src/client/Inode.h @@ -242,6 +242,7 @@ struct Inode : RefCountedObject { std::map> frag_repmap; // non-auth mds mappings std::list waitfor_caps; + std::list waitfor_caps_pending; std::list waitfor_commit; std::list waitfor_deleg; -- 2.39.5