From: Sam Lang Date: Tue, 26 Feb 2013 00:51:19 +0000 (-0600) Subject: client: Ensure inode/dentries are ref counted X-Git-Tag: v0.60~58 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=fc80c1dc6ee315ae5e039986602ffadba46cb43b;p=ceph.git client: Ensure inode/dentries are ref counted The MetaRequest holds onto inodes and dentries for retrying unsafe requests, but those objects might be removed from the cache (unlink for example) causing the inode/dentry to be freed. Ensure that the inode/dentry is never freed while the MetaRequest holds onto it by putting/getting the refs using set/get interfaces. Signed-off-by: Sam Lang Reviewed-by: Greg Farnum --- diff --git a/src/client/Client.cc b/src/client/Client.cc index a716672a0943..08330e15b84c 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -904,12 +904,12 @@ Inode* Client::insert_trace(MetaRequest *request, MetaSession *session) if (p.end()) { ldout(cct, 10) << "insert_trace -- no trace" << dendl; - if (request->dentry && - request->dentry->dir && - (request->dentry->dir->parent_inode->flags & I_COMPLETE)) { - ldout(cct, 10) << " clearing I_COMPLETE on " << *request->dentry->dir->parent_inode << dendl; - request->dentry->dir->parent_inode->flags &= ~I_COMPLETE; - request->dentry->dir->release_count++; + Dentry *d = request->dentry(); + if (d && d->dir && + (d->dir->parent_inode->flags & I_COMPLETE)) { + ldout(cct, 10) << " clearing I_COMPLETE on " << *d->dir->parent_inode << dendl; + d->dir->parent_inode->flags &= ~I_COMPLETE; + d->dir->release_count++; } return NULL; } @@ -955,7 +955,7 @@ Inode* Client::insert_trace(MetaRequest *request, MetaSession *session) Dir *dir = diri->open_dir(); insert_dentry_inode(dir, dname, &dlease, in, request->sent_stamp, session, true, ((request->head.op == CEPH_MDS_OP_RENAME) ? - request->old_dentry : NULL)); + request->old_dentry() : NULL)); } else { if (diri->dir && diri->dir->dentries.count(dname)) { Dentry *dn = diri->dir->dentries[dname]; @@ -1007,6 +1007,7 @@ int Client::choose_target_mds(MetaRequest *req) bool is_hash = false; Inode *in = NULL; + Dentry *de = NULL; Cap *cap = NULL; if (req->resend_mds >= 0) { @@ -1019,8 +1020,9 @@ int Client::choose_target_mds(MetaRequest *req) if (cct->_conf->client_use_random_mds) goto random_mds; - if (req->inode) { - in = req->inode; + in = req->inode(); + de = req->dentry(); + if (in) { ldout(cct, 20) << "choose_target_mds starting with req->inode " << *in << dendl; if (req->path.depth()) { hash = ceph_str_hash(in->dir_layout.dl_dir_hash, @@ -1031,17 +1033,17 @@ int Client::choose_target_mds(MetaRequest *req) << " => " << hash << dendl; is_hash = true; } - } else if (req->dentry) { - if (req->dentry->inode) { - in = req->dentry->inode; + } else if (de) { + if (de->inode) { + in = de->inode; ldout(cct, 20) << "choose_target_mds starting with req->dentry inode " << *in << dendl; } else { - in = req->dentry->dir->parent_inode; + in = de->dir->parent_inode; hash = ceph_str_hash(in->dir_layout.dl_dir_hash, - req->dentry->name.data(), - req->dentry->name.length()); + de->name.data(), + de->name.length()); ldout(cct, 20) << "choose_target_mds dentry dir hash is " << (int)in->dir_layout.dl_dir_hash - << " on " << req->dentry->name + << " on " << de->name << " => " << hash << dendl; is_hash = true; } @@ -1177,23 +1179,26 @@ int Client::verify_reply_trace(int r, // created. for now, do this by name. someday, do this by the // ino... which we know! FIXME. Inode *target = 0; // ptarget may be NULL - if (request->dentry) { + Dentry *d = request->dentry(); + if (d) { // rename is special: we handle old_dentry unlink explicitly in insert_dentry_inode(), so // we need to compensate and do the same here. - if (request->old_dentry) { - unlink(request->old_dentry, false); + Dentry *od = request->old_dentry(); + if (od) { + unlink(od, false); } ldout(cct, 10) << "make_request got traceless reply, looking up #" - << request->dentry->dir->parent_inode->ino << "/" << request->dentry->name + << d->dir->parent_inode->ino << "/" << d->name << " got_ino " << got_created_ino << " ino " << created_ino << dendl; - r = _do_lookup(request->dentry->dir->parent_inode, request->dentry->name, &target); + r = _do_lookup(d->dir->parent_inode, d->name, &target); } else { + Inode *in = request->inode(); ldout(cct, 10) << "make_request got traceless reply, forcing getattr on #" - << request->inode->ino << dendl; - r = _getattr(request->inode, request->regetattr_mask, uid, gid, true); - target = request->inode; + << in->ino << dendl; + r = _getattr(in, request->regetattr_mask, uid, gid, true); + target = in; } if (r >= 0) { if (ptarget) @@ -1427,27 +1432,27 @@ void Client::encode_cap_releases(MetaRequest *req, int mds) { ldout(cct, 20) << "encode_cap_releases enter (req: " << req << ", mds: " << mds << ")" << dendl; - if (req->inode_drop && req->inode) - encode_inode_release(req->inode, req, + if (req->inode_drop && req->inode()) + encode_inode_release(req->inode(), req, mds, req->inode_drop, req->inode_unless); - if (req->old_inode_drop && req->old_inode) - encode_inode_release(req->old_inode, req, + if (req->old_inode_drop && req->old_inode()) + encode_inode_release(req->old_inode(), req, mds, req->old_inode_drop, req->old_inode_unless); - if (req->other_inode_drop && req->other_inode) - encode_inode_release(req->other_inode, req, + if (req->other_inode_drop && req->other_inode()) + encode_inode_release(req->other_inode(), req, mds, req->other_inode_drop, req->other_inode_unless); - if (req->dentry_drop && req->dentry) - encode_dentry_release(req->dentry, req, + if (req->dentry_drop && req->dentry()) + encode_dentry_release(req->dentry(), req, mds, req->dentry_drop, req->dentry_unless); - if (req->old_dentry_drop && req->old_dentry) - encode_dentry_release(req->old_dentry, req, + if (req->old_dentry_drop && req->old_dentry()) + encode_dentry_release(req->old_dentry(), req, mds, req->old_dentry_drop, req->old_dentry_unless); ldout(cct, 25) << "encode_cap_releases exit (req: " @@ -1572,7 +1577,7 @@ void Client::send_request(MetaRequest *request, MetaSession *session) ldout(cct, 10) << "send_request rebuilding request " << request->get_tid() << " for mds." << mds << dendl; MClientRequest *r = build_client_request(request); - if (request->dentry) + if (request->dentry()) r->set_dentry_wanted(); if (request->got_unsafe) r->set_replayed_op(); @@ -1587,8 +1592,9 @@ void Client::send_request(MetaRequest *request, MetaSession *session) } request->mds = mds; - if (request->inode && request->inode->caps.count(mds)) - request->sent_on_mseq = request->inode->caps[mds]->mseq; + Inode *in = request->inode(); + if (in && in->caps.count(mds)) + request->sent_on_mseq = in->caps[mds]->mseq; session->requests.push_back(&request->item); @@ -1604,14 +1610,16 @@ MClientRequest* Client::build_client_request(MetaRequest *request) // if the filepath's haven't been set, set them! if (request->path.empty()) { - if (request->inode) - request->inode->make_nosnap_relative_path(request->path); - else if (request->dentry) { - if (request->dentry->inode) - request->dentry->inode->make_nosnap_relative_path(request->path); - else if (request->dentry->dir) { - request->dentry->dir->parent_inode->make_nosnap_relative_path(request->path); - request->path.push_dentry(request->dentry->name); + Inode *in = request->inode(); + Dentry *de = request->dentry(); + if (in) + in->make_nosnap_relative_path(request->path); + else if (de) { + if (de->inode) + de->inode->make_nosnap_relative_path(request->path); + else if (de->dir) { + de->dir->parent_inode->make_nosnap_relative_path(request->path); + request->path.push_dentry(de->name); } else ldout(cct, 1) << "Warning -- unable to construct a filepath!" << " No path, inode, or appropriately-endowed dentry given!" @@ -1706,11 +1714,12 @@ void Client::handle_client_reply(MClientReply *reply) << " from mds." << request->mds << dendl; request->send_to_auth = true; request->resend_mds = choose_target_mds(request); + Inode *in = request->inode(); if (request->resend_mds >= 0 && request->resend_mds == request->mds && - (request->inode == NULL || - request->inode->caps.count(request->resend_mds) == 0 || - request->sent_on_mseq == request->inode->caps[request->resend_mds]->mseq)) { + (in == NULL || + in->caps.count(request->resend_mds) == 0 || + request->sent_on_mseq == in->caps[request->resend_mds]->mseq)) { // have to return ESTALE } else { request->caller_cond->Signal(); @@ -3867,7 +3876,7 @@ int Client::_do_lookup(Inode *dir, const string& name, Inode **target) dir->make_nosnap_relative_path(path); path.push_dentry(name); req->set_filepath(path); - req->inode = dir; + req->set_inode(dir); req->head.args.getattr.mask = 0; ldout(cct, 10) << "_do_lookup on " << path << dendl; @@ -3990,7 +3999,6 @@ int Client::get_or_create(Inode *dir, const char* name, } // success - (*pdn)->get(); return 0; } @@ -4297,7 +4305,7 @@ int Client::_getattr(Inode *in, int mask, int uid, int gid, bool force) filepath path; in->make_nosnap_relative_path(path); req->set_filepath(path); - req->inode = in; + req->set_inode(in); req->head.args.getattr.mask = mask; int res = make_request(req, uid, gid); @@ -4369,7 +4377,7 @@ int Client::_setattr(Inode *in, struct stat *attr, int mask, int uid, int gid, I filepath path; in->make_nosnap_relative_path(path); req->set_filepath(path); - req->inode = in; + req->set_inode(in); if (mask & CEPH_SETATTR_MODE) { req->head.args.setattr.mode = attr->st_mode; @@ -4861,7 +4869,7 @@ int Client::_readdir_get_frag(dir_result_t *dirp) filepath path; diri->make_nosnap_relative_path(path); req->set_filepath(path); - req->inode = diri; + req->set_inode(diri); req->head.args.readdir.frag = fg; if (dirp->last_name.length()) { req->path2.set_path(dirp->last_name.c_str()); @@ -5477,7 +5485,7 @@ int Client::_open(Inode *in, int flags, mode_t mode, Fh **fhp, int uid, int gid) req->head.args.open.mode = mode; req->head.args.open.pool = -1; req->head.args.open.old_size = in->size; // for O_TRUNC - req->inode = in; + req->set_inode(in); result = make_request(req, uid, gid); } @@ -6170,7 +6178,7 @@ void Client::getcwd(string& dir) MetaRequest *req = new MetaRequest(CEPH_MDS_OP_LOOKUPPARENT); filepath path(in->ino); req->set_filepath(path); - req->inode = in; + req->set_inode(in); int res = make_request(req, -1, -1); if (res < 0) break; @@ -6801,7 +6809,7 @@ int Client::_setxattr(Inode *in, const char *name, const void *value, size_t siz in->make_nosnap_relative_path(path); req->set_filepath(path); req->set_string2(name); - req->inode = in; + req->set_inode(in); req->head.args.setxattr.flags = flags; bufferlist bl; @@ -6846,7 +6854,7 @@ int Client::_removexattr(Inode *in, const char *name, int uid, int gid) in->make_nosnap_relative_path(path); req->set_filepath(path); req->set_filepath2(name); - req->inode = in; + req->set_inode(in); int res = make_request(req, uid, gid); @@ -6912,15 +6920,17 @@ int Client::_mknod(Inode *dir, const char *name, mode_t mode, dev_t rdev, int ui dir->make_nosnap_relative_path(path); path.push_dentry(name); req->set_filepath(path); - req->inode = dir; + req->set_inode(dir); req->head.args.mknod.mode = mode; req->head.args.mknod.rdev = rdev; req->dentry_drop = CEPH_CAP_FILE_SHARED; req->dentry_unless = CEPH_CAP_FILE_EXCL; - int res = get_or_create(dir, name, &req->dentry); + Dentry *de; + int res = get_or_create(dir, name, &de); if (res < 0) goto fail; + req->set_dentry(de); res = make_request(req, uid, gid, inp); @@ -6987,7 +6997,7 @@ int Client::_create(Inode *dir, const char *name, int flags, mode_t mode, Inode dir->make_nosnap_relative_path(path); path.push_dentry(name); req->set_filepath(path); - req->inode = dir; + req->set_inode(dir); req->head.args.open.flags = flags | O_CREAT; req->head.args.open.mode = mode; @@ -6998,9 +7008,14 @@ int Client::_create(Inode *dir, const char *name, int flags, mode_t mode, Inode req->dentry_drop = CEPH_CAP_FILE_SHARED; req->dentry_unless = CEPH_CAP_FILE_EXCL; - int res = get_or_create(dir, name, &req->dentry); + bufferlist extra_bl; + inodeno_t created_ino; + + Dentry *de; + int res = get_or_create(dir, name, &de); if (res < 0) goto fail; + req->set_dentry(de); res = make_request(req, uid, gid, inp, created); if (res < 0) { @@ -7044,14 +7059,16 @@ int Client::_mkdir(Inode *dir, const char *name, mode_t mode, int uid, int gid, dir->make_nosnap_relative_path(path); path.push_dentry(name); req->set_filepath(path); - req->inode = dir; + req->set_inode(dir); req->head.args.mkdir.mode = mode; req->dentry_drop = CEPH_CAP_FILE_SHARED; req->dentry_unless = CEPH_CAP_FILE_EXCL; - int res = get_or_create(dir, name, &req->dentry); + Dentry *de; + int res = get_or_create(dir, name, &de); if (res < 0) goto fail; + req->set_dentry(de); ldout(cct, 10) << "_mkdir: making request" << dendl; res = make_request(req, uid, gid, inp); @@ -7109,14 +7126,16 @@ int Client::_symlink(Inode *dir, const char *name, const char *target, int uid, dir->make_nosnap_relative_path(path); path.push_dentry(name); req->set_filepath(path); - req->inode = dir; + req->set_inode(dir); req->set_string2(target); req->dentry_drop = CEPH_CAP_FILE_SHARED; req->dentry_unless = CEPH_CAP_FILE_EXCL; - int res = get_or_create(dir, name, &req->dentry); + Dentry *de; + int res = get_or_create(dir, name, &de); if (res < 0) goto fail; + req->set_dentry(de); res = make_request(req, uid, gid, inp); @@ -7167,16 +7186,20 @@ int Client::_unlink(Inode *dir, const char *name, int uid, int gid) path.push_dentry(name); req->set_filepath(path); - int res = get_or_create(dir, name, &req->dentry); + Dentry *de; + int res = get_or_create(dir, name, &de); if (res < 0) goto fail; + req->set_dentry(de); req->dentry_drop = CEPH_CAP_FILE_SHARED; req->dentry_unless = CEPH_CAP_FILE_EXCL; - res = _lookup(dir, name, &req->other_inode); + Inode *otherin; + res = _lookup(dir, name, &otherin); + req->set_other_inode(otherin); req->other_inode_drop = CEPH_CAP_LINK_SHARED | CEPH_CAP_LINK_EXCL; - req->inode = dir; + req->set_inode(dir); res = make_request(req, uid, gid); if (res == 0) { @@ -7225,12 +7248,18 @@ int Client::_rmdir(Inode *dir, const char *name, int uid, int gid) req->dentry_drop = CEPH_CAP_FILE_SHARED; req->dentry_unless = CEPH_CAP_FILE_EXCL; req->inode_drop = CEPH_CAP_LINK_SHARED | CEPH_CAP_LINK_EXCL; - req->inode = dir; + req->set_inode(dir); - int res = get_or_create(dir, name, &req->dentry); + Dentry *de; + int res = get_or_create(dir, name, &de); if (res < 0) goto fail; - res = _lookup(dir, name, &req->inode); + req->set_dentry(de); + Inode *in; + res = _lookup(dir, name, &in); + if (res < 0) + goto fail; + req->set_inode(in); res = make_request(req, uid, gid); if (res == 0) { @@ -7285,27 +7314,37 @@ int Client::_rename(Inode *fromdir, const char *fromname, Inode *todir, const ch req->set_filepath(to); req->set_filepath2(from); - int res = get_or_create(fromdir, fromname, &req->old_dentry); + Dentry *oldde; + int res = get_or_create(fromdir, fromname, &oldde); if (res < 0) goto fail; + req->set_old_dentry(oldde); req->old_dentry_drop = CEPH_CAP_FILE_SHARED; req->old_dentry_unless = CEPH_CAP_FILE_EXCL; - res = get_or_create(todir, toname, &req->dentry); + Dentry *de; + res = get_or_create(todir, toname, &de); if (res < 0) goto fail; + req->set_dentry(de); req->dentry_drop = CEPH_CAP_FILE_SHARED; req->dentry_unless = CEPH_CAP_FILE_EXCL; - res = _lookup(fromdir, fromname, &req->old_inode); + Inode *oldin; + res = _lookup(fromdir, fromname, &oldin); if (res < 0) goto fail; + req->set_old_inode(oldin); req->old_inode_drop = CEPH_CAP_LINK_SHARED; - res = _lookup(todir, toname, &req->other_inode); + Inode *otherin; + res = _lookup(todir, toname, &otherin); + if (res < 0) + goto fail; + req->set_other_inode(otherin); req->other_inode_drop = CEPH_CAP_LINK_SHARED | CEPH_CAP_LINK_EXCL; - req->inode = todir; + req->set_inode(todir); Inode *target; res = make_request(req, uid, gid, &target); @@ -7358,13 +7397,15 @@ int Client::_link(Inode *in, Inode *dir, const char *newname, int uid, int gid, filepath existing(in->ino); req->set_filepath2(existing); - req->inode = dir; + req->set_inode(dir); req->inode_drop = CEPH_CAP_FILE_SHARED; req->inode_unless = CEPH_CAP_FILE_EXCL; - int res = get_or_create(dir, newname, &req->dentry); + Dentry *de; + int res = get_or_create(dir, newname, &de); if (res < 0) goto fail; + req->set_dentry(de); res = make_request(req, uid, gid, inp); ldout(cct, 10) << "link result is " << res << dendl; diff --git a/src/client/MetaRequest.cc b/src/client/MetaRequest.cc index 2c50f065576a..199708c5de9c 100644 --- a/src/client/MetaRequest.cc +++ b/src/client/MetaRequest.cc @@ -14,18 +14,18 @@ void MetaRequest::dump(Formatter *f) const f->dump_string("op", ceph_mds_op_name(head.op)); f->dump_stream("path") << path; f->dump_stream("path2") << path2; - if (inode) - f->dump_stream("ino") << inode->ino; - if (old_inode) - f->dump_stream("old_ino") << old_inode->ino; - if (other_inode) - f->dump_stream("other_ino") << other_inode->ino; + if (_inode) + f->dump_stream("ino") << _inode->ino; + if (_old_inode) + f->dump_stream("old_ino") << _old_inode->ino; + if (_other_inode) + f->dump_stream("other_ino") << _other_inode->ino; if (target) f->dump_stream("target_ino") << target->ino; - if (dentry) - f->dump_string("dentry", dentry->name); - if (old_dentry) - f->dump_string("old_dentry", old_dentry->name); + if (_dentry) + f->dump_string("dentry", _dentry->name); + if (_old_dentry) + f->dump_string("old_dentry", _old_dentry->name); f->dump_stream("hint_ino") << inodeno_t(head.ino); f->dump_stream("sent_stamp") << sent_stamp; @@ -58,11 +58,61 @@ void MetaRequest::dump(Formatter *f) const MetaRequest::~MetaRequest() { - if (dentry) - dentry->put(); - if (old_dentry) - old_dentry->put(); + if (_inode) + _inode->put(); + if (_old_inode) + _old_inode->put(); + if (_other_inode) + _other_inode->put(); + if (_dentry) + _dentry->put(); + if (_old_dentry) + _old_dentry->put(); if (reply) 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; + _dentry->get(); +} +Dentry *MetaRequest::dentry() { + return _dentry; +} + +void MetaRequest::set_old_dentry(Dentry *d) { + assert(_old_dentry == NULL); + _old_dentry = d; + _old_dentry->get(); +} +Dentry *MetaRequest::old_dentry() { + return _old_dentry; +} diff --git a/src/client/MetaRequest.h b/src/client/MetaRequest.h index 03008c8bbb94..c167ec98f5b1 100644 --- a/src/client/MetaRequest.h +++ b/src/client/MetaRequest.h @@ -20,6 +20,12 @@ class Inode; class Dentry; struct MetaRequest { +private: + Inode *_inode; + Inode *_old_inode, *_other_inode; + Dentry *_dentry; //associated with path + Dentry *_old_dentry; //associated with path2 +public: uint64_t tid; ceph_mds_request_head head; filepath path, path2; @@ -31,10 +37,6 @@ struct MetaRequest { int old_dentry_drop, old_dentry_unless; int other_inode_drop, other_inode_unless; vector cap_releases; - Inode *inode; - Inode *old_inode, *other_inode; - Dentry *dentry; //associated with path - Dentry *old_dentry; //associated with path2 int regetattr_mask; // getattr mask if i need to re-stat after a traceless reply @@ -74,14 +76,14 @@ struct MetaRequest { Inode *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), old_inode_drop(0), old_inode_unless(0), dentry_drop(0), dentry_unless(0), old_dentry_drop(0), old_dentry_unless(0), other_inode_drop(0), other_inode_unless(0), - inode(NULL), old_inode(NULL), other_inode(NULL), - dentry(NULL), old_dentry(NULL), regetattr_mask(0), mds(-1), resend_mds(-1), send_to_auth(false), sent_on_mseq(0), num_fwd(0), retry_attempt(0), @@ -97,6 +99,17 @@ struct MetaRequest { } ~MetaRequest(); + void set_inode(Inode *in); + Inode *inode(); + void set_old_inode(Inode *in); + Inode *old_inode(); + void set_other_inode(Inode *in); + Inode *other_inode(); + void set_dentry(Dentry *d); + Dentry *dentry(); + void set_old_dentry(Dentry *d); + Dentry *old_dentry(); + MetaRequest* get() { ++ref; return this; @@ -150,6 +163,7 @@ struct MetaRequest { } void dump(Formatter *f) const; + }; #endif