From: huanghua Date: Thu, 13 Jul 2006 15:15:38 +0000 (+0000) Subject: a few fixes after testing & inspection: X-Git-Tag: v1_8_0_110~486^2~1430 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=d59a415d85dff1961efea1aa83600a02ed9a78c1;p=fs%2Flustre-release.git a few fixes after testing & inspection: (1) more accurate & useful debug message; (2) more clean code; (3) fix mdt_reint_link: src & target dir. --- diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index 2e3a343..5996d90 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -214,6 +214,7 @@ static int mdt_getattr_internal(struct mdt_thread_info *info, if (rc == -EREMOTE) { /* FIXME: This object is located on remote node. * What value should we return to client? + * also in mdt_md_create() and mdt_object_open() */ if (need_pack_reply) { rc = req_capsule_pack(&info->mti_pill); @@ -397,6 +398,10 @@ static int mdt_getattr_name_lock(struct mdt_thread_info *info, /* only open the child. parent is on another node. */ intent_set_disposition(ldlm_rep, DISP_LOOKUP_POS); child = parent; + CDEBUG(D_INFO, "partial getattr_name child_fid = "DFID3 + ", ldlm_rep=%p\n", + PFID3(mdt_object_fid(child)), ldlm_rep); + mdt_lock_handle_init(lhc); lhc->mlh_mode = LCK_CR; result = mdt_object_lock(info, child, lhc, child_bits); @@ -407,9 +412,11 @@ static int mdt_getattr_name_lock(struct mdt_thread_info *info, if (result != 0) mdt_object_unlock(info, child, lhc); } - RETURN(result); + GOTO(out, result); } + CDEBUG(D_INFO, DFID3"/%s, ldlm_rep = %p\n", + PFID3(mdt_object_fid(parent)), name, ldlm_rep); /*step 1: lock parent */ lhp = &info->mti_lh[MDT_LH_PARENT]; lhp->mlh_mode = LCK_CR; @@ -444,6 +451,7 @@ static int mdt_getattr_name_lock(struct mdt_thread_info *info, EXIT; out_parent: mdt_object_unlock(info, parent, lhp); +out: return result; } @@ -728,7 +736,6 @@ int fid_lock(struct ldlm_namespace *ns, const struct lu_fid *f, { int flags = 0; int rc; - ENTRY; LASSERT(ns != NULL); LASSERT(lh != NULL); @@ -739,14 +746,13 @@ int fid_lock(struct ldlm_namespace *ns, const struct lu_fid *f, LDLM_IBITS, policy, mode, &flags, ldlm_blocking_ast, ldlm_completion_ast, NULL, NULL, NULL, 0, NULL, lh); - RETURN(rc == ELDLM_OK ? 0 : -EIO); + return rc == ELDLM_OK ? 0 : -EIO; } void fid_unlock(struct ldlm_namespace *ns, const struct lu_fid *f, struct lustre_handle *lh, ldlm_mode_t mode) { struct ldlm_lock *lock; - ENTRY; /* FIXME: this is debug stuff, remove it later. */ lock = ldlm_handle2lock(lh); @@ -758,7 +764,6 @@ void fid_unlock(struct ldlm_namespace *ns, const struct lu_fid *f, LASSERT(fid_res_name_eq(f, &lock->l_resource->lr_name)); ldlm_lock_decref(lh, mode); - EXIT; } static struct mdt_object *mdt_obj(struct lu_object *o) @@ -772,12 +777,15 @@ struct mdt_object *mdt_object_find(const struct lu_context *ctxt, const struct lu_fid *f) { struct lu_object *o; + struct mdt_object *m; + ENTRY; o = lu_object_find(ctxt, d->mdt_md_dev.md_lu_dev.ld_site, f); if (IS_ERR(o)) - return (struct mdt_object *)o; + m = (struct mdt_object *)o; else - return mdt_obj(o); + m = mdt_obj(o); + RETURN(m); } int mdt_object_lock(struct mdt_thread_info *info, struct mdt_object *o, @@ -786,24 +794,29 @@ int mdt_object_lock(struct mdt_thread_info *info, struct mdt_object *o, ldlm_policy_data_t *policy = &info->mti_policy; struct ldlm_res_id *res_id = &info->mti_res_id; struct ldlm_namespace *ns = info->mti_mdt->mdt_namespace; + int rc; + ENTRY; LASSERT(!lustre_handle_is_used(&lh->mlh_lh)); LASSERT(lh->mlh_mode != LCK_MINMODE); policy->l_inodebits.bits = ibits; - return fid_lock(ns, mdt_object_fid(o), &lh->mlh_lh, lh->mlh_mode, policy, res_id); + rc = fid_lock(ns, mdt_object_fid(o), &lh->mlh_lh, lh->mlh_mode, policy, res_id); + RETURN(rc); } void mdt_object_unlock(struct mdt_thread_info *info, struct mdt_object *o, struct mdt_lock_handle *lh) { struct ldlm_namespace *ns = info->mti_mdt->mdt_namespace; + ENTRY; if (lustre_handle_is_used(&lh->mlh_lh)) { fid_unlock(ns, mdt_object_fid(o), &lh->mlh_lh, lh->mlh_mode); lh->mlh_lh.cookie = 0; } + EXIT; } struct mdt_object *mdt_object_find_lock(struct mdt_thread_info *info, @@ -948,7 +961,6 @@ int mdt_update_last_transno(struct mdt_thread_info *info, int rc) LASSERT(mdt != NULL); - last_transno = req->rq_reqmsg->transno; if (rc != 0) { if (last_transno != 0) { @@ -1184,6 +1196,8 @@ static int mds_msg_check_version(struct lustre_msg *msg) static int mdt_filter_recovery_request(struct ptlrpc_request *req, struct obd_device *obd, int *process) { + ENTRY; + switch (req->rq_reqmsg->opc) { case MDS_CONNECT: /* This will never get here, but for completeness. */ case OST_CONNECT: /* This will never get here, but for completeness. */ @@ -1274,6 +1288,7 @@ static int mdt_reply(struct ptlrpc_request *req, int result, struct mdt_thread_info *info) { struct obd_device *obd; + ENTRY; if (lustre_msg_get_flags(req->rq_reqmsg) & MSG_LAST_REPLAY) { if (req->rq_reqmsg->opc != OBD_PING) @@ -2289,14 +2304,19 @@ static int mdt_object_init(const struct lu_context *ctxt, struct lu_object *o) struct mdt_device *d = mdt_dev(o->lo_dev); struct lu_device *under; struct lu_object *below; + int rc = 0; + ENTRY; + + CDEBUG(D_INFO, "object init, fid = "DFID3"\n", + PFID3(&o->lo_header->loh_fid)); under = &d->mdt_child->md_lu_dev; below = under->ld_ops->ldo_object_alloc(ctxt, o->lo_header, under); if (below != NULL) { lu_object_add(o, below); - return 0; } else - return -ENOMEM; + rc = -ENOMEM; + RETURN(rc); } static void mdt_object_free(const struct lu_context *ctxt, struct lu_object *o) @@ -2306,6 +2326,8 @@ static void mdt_object_free(const struct lu_context *ctxt, struct lu_object *o) ENTRY; h = o->lo_header; + CDEBUG(D_INFO, "object free, fid = "DFID3"\n", PFID3(&h->loh_fid)); + lu_object_fini(o); lu_object_header_fini(h); OBD_FREE_PTR(mo); diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c index 895f7ed..a9c700b 100644 --- a/lustre/mdt/mdt_open.c +++ b/lustre/mdt/mdt_open.c @@ -52,20 +52,19 @@ static void mdt_mfd_get(void *mfdp) static struct mdt_file_data *mdt_mfd_new(void) { struct mdt_file_data *mfd; + ENTRY; OBD_ALLOC_PTR(mfd); - if (mfd == NULL) { - CERROR("mds: out of memory\n"); - return NULL; - } + if (mfd != NULL) { + atomic_set(&mfd->mfd_refcount, 1); - atomic_set(&mfd->mfd_refcount, 1); - - INIT_LIST_HEAD(&mfd->mfd_handle.h_link); - INIT_LIST_HEAD(&mfd->mfd_list); - class_handle_hash(&mfd->mfd_handle, mdt_mfd_get); + INIT_LIST_HEAD(&mfd->mfd_handle.h_link); + INIT_LIST_HEAD(&mfd->mfd_list); + class_handle_hash(&mfd->mfd_handle, mdt_mfd_get); + } else + CERROR("mdt: out of memory\n"); - return mfd; + RETURN(mfd); } /* Get a new reference on the mfd pointed to by handle, if handle is still @@ -110,7 +109,8 @@ static int mdt_object_open(struct mdt_thread_info *info, if (rc == -EREMOTE) { repbody->fid1 = *mdt_object_fid(o); repbody->valid |= OBD_MD_FLID; - RETURN(-EREMOTE); + /*FIXME: should be return 0 or -EREMOTE? */ + /* also in mdt_reint:mdt_md_create() */ } if (rc != 0) RETURN(rc); @@ -129,31 +129,31 @@ static int mdt_object_open(struct mdt_thread_info *info, repbody->eadatasize = rc; rc = 0; */ - mfd = mdt_mfd_new(); - if (mfd == NULL) { - CERROR("mds: out of memory\n"); - RETURN(-ENOMEM); - } - if (flags & FMODE_WRITE) { /*mds_get_write_access*/ } else if (flags & MDS_FMODE_EXEC) { /*mds_deny_write_access*/ } - /* keep a reference on this object for this open, - * and is released by mdt_mfd_close() */ - mdt_object_get(info->mti_ctxt, o); + mfd = mdt_mfd_new(); + if (mfd != NULL) { + /* keep a reference on this object for this open, + * and is released by mdt_mfd_close() */ + mdt_object_get(info->mti_ctxt, o); - mfd->mfd_mode = flags; - mfd->mfd_object = o; - mfd->mfd_xid = mdt_info_req(info)->rq_xid; + mfd->mfd_mode = flags; + mfd->mfd_object = o; + mfd->mfd_xid = mdt_info_req(info)->rq_xid; - spin_lock(&med->med_open_lock); - list_add(&mfd->mfd_list, &med->med_open_head); - spin_unlock(&med->med_open_lock); + spin_lock(&med->med_open_lock); + list_add(&mfd->mfd_list, &med->med_open_head); + spin_unlock(&med->med_open_lock); - repbody->handle.cookie = mfd->mfd_handle.h_cookie; + repbody->handle.cookie = mfd->mfd_handle.h_cookie; + } else { + CERROR("mdt: out of memory\n"); + rc = -ENOMEM; + } RETURN(rc); } @@ -165,19 +165,18 @@ int mdt_open_by_fid(struct mdt_thread_info* info, const struct lu_fid *fid, int rc; ENTRY; + intent_set_disposition(rep, DISP_LOOKUP_EXECD); o = mdt_object_find(info->mti_ctxt, info->mti_mdt, fid); if (!IS_ERR(o)) { if (mdt_object_exists(info->mti_ctxt, &o->mot_obj.mo_lu)) { - intent_set_disposition(rep, DISP_LOOKUP_EXECD); intent_set_disposition(rep, DISP_LOOKUP_POS); rc = mdt_object_open(info, o, flags); intent_set_disposition(rep, DISP_OPEN_OPEN); - mdt_object_put(info->mti_ctxt, o); } else { - intent_set_disposition(rep, DISP_LOOKUP_EXECD); intent_set_disposition(rep, DISP_LOOKUP_NEG); rc = -ENOENT; } + mdt_object_put(info->mti_ctxt, o); } else rc = PTR_ERR(o); @@ -203,6 +202,7 @@ int mdt_lock_new_child(struct mdt_thread_info *info, { struct mdt_lock_handle lockh; int rc; + ENTRY; if (child_lockh == NULL) child_lockh = &lockh; @@ -304,12 +304,14 @@ int mdt_reint_open(struct mdt_thread_info *info) GOTO(destroy_child, result); destroy_child: - if (created && result != 0 && result != -EREMOTE) { - mdo_unlink(info->mti_ctxt, mdt_object_child(parent), - mdt_object_child(child), rr->rr_name); - } else if (created) { - /* barrier with other thread */ - mdt_lock_new_child(info, child, NULL); + if (created) { + if (result != 0 && result != -EREMOTE) { + mdo_unlink(info->mti_ctxt, mdt_object_child(parent), + mdt_object_child(child), rr->rr_name); + } else { + /* barrier with other thread */ + mdt_lock_new_child(info, child, NULL); + } } out_child: mdt_object_put(info->mti_ctxt, child); @@ -348,28 +350,29 @@ int mdt_close(struct mdt_thread_info *info) ENTRY; med = &mdt_info_req(info)->rq_export->exp_mdt_data; - LASSERT(med); spin_lock(&med->med_open_lock); mfd = mdt_handle2mfd(&(info->mti_body->handle)); if (mfd == NULL) { spin_unlock(&med->med_open_lock); - CDEBUG(D_INODE, "no handle for file close ino "DFID3 - ": cookie "LPX64, PFID3(&info->mti_body->fid1), + CDEBUG(D_INODE, "no handle for file close: fid = "DFID3 + ": cookie = "LPX64, PFID3(&info->mti_body->fid1), info->mti_body->handle.cookie); - RETURN(-ESTALE); - } - class_handle_unhash(&mfd->mfd_handle); - list_del_init(&mfd->mfd_list); - spin_unlock(&med->med_open_lock); - /* mdt_handle2mfd increase reference count, we must drop it here */ - mdt_mfd_put(mfd); - - rc = mdt_handle_last_unlink(info, mfd->mfd_object, - &RQF_MDS_CLOSE_LAST); + rc = -ESTALE; + } else { + class_handle_unhash(&mfd->mfd_handle); + list_del_init(&mfd->mfd_list); + spin_unlock(&med->med_open_lock); + + /* mdt_handle2mfd increases reference count on mfd, + * we must drop it here. */ + mdt_mfd_put(mfd); - rc = mdt_mfd_close(info->mti_ctxt, mfd, 1); + rc = mdt_handle_last_unlink(info, mfd->mfd_object, + &RQF_MDS_CLOSE_LAST); + rc = mdt_mfd_close(info->mti_ctxt, mfd, 1); + } RETURN(rc); } diff --git a/lustre/mdt/mdt_reint.c b/lustre/mdt/mdt_reint.c index c2f2af2..14a61ad 100644 --- a/lustre/mdt/mdt_reint.c +++ b/lustre/mdt/mdt_reint.c @@ -66,7 +66,7 @@ static int mdt_md_create(struct mdt_thread_info *info) rc = mdo_create(info->mti_ctxt, next, rr->rr_name, mdt_object_child(child), rr->rr_tgt, attr); if (rc == 0) { - /* return fid to client. attr is over-written!!*/ + /* return fid & attr to client. attr is over-written!*/ rc = mo_attr_get(info->mti_ctxt, mdt_object_child(child), attr); @@ -76,6 +76,8 @@ static int mdt_md_create(struct mdt_thread_info *info) mdt_object_fid(child)); } else if (rc == -EREMOTE) { /* parent is local, child is remote. */ + /*FIXME: should be return 0 or -EREMOTE? */ + /* also in mdt_open:mdt_object_open() */ repbody->fid1 = *mdt_object_fid(child); repbody->valid |= OBD_MD_FLID; } @@ -310,10 +312,10 @@ static int mdt_reint_link(struct mdt_thread_info *info) { struct mdt_reint_record *rr = &info->mti_rr; struct ptlrpc_request *req = mdt_info_req(info); - struct mdt_object *mp; struct mdt_object *ms; - struct mdt_lock_handle *lhp; + struct mdt_object *mp; struct mdt_lock_handle *lhs; + struct mdt_lock_handle *lhp; int rc; ENTRY; @@ -324,26 +326,26 @@ static int mdt_reint_link(struct mdt_thread_info *info) /* MDS_CHECK_RESENT here */ /* step 1: lock the source */ - lhp = &info->mti_lh[MDT_LH_PARENT]; - lhp->mlh_mode = LCK_EX; - mp = mdt_object_find_lock(info, rr->rr_fid1, lhp, MDS_INODELOCK_UPDATE); - if (IS_ERR(mp)) - RETURN(PTR_ERR(mp)); + lhs = &info->mti_lh[MDT_LH_PARENT]; + lhs->mlh_mode = LCK_EX; + ms = mdt_object_find_lock(info, rr->rr_fid1, lhs, MDS_INODELOCK_UPDATE); + if (IS_ERR(ms)) + RETURN(PTR_ERR(ms)); if (strlen(rr->rr_name) == 0) { if (req->rq_export->exp_connect_flags & OBD_CONNECT_RDONLY) GOTO(out_unlock_source, rc = -EROFS); /* remote partial operation */ - rc = mo_ref_add(info->mti_ctxt, mdt_object_child(mp)); + rc = mo_ref_add(info->mti_ctxt, mdt_object_child(ms)); GOTO(out_unlock_source, rc); } - /*step 2: find & lock the target */ - lhs = &info->mti_lh[MDT_LH_CHILD]; - lhs->mlh_mode = LCK_EX; - ms = mdt_object_find_lock(info, rr->rr_fid2, lhs, MDS_INODELOCK_UPDATE); - if (IS_ERR(ms)) - GOTO(out_unlock_source, rc = PTR_ERR(ms)); + /*step 2: find & lock the target parent dir*/ + lhp = &info->mti_lh[MDT_LH_CHILD]; + lhp->mlh_mode = LCK_EX; + mp = mdt_object_find_lock(info, rr->rr_fid2, lhp, MDS_INODELOCK_UPDATE); + if (IS_ERR(mp)) + GOTO(out_unlock_source, rc = PTR_ERR(mp)); if (req->rq_export->exp_connect_flags & OBD_CONNECT_RDONLY) GOTO(out_unlock_target, rc = -EROFS); @@ -354,9 +356,9 @@ static int mdt_reint_link(struct mdt_thread_info *info) GOTO(out_unlock_target, rc); out_unlock_target: - mdt_object_unlock_put(info, ms, lhs); -out_unlock_source: mdt_object_unlock_put(info, mp, lhp); +out_unlock_source: + mdt_object_unlock_put(info, ms, lhs); return rc; } diff --git a/lustre/mdt/mdt_xattr.c b/lustre/mdt/mdt_xattr.c index e9c7f86..1ac9459 100644 --- a/lustre/mdt/mdt_xattr.c +++ b/lustre/mdt/mdt_xattr.c @@ -103,9 +103,11 @@ int mdt_getxattr(struct mdt_thread_info *info) LASSERT(lu_object_assert_exists(info->mti_ctxt, &info->mti_object->mot_obj.mo_lu)); - if (MDT_FAIL_CHECK(OBD_FAIL_MDS_GETXATTR_PACK)) { - RETURN(rc = -ENOMEM); - } + CDEBUG(D_INODE, "getxattr "DFID3"\n", + PFID3(&info->mti_body->fid1)); + + if (MDT_FAIL_CHECK(OBD_FAIL_MDS_GETXATTR_PACK)) + RETURN(-ENOMEM); next = mdt_object_child(info->mti_object); @@ -162,18 +164,16 @@ int mdt_setxattr(struct mdt_thread_info *info) int rc; ENTRY; + CDEBUG(D_INODE, "setxattr "DFID3"\n", + PFID3(&info->mti_body->fid1)); - DEBUG_REQ(D_INODE, req, "setxattr "DFID3"\n", - PFID3(&info->mti_body->fid1)); - if (MDT_FAIL_CHECK(OBD_FAIL_MDS_SETXATTR)) { - RETURN(rc = -ENOMEM); - } + if (MDT_FAIL_CHECK(OBD_FAIL_MDS_SETXATTR)) + RETURN(-ENOMEM); /* various sanity check for xattr name */ xattr_name = req_capsule_client_get(&info->mti_pill, &RMF_NAME); - if (!xattr_name) { + if (!xattr_name) GOTO(out, rc = -EFAULT); - } CDEBUG(D_INODE, "%s xattr %s\n", info->mti_body->valid & OBD_MD_FLXATTR ? "set" : "remove",