From 67c4cc480e00e6265ee09b6d16401341b0b956ab Mon Sep 17 00:00:00 2001 From: yury Date: Thu, 24 Aug 2006 15:27:25 +0000 Subject: [PATCH] - add libiam.a (if exists) to rpm, this is neededfor testing our branch; - replace all memcmp of fids by lu_fid_eq; - allocate new fid for create case in ll_revalidate_it() in case of doing "goto do_lookup"; - removed needless lmv_fld_lookup() in lmv; - fixed lmv_obj refcount management in error cases; - comments are adjusted to be coherent with 80 columns style. --- lustre/llite/dcache.c | 49 ++++++++++++++++++++++++++++++++++++++--------- lustre/lmv/lmv_intent.c | 51 ++++++++++++++++++++++++++----------------------- lustre/lmv/lmv_obd.c | 6 ++---- lustre/mdc/mdc_locks.c | 4 ++-- 4 files changed, 71 insertions(+), 39 deletions(-) diff --git a/lustre/llite/dcache.c b/lustre/llite/dcache.c index 4ea16cf..ca33b76 100644 --- a/lustre/llite/dcache.c +++ b/lustre/llite/dcache.c @@ -394,6 +394,14 @@ int ll_revalidate_it(struct dentry *de, int lookup_flags, RETURN(-ENOMEM); if (it->it_op & IT_CREAT) { + /* + * Allocate new fid for case of create or open(O_CREAT). In both + * cases it->it_op will contain IT_CREAT. In case of + * open(O_CREAT) agains existing file, fid allocating is not + * needed, but this is not known until server returns + * anything. Well, in this case new allocated fid is lost. But + * this is not big deal, we have 64bit fids. --umka + */ struct lu_placement_hint hint = { .ph_pname = NULL, .ph_pfid = ll_inode2fid(parent), .ph_cname = &de->d_name, @@ -417,13 +425,16 @@ int ll_revalidate_it(struct dentry *de, int lookup_flags, struct ll_inode_info *lli = ll_i2info(inode); struct obd_client_handle **och_p; __u64 *och_usecount; - /* We used to check for MDS_INODELOCK_OPEN here, but in fact + + /* + * We used to check for MDS_INODELOCK_OPEN here, but in fact * just having LOOKUP lock is enough to justify inode is the * same. And if inode is the same and we have suitable * openhandle, then there is no point in doing another OPEN RPC - * just to throw away newly received openhandle. - * There are no security implications too, if file owner or - * access mode is change, LOOKUP lock is revoked */ + * just to throw away newly received openhandle. There are no + * security implications too, if file owner or access mode is + * change, LOOKUP lock is revoked. + */ it->it_create_mode &= ~current->fs->umask; @@ -535,7 +546,8 @@ out: } RETURN(rc); - /* This part is here to combat evil-evil race in real_lookup on 2.6 + /* + * This part is here to combat evil-evil race in real_lookup on 2.6 * kernels. The race details are: We enter do_lookup() looking for some * name, there is nothing in dcache for this name yet and d_lookup() * returns NULL. We proceed to real_lookup(), and while we do this, @@ -547,7 +559,8 @@ out: * returns -ENOENT in such a case instead of retrying the lookup. Once * this is dealt with in real_lookup(), all of this ugly mess can go and * we can just check locks in ->d_revalidate without doing any RPCs - * ever. */ + * ever. + */ do_lookup: if (it != &lookup_it) { ll_lookup_finish_locks(it, de); @@ -559,8 +572,27 @@ do_lookup: RETURN(-ENOMEM); /* do real lookup here */ - ll_prepare_md_op_data(op_data, de->d_parent->d_inode, NULL, + ll_prepare_md_op_data(op_data, parent, NULL, de->d_name.name, de->d_name.len, 0); + + if (it->it_op & IT_CREAT) { + /* + * Allocate new fid for case of create or open with O_CREAT. In + * both cases it->it_op will contain IT_CREAT. + */ + struct lu_placement_hint hint = { .ph_pname = NULL, + .ph_pfid = ll_inode2fid(parent), + .ph_cname = &de->d_name, + .ph_opc = LUSTRE_OPC_CREATE }; + + rc = ll_fid_md_alloc(ll_i2sbi(parent), &op_data->fid2, + &hint); + if (rc) { + CERROR("can't allocate new fid, rc %d\n", rc); + LBUG(); + } + } + rc = md_intent_lock(exp, op_data, NULL, 0, it, 0, &req, ll_md_blocking_ast, 0); if (rc >= 0) { @@ -568,8 +600,7 @@ do_lookup: DLM_REPLY_REC_OFF, sizeof(*mdt_body)); /* see if we got same inode, if not - return error */ - if(!memcmp(&op_data->fid2, &mdt_body->fid1, - sizeof(op_data->fid2))) + if(lu_fid_eq(&op_data->fid2, &mdt_body->fid1)) goto revalidate_finish; ll_intent_release(it); } diff --git a/lustre/lmv/lmv_intent.c b/lustre/lmv/lmv_intent.c index 2165daa..bdb7be4 100644 --- a/lustre/lmv/lmv_intent.c +++ b/lustre/lmv/lmv_intent.c @@ -138,6 +138,10 @@ out: return rc; } +/* + * IT_OPEN is intended to open (and create, possible) an object. Parent (pid) + * may be split dir. + */ int lmv_intent_open(struct obd_export *exp, const struct lu_fid *pid, const char *name, int len, void *lmm, int lmmsize, const struct lu_fid *cid, struct lookup_intent *it, @@ -160,11 +164,6 @@ int lmv_intent_open(struct obd_export *exp, const struct lu_fid *pid, if (op_data == NULL) RETURN(-ENOMEM); - /* - * IT_OPEN is intended to open (and create, possible) an object. Parent - * (pid) may be split dir. - */ - repeat: LASSERT(++loop <= 2); rc = lmv_fld_lookup(lmv, &rpid, &mds); @@ -215,7 +214,7 @@ repeat: LASSERT(rc < 0); /* - * this is possible, that some userspace application will try to + * This is possible, that some userspace application will try to * open file as directory and we will have -ENOTDIR here. As * this is "usual" situation, we should not print error here, * only debug info. @@ -311,8 +310,10 @@ int lmv_intent_getattr(struct obd_export *exp, const struct lu_fid *pid, if (!lu_fid_eq(pid, cid)){ rpid = obj->lo_inodes[mds].li_fid; rc = lmv_fld_lookup(lmv, &rpid, &mds); - if (rc) + if (rc) { + lmv_obj_put(obj); GOTO(out_free_op_data, rc); + } } lmv_obj_put(obj); } @@ -331,8 +332,10 @@ int lmv_intent_getattr(struct obd_export *exp, const struct lu_fid *pid, (char *)name, len); rpid = obj->lo_inodes[mds].li_fid; rc = lmv_fld_lookup(lmv, &rpid, &mds); - if (rc) + if (rc) { + lmv_obj_put(obj); GOTO(out_free_op_data, rc); + } lmv_obj_put(obj); CDEBUG(D_OTHER, "forward to MDS #"LPU64" (slave "DFID")\n", @@ -575,7 +578,7 @@ int lmv_intent_lookup(struct obd_export *exp, const struct lu_fid *pid, */ if (cid) { /* - * this is revalidation: we have to check is LOOKUP lock still + * This is revalidate: we have to check is LOOKUP lock still * valid for given fid. Very important part is that we have to * choose right mds because namespace is per mds. */ @@ -603,7 +606,7 @@ repeat: LASSERT(++loop <= 2); /* - * this is lookup. during lookup we have to update all the + * This is lookup. During lookup we have to update all the * attributes, because returned values will be put in struct * inode. */ @@ -615,12 +618,14 @@ repeat: (char *)name, len); rpid = obj->lo_inodes[mds].li_fid; rc = lmv_fld_lookup(lmv, &rpid, &mds); - if (rc) + if (rc) { + lmv_obj_put(obj); GOTO(out_free_op_data, rc); + } } lmv_obj_put(obj); } - memset(&op_data->fid2, 0, sizeof(op_data->fid2)); + fid_zero(&op_data->fid2); } op_data->fid1 = rpid; @@ -706,21 +711,19 @@ int lmv_intent_lock(struct obd_export *exp, struct md_op_data *op_data, const char *name = op_data->name; int len = op_data->namelen; struct lu_fid *pid, *cid; - mdsno_t mds; int rc; ENTRY; - LASSERT(it); - - pid = fid_is_sane(&op_data->fid1) ? &op_data->fid1 : NULL; - cid = fid_is_sane(&op_data->fid2) ? &op_data->fid2 : NULL; - - rc = lmv_fld_lookup(lmv, pid, &mds); - if (rc) - RETURN(rc); + LASSERT(it != NULL); + LASSERT(fid_is_sane(&op_data->fid1)); + + pid = &op_data->fid1; - CDEBUG(D_OTHER, "INTENT LOCK '%s' for '%*s' on "DFID" -> #"LPU64"\n", - LL_IT2STR(it), len, name, PFID(pid), mds); + cid = !fid_is_sane(&op_data->fid2) ? + &op_data->fid2 : NULL; + + CDEBUG(D_OTHER, "INTENT LOCK '%s' for '%*s' on "DFID"\n", + LL_IT2STR(it), len, name, PFID(pid)); rc = lmv_check_connect(obd); if (rc) @@ -734,7 +737,7 @@ int lmv_intent_lock(struct obd_export *exp, struct md_op_data *op_data, rc = lmv_intent_open(exp, pid, name, len, lmm, lmmsize, cid, it, flags, reqp, cb_blocking, extra_lock_flags); - else if (it->it_op & IT_GETATTR/* || it->it_op & IT_CHDIR*/) + else if (it->it_op & IT_GETATTR) rc = lmv_intent_getattr(exp, pid, name, len, lmm, lmmsize, cid, it, flags, reqp, cb_blocking, extra_lock_flags); diff --git a/lustre/lmv/lmv_obd.c b/lustre/lmv/lmv_obd.c index 3d4713b..c71381f 100644 --- a/lustre/lmv/lmv_obd.c +++ b/lustre/lmv/lmv_obd.c @@ -1474,9 +1474,6 @@ lmv_getattr_name(struct obd_export *exp, const struct lu_fid *fid, if (rc) RETURN(rc); - rc = lmv_fld_lookup(lmv, fid, &mds); - if (rc) - RETURN(rc); repeat: LASSERT(++loop <= 2); obj = lmv_obj_grab(obd, fid); @@ -1488,7 +1485,7 @@ repeat: lmv_obj_put(obj); } - CDEBUG(D_OTHER, "getattr_lock for %*s on "DFID" -> "DFID"\n", + CDEBUG(D_OTHER, "getattr_name for %*s on "DFID" -> "DFID"\n", namelen, filename, PFID(fid), PFID(&rid)); tgt_exp = lmv_get_export(lmv, &rid); @@ -1579,6 +1576,7 @@ static int lmv_link(struct obd_export *exp, struct md_op_data *op_data, CDEBUG(D_OTHER, "forward to MDS #"LPU64" ("DFID")\n", mds, PFID(&op_data->fid1)); + rc = md_link(lmv->tgts[mds].ltd_exp, op_data, request); RETURN(rc); diff --git a/lustre/mdc/mdc_locks.c b/lustre/mdc/mdc_locks.c index 4eb55fd..a6a5894 100644 --- a/lustre/mdc/mdc_locks.c +++ b/lustre/mdc/mdc_locks.c @@ -708,9 +708,9 @@ int mdc_intent_lock(struct obd_export *exp, struct md_op_data *op_data, if (fid_is_sane(&op_data->fid2) && it->it_flags & O_CHECK_STALE && it->it_op != IT_GETATTR) { it_set_disposition(it, DISP_ENQ_COMPLETE); + /* Also: did we find the same inode? */ - if (memcmp(&op_data->fid2, &mdt_body->fid1, - sizeof(op_data->fid2))) + if (!lu_fid_eq(&op_data->fid2, &mdt_body->fid1)) RETURN(-ESTALE); } -- 1.8.3.1