From 5bebb3acf1fa85ab301aa8ca4e99542c9d34d173 Mon Sep 17 00:00:00 2001 From: "Yan, Zheng" Date: Wed, 24 Jun 2015 15:00:50 +0800 Subject: [PATCH] client: use smart pointers in MetaRequest Signed-off-by: Yan, Zheng (cherry picked from commit fd02f0f245ccdc1d33c6d24f57267c8fb102618b) --- src/client/Client.cc | 37 +++++++++--------------- src/client/Client.h | 4 +-- src/client/MetaRequest.cc | 30 -------------------- src/client/MetaRequest.h | 59 ++++++++++++++++++++------------------- 4 files changed, 46 insertions(+), 84 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 9f05856ac76ff..47bc6a9077d45 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -148,7 +148,6 @@ dir_result_t::dir_result_t(Inode *in) : inode(in), offset(0), this_offset(2), next_offset(2), release_count(0), ordered_count(0), start_shared_gen(0), buffer(0) { - inode->get(); } // cons/des @@ -1059,8 +1058,7 @@ void Client::insert_readdir_results(MetaRequest *request, MetaSession *session, dn->offset = dir_result_t::make_fpos(fg, i + readdir_offset); // add to cached result list - in->get(); - request->readdir_result.push_back(pair(dname, in)); + request->readdir_result.push_back(pair(dname, in)); ldout(cct, 15) << __func__ << " " << hex << dn->offset << dec << ": '" << dname << "' -> " << in->ino << dendl; } @@ -1376,7 +1374,7 @@ int Client::verify_reply_trace(int r, *pcreated = got_created_ino; if (request->target) { - (*ptarget) = request->target; + ptarget->swap(request->target); ldout(cct, 20) << "make_request target is " << *ptarget->get() << dendl; } else { if (got_created_ino && (p = inode_map.find(vinodeno_t(created_ino, CEPH_NOSNAP))) != inode_map.end()) { @@ -1581,15 +1579,8 @@ int Client::make_request(MetaRequest *request, void Client::put_request(MetaRequest *request) { - if (request->_put()) { - if (request->inode()) - put_inode(request->take_inode()); - if (request->old_inode()) - put_inode(request->take_old_inode()); - if (request->other_inode()) - put_inode(request->take_other_inode()); + if (request->_put()) delete request; - } } int Client::encode_inode_release(Inode *in, MetaRequest *req, @@ -5816,8 +5807,7 @@ void Client::_closedir(dir_result_t *dirp) ldout(cct, 10) << "_closedir(" << dirp << ")" << dendl; if (dirp->inode) { ldout(cct, 10) << "_closedir detaching inode " << dirp->inode << dendl; - put_inode(dirp->inode); - dirp->inode = 0; + dirp->inode.reset(); } _readdir_drop_dirp_buffer(dirp); delete dirp; @@ -5918,8 +5908,6 @@ void Client::_readdir_drop_dirp_buffer(dir_result_t *dirp) { ldout(cct, 10) << "_readdir_drop_dirp_buffer " << dirp << dendl; if (dirp->buffer) { - for (unsigned i = 0; i < dirp->buffer->size(); i++) - put_inode((*dirp->buffer)[i].second); delete dirp->buffer; dirp->buffer = NULL; } @@ -5941,13 +5929,13 @@ int Client::_readdir_get_frag(dir_result_t *dirp) if (dirp->inode && dirp->inode->snapid == CEPH_SNAPDIR) op = CEPH_MDS_OP_LSSNAP; - Inode *diri = dirp->inode; + InodeRef& diri = dirp->inode; MetaRequest *req = new MetaRequest(op); filepath path; diri->make_nosnap_relative_path(path); req->set_filepath(path); - req->set_inode(diri); + req->set_inode(diri.get()); req->head.args.readdir.frag = fg; if (dirp->last_name.length()) { req->path2.set_path(dirp->last_name.c_str()); @@ -5972,7 +5960,7 @@ int Client::_readdir_get_frag(dir_result_t *dirp) _readdir_drop_dirp_buffer(dirp); - dirp->buffer = new vector >; + dirp->buffer = new vector >; dirp->buffer->swap(req->readdir_result); if (fg != req->readdir_reply_frag) { @@ -6102,7 +6090,7 @@ int Client::readdir_r_cb(dir_result_t *d, add_dirent_cb_t cb, void *p) frag_t fg = dirp->frag(); uint32_t off = dirp->fragpos(); - Inode *diri = dirp->inode; + InodeRef& diri = dirp->inode; if (dirp->at_end()) return 0; @@ -6191,9 +6179,9 @@ int Client::readdir_r_cb(dir_result_t *d, add_dirent_cb_t cb, void *p) dirp->offset = dir_result_t::make_fpos(fg, off); while (off >= dirp->this_offset && off - dirp->this_offset < dirp->buffer->size()) { - pair& ent = (*dirp->buffer)[off - dirp->this_offset]; + pair& ent = (*dirp->buffer)[off - dirp->this_offset]; - int stmask = fill_stat(ent.second, &st); + int stmask = fill_stat(ent.second, &st); fill_dirent(&de, ent.first.c_str(), st.st_mode, st.st_ino, dirp->offset + 1); client_lock.Unlock(); @@ -6578,11 +6566,12 @@ int Client::lookup_parent(Inode *ino, Inode **parent) req->set_filepath(path); req->set_inode(ino); - int r = make_request(req, -1, -1, NULL, NULL, rand() % mdsmap->get_num_in_mds()); + InodeRef target; + int r = make_request(req, -1, -1, &target, NULL, rand() % mdsmap->get_num_in_mds()); // Give caller a reference to the parent ino if they provided a pointer. if (parent != NULL) { if (r == 0) { - *parent = req->target; + *parent = target.get(); _ll_get(*parent); ldout(cct, 3) << "lookup_parent found parent " << (*parent)->ino << dendl; } else { diff --git a/src/client/Client.h b/src/client/Client.h index 140ef76d27ad6..2a7e75f93706d 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -169,7 +169,7 @@ struct dir_result_t { } - Inode *inode; + InodeRef inode; int64_t offset; // high bits: frag_t, low bits: an offset @@ -182,7 +182,7 @@ struct dir_result_t { int start_shared_gen; // dir shared_gen at start of readdir frag_t buffer_frag; - vector > *buffer; + vector > *buffer; string at_cache_name; // last entry we successfully returned diff --git a/src/client/MetaRequest.cc b/src/client/MetaRequest.cc index c8c4552d007a3..330edde1c851c 100644 --- a/src/client/MetaRequest.cc +++ b/src/client/MetaRequest.cc @@ -57,9 +57,6 @@ void MetaRequest::dump(Formatter *f) const MetaRequest::~MetaRequest() { - assert(!_inode); - assert(!_old_inode); - assert(!_other_inode); if (_dentry) _dentry->put(); if (_old_dentry) @@ -68,33 +65,6 @@ MetaRequest::~MetaRequest() reply->put(); } -void MetaRequest::set_inode(Inode *in) { - assert(_inode == NULL); - _inode = in; - _inode->get(); -} -Inode *MetaRequest::inode() { - return _inode; -} - -void MetaRequest::set_old_inode(Inode *in) { - assert(_old_inode == NULL); - _old_inode = in; - _old_inode->get(); -} -Inode *MetaRequest::old_inode() { - return _old_inode; -} - -void MetaRequest::set_other_inode(Inode *in) { - assert(_other_inode == NULL); - _other_inode = in; - _other_inode->get(); -} -Inode *MetaRequest::other_inode() { - return _other_inode; -} - void MetaRequest::set_dentry(Dentry *d) { assert(_dentry == NULL); _dentry = d; diff --git a/src/client/MetaRequest.h b/src/client/MetaRequest.h index e3b6bd167cc0a..aeb68cb43094e 100644 --- a/src/client/MetaRequest.h +++ b/src/client/MetaRequest.h @@ -11,19 +11,18 @@ #include "include/filepath.h" #include "include/atomic.h" #include "mds/mdstypes.h" +#include "InodeRef.h" #include "common/Mutex.h" #include "messages/MClientRequest.h" class MClientReply; -struct Inode; class Dentry; struct MetaRequest { private: - Inode *_inode; - Inode *_old_inode, *_other_inode; + InodeRef _inode, _old_inode, _other_inode; Dentry *_dentry; //associated with path Dentry *_old_dentry; //associated with path2 public: @@ -61,7 +60,7 @@ public: uint64_t readdir_offset; frag_t readdir_reply_frag; - vector > readdir_result; + vector > readdir_result; bool readdir_end; int readdir_num; string readdir_last_name; @@ -76,10 +75,9 @@ public: Cond *caller_cond; // who to take up Cond *dispatch_cond; // who to kick back - Inode *target; + InodeRef target; MetaRequest(int op) : - _inode(NULL), _old_inode(NULL), _other_inode(NULL), _dentry(NULL), _old_dentry(NULL), tid(0), inode_drop(0), inode_unless(0), @@ -95,33 +93,38 @@ public: readdir_offset(0), readdir_end(false), readdir_num(0), got_unsafe(false), item(this), unsafe_item(this), lock("MetaRequest lock"), - caller_cond(0), dispatch_cond(0), - target(0) { + caller_cond(0), dispatch_cond(0) { memset(&head, 0, sizeof(ceph_mds_request_head)); head.op = op; } ~MetaRequest(); - void set_inode(Inode *in); - Inode *inode(); - Inode *take_inode() { - Inode *i = _inode; - _inode = 0; - return i; - } - void set_old_inode(Inode *in); - Inode *old_inode(); - Inode *take_old_inode() { - Inode *i = _old_inode; - _old_inode = NULL; - return i; - } - void set_other_inode(Inode *in); - Inode *other_inode(); - Inode *take_other_inode() { - Inode *i = _other_inode; - _other_inode = 0; - return i; + void set_inode(Inode *in) { + _inode = in; + } + Inode *inode() { + return _inode.get(); + } + void take_inode(InodeRef *out) { + out->swap(_inode); + } + void set_old_inode(Inode *in) { + _old_inode = in; + } + Inode *old_inode() { + return _old_inode.get(); + } + void take_old_inode(InodeRef *out) { + out->swap(_old_inode); + } + void set_other_inode(Inode *in) { + _old_inode = in; + } + Inode *other_inode() { + return _other_inode.get(); + } + void take_other_inode(InodeRef *out) { + out->swap(_other_inode); } void set_dentry(Dentry *d); Dentry *dentry(); -- 2.39.5