From 1cb9e85039c8df5fbe061aad3b0666c59ff7aa4e Mon Sep 17 00:00:00 2001 From: Vitaly Fertman Date: Fri, 29 Jul 2016 00:44:14 +0300 Subject: [PATCH] LU-7433 ldlm: xattr locks are lost on mdt mdt_intent_getxattr() can return EFAULT if a buffer cannot be found, it is returned after lock_replace, where a new lock is installed into lockp. An error forces ldlm_lock_enqueue() to destroy the original lock, but ldlm_handle_enqueue0() drops the reference on the new lock. xattr client code implied intent error is returned under a lock, which is immediately cancelled. Check if a lock obtained and cancel it properly for error cases. Note: we should support both cases for interop needs, an intent error under a lock and with a lock abort. Keep returning a lock with an intent error for interop purposes for now, to be dropped later when client will get old enough. make all intent ops to work through md_intent_lock: getxattr and layout, which should extract the intent error. Signed-off-by: Vitaly Fertman Change-Id: I7b628b50448c4bdb26a3a8758fc16a44212ad9ac Seagate-bug-id: MRP-3072 MRP-3137 Reviewed-by: Andrew Perepechko Reviewed-by: Andriy Skulysh Tested-by: Elena V. Gryaznova Reviewed-on: http://review.whamcloud.com/17220 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: John L. Hammond Reviewed-by: Lai Siyao Reviewed-by: Oleg Drokin --- lustre/include/obd.h | 13 +++- lustre/include/obd_class.h | 3 +- lustre/include/obd_support.h | 1 + lustre/llite/file.c | 16 ++--- lustre/llite/xattr_cache.c | 76 +++++++++-------------- lustre/lmv/lmv_intent.c | 13 ++-- lustre/lmv/lmv_obd.c | 12 ++-- lustre/mdc/mdc_internal.h | 2 +- lustre/mdc/mdc_locks.c | 144 ++++++++++++++++++++++++++----------------- lustre/mdt/mdt_handler.c | 27 +++++--- lustre/mdt/mdt_xattr.c | 6 +- lustre/tests/sanity.sh | 23 +++++++ lustre/tests/sanityn.sh | 28 +++++++++ 13 files changed, 222 insertions(+), 142 deletions(-) diff --git a/lustre/include/obd.h b/lustre/include/obd.h index 349d660..76afcd1 100644 --- a/lustre/include/obd.h +++ b/lustre/include/obd.h @@ -792,6 +792,16 @@ enum md_cli_flags { CLI_MIGRATE = 1 << 4, }; +/** + * GETXATTR is not included as only a couple of fields in the reply body + * is filled, but not FID which is needed for common intent handling in + * mdc_finish_intent_lock() + */ +static inline bool it_has_reply_body(const struct lookup_intent *it) +{ + return it->it_op & (IT_OPEN | IT_UNLINK | IT_LOOKUP | IT_GETATTR); +} + struct md_op_data { struct lu_fid op_fid1; /* operation fid1 (usualy parent) */ struct lu_fid op_fid2; /* operation fid2 (usualy child) */ @@ -1018,8 +1028,7 @@ struct md_ops { cfs_cap_t, __u64, struct ptlrpc_request **); int (*m_enqueue)(struct obd_export *, struct ldlm_enqueue_info *, - const union ldlm_policy_data *, - struct lookup_intent *, struct md_op_data *, + const union ldlm_policy_data *, struct md_op_data *, struct lustre_handle *, __u64); int (*m_getattr)(struct obd_export *, struct md_op_data *, diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index d6127a7..a8b2043 100644 --- a/lustre/include/obd_class.h +++ b/lustre/include/obd_class.h @@ -1326,7 +1326,6 @@ static inline int md_create(struct obd_export *exp, struct md_op_data *op_data, static inline int md_enqueue(struct obd_export *exp, struct ldlm_enqueue_info *einfo, const union ldlm_policy_data *policy, - struct lookup_intent *it, struct md_op_data *op_data, struct lustre_handle *lockh, __u64 extra_lock_flags) @@ -1335,7 +1334,7 @@ static inline int md_enqueue(struct obd_export *exp, ENTRY; EXP_CHECK_MD_OP(exp, enqueue); EXP_MD_COUNTER_INCREMENT(exp, enqueue); - rc = MDP(exp->exp_obd, enqueue)(exp, einfo, policy, it, op_data, lockh, + rc = MDP(exp->exp_obd, enqueue)(exp, einfo, policy, op_data, lockh, extra_lock_flags); RETURN(rc); } diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h index ee9d9ad..70e21dd 100644 --- a/lustre/include/obd_support.h +++ b/lustre/include/obd_support.h @@ -250,6 +250,7 @@ extern char obd_jobid_var[]; #define OBD_FAIL_MDS_LLOG_CREATE_FAILED2 0x15b #define OBD_FAIL_MDS_FLD_LOOKUP 0x15c #define OBD_FAIL_MDS_INTENT_DELAY 0x160 +#define OBD_FAIL_MDS_XATTR_REP 0x161 #define OBD_FAIL_MDS_TRACK_OVERFLOW 0x162 /* layout lock */ diff --git a/lustre/llite/file.c b/lustre/llite/file.c index b7bd2bf..4c8abc1 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -3098,7 +3098,7 @@ ll_file_flock(struct file *file, int cmd, struct file_lock *file_lock) flock.l_flock.pid, flags, einfo.ei_mode, flock.l_flock.start, flock.l_flock.end); - rc = md_enqueue(sbi->ll_md_exp, &einfo, &flock, NULL, op_data, &lockh, + rc = md_enqueue(sbi->ll_md_exp, &einfo, &flock, op_data, &lockh, flags); /* Restore the file lock type if not TEST lock. */ @@ -3121,7 +3121,7 @@ ll_file_flock(struct file *file, int cmd, struct file_lock *file_lock) if (rc2 && file_lock->fl_type != F_UNLCK) { einfo.ei_mode = LCK_NL; - md_enqueue(sbi->ll_md_exp, &einfo, &flock, NULL, op_data, + md_enqueue(sbi->ll_md_exp, &einfo, &flock, op_data, &lockh, flags); rc = rc2; } @@ -4161,12 +4161,7 @@ static int ll_layout_refresh_locked(struct inode *inode) struct lookup_intent it; struct lustre_handle lockh; enum ldlm_mode mode; - struct ldlm_enqueue_info einfo = { - .ei_type = LDLM_IBITS, - .ei_mode = LCK_CR, - .ei_cb_bl = &ll_md_blocking_ast, - .ei_cb_cp = &ldlm_completion_ast, - }; + struct ptlrpc_request *req; int rc; ENTRY; @@ -4191,13 +4186,13 @@ again: /* have to enqueue one */ memset(&it, 0, sizeof(it)); it.it_op = IT_LAYOUT; - lockh.cookie = 0ULL; LDLM_DEBUG_NOLOCK("%s: requeue layout lock for file "DFID"(%p)", ll_get_fsname(inode->i_sb, NULL, 0), PFID(&lli->lli_fid), inode); - rc = md_enqueue(sbi->ll_md_exp, &einfo, NULL, &it, op_data, &lockh, 0); + rc = md_intent_lock(sbi->ll_md_exp, op_data, &it, &req, + &ll_md_blocking_ast, 0); if (it.it_request != NULL) ptlrpc_req_finished(it.it_request); it.it_request = NULL; @@ -4211,6 +4206,7 @@ again: if (rc == 0) { /* set lock data in case this is a new lock */ ll_set_lock_data(sbi->ll_md_exp, inode, &it, NULL); + lockh.cookie = it.it_lock_handle; rc = ll_layout_lock_set(&lockh, mode, inode); if (rc == -EAGAIN) goto again; diff --git a/lustre/llite/xattr_cache.c b/lustre/llite/xattr_cache.c index a619db3..cd9e2a5 100644 --- a/lustre/llite/xattr_cache.c +++ b/lustre/llite/xattr_cache.c @@ -316,12 +316,6 @@ static int ll_xattr_find_get_lock(struct inode *inode, struct lustre_handle lockh = { 0 }; struct md_op_data *op_data; struct ll_inode_info *lli = ll_i2info(inode); - struct ldlm_enqueue_info einfo = { - .ei_type = LDLM_IBITS, - .ei_mode = it_to_lock_mode(oit), - .ei_cb_bl = &ll_md_blocking_ast, - .ei_cb_cp = &ldlm_completion_ast, - }; struct ll_sb_info *sbi = ll_i2sbi(inode); struct obd_export *exp = sbi->ll_md_exp; int rc; @@ -353,8 +347,9 @@ static int ll_xattr_find_get_lock(struct inode *inode, op_data->op_valid = OBD_MD_FLXATTR | OBD_MD_FLXATTRLS; - rc = md_enqueue(exp, &einfo, NULL, oit, op_data, &lockh, 0); + rc = md_intent_lock(exp, op_data, oit, req, &ll_md_blocking_ast, 0); ll_finish_md_op_data(op_data); + *req = oit->it_request; if (rc < 0) { CDEBUG(D_CACHE, "md_intent_lock failed with %d for fid "DFID"\n", @@ -363,7 +358,6 @@ static int ll_xattr_find_get_lock(struct inode *inode, RETURN(rc); } - *req = oit->it_request; out: down_write(&lli->lli_xattrs_list_rwsem); mutex_unlock(&lli->lli_xattrs_enq_lock); @@ -374,16 +368,15 @@ out: /** * Refill the xattr cache. * - * Fetch and cache the whole of xattrs for @inode, acquiring - * a read or a write xattr lock depending on operation in @oit. - * Intent is dropped on exit unless the operation is setxattr. + * Fetch and cache the whole of xattrs for @inode, acquiring a read lock. * * \retval 0 no error occured * \retval -EPROTO network protocol error * \retval -ENOMEM not enough memory for the cache */ -static int ll_xattr_cache_refill(struct inode *inode, struct lookup_intent *oit) +static int ll_xattr_cache_refill(struct inode *inode) { + struct lookup_intent oit = { .it_op = IT_GETXATTR }; struct ll_sb_info *sbi = ll_i2sbi(inode); struct ptlrpc_request *req = NULL; const char *xdata, *xval, *xtail, *xvtail; @@ -394,36 +387,28 @@ static int ll_xattr_cache_refill(struct inode *inode, struct lookup_intent *oit) ENTRY; - rc = ll_xattr_find_get_lock(inode, oit, &req); + rc = ll_xattr_find_get_lock(inode, &oit, &req); if (rc) - GOTO(out_no_unlock, rc); + GOTO(err_req, rc); /* Do we have the data at this point? */ if (ll_xattr_cache_valid(lli)) { ll_stats_ops_tally(sbi, LPROC_LL_GETXATTR_HITS, 1); - GOTO(out_maybe_drop, rc = 0); + ll_intent_drop_lock(&oit); + GOTO(err_req, rc = 0); } /* Matched but no cache? Cancelled on error by a parallel refill. */ if (unlikely(req == NULL)) { CDEBUG(D_CACHE, "cancelled by a parallel getxattr\n"); - GOTO(out_maybe_drop, rc = -EIO); - } - - if (oit->it_status < 0) { - CDEBUG(D_CACHE, "getxattr intent returned %d for fid "DFID"\n", - oit->it_status, PFID(ll_inode2fid(inode))); - rc = oit->it_status; - /* xattr data is so large that we don't want to cache it */ - if (rc == -ERANGE) - rc = -EAGAIN; - GOTO(out_destroy, rc); + ll_intent_drop_lock(&oit); + GOTO(err_unlock, rc = -EIO); } body = req_capsule_server_get(&req->rq_pill, &RMF_MDT_BODY); if (body == NULL) { CERROR("no MDT BODY in the refill xattr reply\n"); - GOTO(out_destroy, rc = -EPROTO); + GOTO(err_cancel, rc = -EPROTO); } /* do not need swab xattr data */ xdata = req_capsule_server_sized_get(&req->rq_pill, &RMF_EADATA, @@ -435,7 +420,7 @@ static int ll_xattr_cache_refill(struct inode *inode, struct lookup_intent *oit) sizeof(__u32)); if (xdata == NULL || xval == NULL || xsizes == NULL) { CERROR("wrong setxattr reply\n"); - GOTO(out_destroy, rc = -EPROTO); + GOTO(err_cancel, rc = -EPROTO); } xtail = xdata + body->mbo_eadatasize; @@ -471,7 +456,7 @@ static int ll_xattr_cache_refill(struct inode *inode, struct lookup_intent *oit) } if (rc < 0) { ll_xattr_cache_destroy_locked(lli); - GOTO(out_destroy, rc); + GOTO(err_cancel, rc); } xdata += strlen(xdata) + 1; xval += *xsizes; @@ -481,28 +466,24 @@ static int ll_xattr_cache_refill(struct inode *inode, struct lookup_intent *oit) if (xdata != xtail || xval != xvtail) CERROR("a hole in xattr data\n"); - ll_set_lock_data(sbi->ll_md_exp, inode, oit, NULL); - - GOTO(out_maybe_drop, rc); -out_maybe_drop: + ll_set_lock_data(sbi->ll_md_exp, inode, &oit, NULL); + ll_intent_drop_lock(&oit); - ll_intent_drop_lock(oit); - - if (rc != 0) - up_write(&lli->lli_xattrs_list_rwsem); -out_no_unlock: ptlrpc_req_finished(req); + RETURN(0); - return rc; - -out_destroy: - up_write(&lli->lli_xattrs_list_rwsem); - +err_cancel: ldlm_lock_decref_and_cancel((struct lustre_handle *) - &oit->it_lock_handle, - oit->it_lock_mode); + &oit.it_lock_handle, + oit.it_lock_mode); +err_unlock: + up_write(&lli->lli_xattrs_list_rwsem); +err_req: + if (rc == -ERANGE) + rc = -EAGAIN; - goto out_no_unlock; + ptlrpc_req_finished(req); + return rc; } /** @@ -525,7 +506,6 @@ int ll_xattr_cache_get(struct inode *inode, size_t size, __u64 valid) { - struct lookup_intent oit = { .it_op = IT_GETXATTR }; struct ll_inode_info *lli = ll_i2info(inode); int rc = 0; @@ -536,7 +516,7 @@ int ll_xattr_cache_get(struct inode *inode, down_read(&lli->lli_xattrs_list_rwsem); if (!ll_xattr_cache_valid(lli)) { up_read(&lli->lli_xattrs_list_rwsem); - rc = ll_xattr_cache_refill(inode, &oit); + rc = ll_xattr_cache_refill(inode); if (rc) RETURN(rc); downgrade_write(&lli->lli_xattrs_list_rwsem); diff --git a/lustre/lmv/lmv_intent.c b/lustre/lmv/lmv_intent.c index 7ae1600..1b8cabf 100644 --- a/lustre/lmv/lmv_intent.c +++ b/lustre/lmv/lmv_intent.c @@ -444,6 +444,9 @@ lmv_intent_lookup(struct obd_export *exp, struct md_op_data *op_data, } } + if (!it_has_reply_body(it)) + RETURN(0); + /* * MDS has returned success. Probably name has been resolved in * remote inode. Let's check this. @@ -482,7 +485,7 @@ int lmv_intent_lock(struct obd_export *exp, struct md_op_data *op_data, (int)op_data->op_namelen, op_data->op_name, PFID(&op_data->op_fid1)); - if (it->it_op & (IT_LOOKUP | IT_GETATTR | IT_LAYOUT)) + if (it->it_op & (IT_LOOKUP | IT_GETATTR | IT_LAYOUT | IT_GETXATTR)) rc = lmv_intent_lookup(exp, op_data, it, reqp, cb_blocking, extra_lock_flags); else if (it->it_op & IT_OPEN) @@ -496,8 +499,8 @@ int lmv_intent_lock(struct obd_export *exp, struct md_op_data *op_data, if (it->it_lock_mode != 0) { lock_handle.cookie = it->it_lock_handle; - ldlm_lock_decref(&lock_handle, - it->it_lock_mode); + ldlm_lock_decref_and_cancel(&lock_handle, + it->it_lock_mode); } it->it_lock_handle = 0; @@ -505,8 +508,8 @@ int lmv_intent_lock(struct obd_export *exp, struct md_op_data *op_data, if (it->it_remote_lock_mode != 0) { lock_handle.cookie = it->it_remote_lock_handle; - ldlm_lock_decref(&lock_handle, - it->it_remote_lock_mode); + ldlm_lock_decref_and_cancel(&lock_handle, + it->it_remote_lock_mode); } it->it_remote_lock_handle = 0; diff --git a/lustre/lmv/lmv_obd.c b/lustre/lmv/lmv_obd.c index d485cc2..74a29be 100644 --- a/lustre/lmv/lmv_obd.c +++ b/lustre/lmv/lmv_obd.c @@ -1707,8 +1707,7 @@ int lmv_create(struct obd_export *exp, struct md_op_data *op_data, static int lmv_enqueue(struct obd_export *exp, struct ldlm_enqueue_info *einfo, - const union ldlm_policy_data *policy, - struct lookup_intent *it, struct md_op_data *op_data, + const union ldlm_policy_data *policy, struct md_op_data *op_data, struct lustre_handle *lockh, __u64 extra_lock_flags) { struct obd_device *obd = exp->exp_obd; @@ -1717,17 +1716,16 @@ lmv_enqueue(struct obd_export *exp, struct ldlm_enqueue_info *einfo, int rc; ENTRY; - CDEBUG(D_INODE, "ENQUEUE '%s' on "DFID"\n", - LL_IT2STR(it), PFID(&op_data->op_fid1)); + CDEBUG(D_INODE, "ENQUEUE on "DFID"\n", PFID(&op_data->op_fid1)); tgt = lmv_locate_mds(lmv, op_data, &op_data->op_fid1); if (IS_ERR(tgt)) RETURN(PTR_ERR(tgt)); - CDEBUG(D_INODE, "ENQUEUE '%s' on "DFID" -> mds #%u\n", - LL_IT2STR(it), PFID(&op_data->op_fid1), tgt->ltd_idx); + CDEBUG(D_INODE, "ENQUEUE on "DFID" -> mds #%u\n", + PFID(&op_data->op_fid1), tgt->ltd_idx); - rc = md_enqueue(tgt->ltd_exp, einfo, policy, it, op_data, lockh, + rc = md_enqueue(tgt->ltd_exp, einfo, policy, op_data, lockh, extra_lock_flags); RETURN(rc); diff --git a/lustre/mdc/mdc_internal.h b/lustre/mdc/mdc_internal.h index b88d7f5..191a160 100644 --- a/lustre/mdc/mdc_internal.h +++ b/lustre/mdc/mdc_internal.h @@ -87,7 +87,7 @@ int mdc_intent_lock(struct obd_export *exp, int mdc_enqueue(struct obd_export *exp, struct ldlm_enqueue_info *einfo, const union ldlm_policy_data *policy, - struct lookup_intent *it, struct md_op_data *op_data, + struct md_op_data *op_data, struct lustre_handle *lockh, __u64 extra_lock_flags); int mdc_resource_get_unused(struct obd_export *exp, const struct lu_fid *fid, diff --git a/lustre/mdc/mdc_locks.c b/lustre/mdc/mdc_locks.c index 4f6f290..b8265afa 100644 --- a/lustre/mdc/mdc_locks.c +++ b/lustre/mdc/mdc_locks.c @@ -587,7 +587,7 @@ static int mdc_finish_enqueue(struct obd_export *exp, it->it_op, it->it_disposition, it->it_status); /* We know what to expect, so we do any byte flipping required here */ - if (it->it_op & (IT_OPEN | IT_UNLINK | IT_LOOKUP | IT_GETATTR)) { + if (it_has_reply_body(it)) { struct mdt_body *body; body = req_capsule_server_get(pill, &RMF_MDT_BODY); @@ -708,11 +708,13 @@ static int mdc_finish_enqueue(struct obd_export *exp, /* We always reserve enough space in the reply packet for a stripe MD, because * we don't know in advance the file type. */ -int mdc_enqueue(struct obd_export *exp, - struct ldlm_enqueue_info *einfo, - const union ldlm_policy_data *policy, - struct lookup_intent *it, struct md_op_data *op_data, - struct lustre_handle *lockh, __u64 extra_lock_flags) +static int mdc_enqueue_base(struct obd_export *exp, + struct ldlm_enqueue_info *einfo, + const union ldlm_policy_data *policy, + struct lookup_intent *it, + struct md_op_data *op_data, + struct lustre_handle *lockh, + __u64 extra_lock_flags) { struct obd_device *obddev = class_exp2obd(exp); struct ptlrpc_request *req = NULL; @@ -874,6 +876,15 @@ resend: RETURN(rc); } +int mdc_enqueue(struct obd_export *exp, struct ldlm_enqueue_info *einfo, + const union ldlm_policy_data *policy, + struct md_op_data *op_data, + struct lustre_handle *lockh, __u64 extra_lock_flags) +{ + return mdc_enqueue_base(exp, einfo, policy, NULL, + op_data, lockh, extra_lock_flags); +} + static int mdc_finish_intent_lock(struct obd_export *exp, struct ptlrpc_request *request, struct md_op_data *op_data, @@ -881,9 +892,8 @@ static int mdc_finish_intent_lock(struct obd_export *exp, struct lustre_handle *lockh) { struct lustre_handle old_lock; - struct mdt_body *mdt_body; struct ldlm_lock *lock; - int rc; + int rc = 0; ENTRY; LASSERT(request != NULL); @@ -893,48 +903,58 @@ static int mdc_finish_intent_lock(struct obd_export *exp, if (it->it_op & IT_READDIR) RETURN(0); - if (!it_disposition(it, DISP_IT_EXECD)) { - /* The server failed before it even started executing the - * intent, i.e. because it couldn't unpack the request. */ - LASSERT(it->it_status != 0); - RETURN(it->it_status); - } - rc = it_open_error(DISP_IT_EXECD, it); - if (rc) - RETURN(rc); - - mdt_body = req_capsule_server_get(&request->rq_pill, &RMF_MDT_BODY); - LASSERT(mdt_body != NULL); /* mdc_enqueue checked */ - - rc = it_open_error(DISP_LOOKUP_EXECD, it); - if (rc) - RETURN(rc); - - /* keep requests around for the multiple phases of the call - * this shows the DISP_XX must guarantee we make it into the call - */ - if (!it_disposition(it, DISP_ENQ_CREATE_REF) && - it_disposition(it, DISP_OPEN_CREATE) && - !it_open_error(DISP_OPEN_CREATE, it)) { - it_set_disposition(it, DISP_ENQ_CREATE_REF); - ptlrpc_request_addref(request); /* balanced in ll_create_node */ - } - if (!it_disposition(it, DISP_ENQ_OPEN_REF) && - it_disposition(it, DISP_OPEN_OPEN) && - !it_open_error(DISP_OPEN_OPEN, it)) { - it_set_disposition(it, DISP_ENQ_OPEN_REF); - ptlrpc_request_addref(request); /* balanced in ll_file_open */ - /* BUG 11546 - eviction in the middle of open rpc processing */ - OBD_FAIL_TIMEOUT(OBD_FAIL_MDC_ENQUEUE_PAUSE, obd_timeout); - } + if (it->it_op & (IT_GETXATTR | IT_LAYOUT)) { + if (it->it_status != 0) + GOTO(out, rc = it->it_status); + } else { + if (!it_disposition(it, DISP_IT_EXECD)) { + /* The server failed before it even started executing + * the intent, i.e. because it couldn't unpack the + * request. + */ + LASSERT(it->it_status != 0); + GOTO(out, rc = it->it_status); + } + rc = it_open_error(DISP_IT_EXECD, it); + if (rc) + GOTO(out, rc); + + rc = it_open_error(DISP_LOOKUP_EXECD, it); + if (rc) + GOTO(out, rc); + + /* keep requests around for the multiple phases of the call + * this shows the DISP_XX must guarantee we make it into the + * call + */ + if (!it_disposition(it, DISP_ENQ_CREATE_REF) && + it_disposition(it, DISP_OPEN_CREATE) && + !it_open_error(DISP_OPEN_CREATE, it)) { + it_set_disposition(it, DISP_ENQ_CREATE_REF); + /* balanced in ll_create_node */ + ptlrpc_request_addref(request); + } + if (!it_disposition(it, DISP_ENQ_OPEN_REF) && + it_disposition(it, DISP_OPEN_OPEN) && + !it_open_error(DISP_OPEN_OPEN, it)) { + it_set_disposition(it, DISP_ENQ_OPEN_REF); + /* balanced in ll_file_open */ + ptlrpc_request_addref(request); + /* BUG 11546 - eviction in the middle of open rpc + * processing + */ + OBD_FAIL_TIMEOUT(OBD_FAIL_MDC_ENQUEUE_PAUSE, + obd_timeout); + } - if (it->it_op & IT_CREAT) { - /* XXX this belongs in ll_create_it */ - } else if (it->it_op == IT_OPEN) { - LASSERT(!it_disposition(it, DISP_OPEN_CREATE)); - } else { - LASSERT(it->it_op & (IT_GETATTR | IT_LOOKUP | IT_LAYOUT)); - } + if (it->it_op & IT_CREAT) { + /* XXX this belongs in ll_create_it */ + } else if (it->it_op == IT_OPEN) { + LASSERT(!it_disposition(it, DISP_OPEN_CREATE)); + } else { + LASSERT(it->it_op & (IT_GETATTR | IT_LOOKUP)); + } + } /* If we already have a matching lock, then cancel the new * one. We have to set the data here instead of in @@ -946,10 +966,19 @@ static int mdc_finish_intent_lock(struct obd_export *exp, union ldlm_policy_data policy = lock->l_policy_data; LDLM_DEBUG(lock, "matching against this"); - LASSERTF(fid_res_name_eq(&mdt_body->mbo_fid1, - &lock->l_resource->lr_name), - "Lock res_id: "DLDLMRES", fid: "DFID"\n", - PLDLMRES(lock->l_resource), PFID(&mdt_body->mbo_fid1)); + if (it_has_reply_body(it)) { + struct mdt_body *body; + + body = req_capsule_server_get(&request->rq_pill, + &RMF_MDT_BODY); + /* mdc_enqueue checked */ + LASSERT(body != NULL); + LASSERTF(fid_res_name_eq(&body->mbo_fid1, + &lock->l_resource->lr_name), + "Lock res_id: "DLDLMRES", fid: "DFID"\n", + PLDLMRES(lock->l_resource), + PFID(&body->mbo_fid1)); + } LDLM_LOCK_PUT(lock); memcpy(&old_lock, lockh, sizeof(*lockh)); @@ -961,12 +990,13 @@ static int mdc_finish_intent_lock(struct obd_export *exp, } } + EXIT; +out: CDEBUG(D_DENTRY,"D_IT dentry %.*s intent: %s status %d disp %x rc %d\n", (int)op_data->op_namelen, op_data->op_name, ldlm_it2str(it->it_op), it->it_status, it->it_disposition, rc); - - RETURN(rc); + return rc; } int mdc_revalidate_lock(struct obd_export *exp, struct lookup_intent *it, @@ -1105,8 +1135,8 @@ int mdc_intent_lock(struct obd_export *exp, struct md_op_data *op_data, } } - rc = mdc_enqueue(exp, &einfo, NULL, it, op_data, &lockh, - extra_lock_flags); + rc = mdc_enqueue_base(exp, &einfo, NULL, it, op_data, &lockh, + extra_lock_flags); if (rc < 0) RETURN(rc); diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index d8ddcdc..aafd3b1 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -3309,7 +3309,8 @@ static int mdt_intent_getxattr(enum mdt_it_code opcode, { struct mdt_lock_handle *lhc = &info->mti_lh[MDT_LH_RMT]; struct ldlm_reply *ldlm_rep = NULL; - int rc, grc; + int rc; + ENTRY; /* * Initialize lhc->mlh_reg_lh either from a previously granted lock @@ -3325,18 +3326,30 @@ static int mdt_intent_getxattr(enum mdt_it_code opcode, return rc; } - grc = mdt_getxattr(info); - - rc = mdt_intent_lock_replace(info, lockp, lhc, flags, 0); + rc = mdt_getxattr(info); if (mdt_info_req(info)->rq_repmsg != NULL) ldlm_rep = req_capsule_server_get(info->mti_pill, &RMF_DLM_REP); - if (ldlm_rep == NULL) + + if (ldlm_rep == NULL || + OBD_FAIL_CHECK(OBD_FAIL_MDS_XATTR_REP)) { + mdt_object_unlock(info, info->mti_object, lhc, 1); RETURN(err_serious(-EFAULT)); + } - ldlm_rep->lock_policy_res2 = grc; + ldlm_rep->lock_policy_res2 = clear_serious(rc); - return rc; + /* This is left for interop instead of adding a new interop flag. + * LU-7433 */ +#if LUSTRE_VERSION_CODE > OBD_OCD_VERSION(3, 0, 0, 0) + if (ldlm_rep->lock_policy_res2) { + mdt_object_unlock(info, info->mti_object, lhc, 1); + RETURN(ELDLM_LOCK_ABORTED); + } +#endif + + rc = mdt_intent_lock_replace(info, lockp, lhc, flags, rc); + RETURN(rc); } static int mdt_intent_getattr(enum mdt_it_code opcode, diff --git a/lustre/mdt/mdt_xattr.c b/lustre/mdt/mdt_xattr.c index 32fb823..c25d471 100644 --- a/lustre/mdt/mdt_xattr.c +++ b/lustre/mdt/mdt_xattr.c @@ -63,9 +63,6 @@ static int mdt_getxattr_pack_reply(struct mdt_thread_info * info) int size, rc; ENTRY; - if (OBD_FAIL_CHECK(OBD_FAIL_MDS_GETXATTR_PACK)) - RETURN(-ENOMEM); - valid = info->mti_body->mbo_valid & (OBD_MD_FLXATTR | OBD_MD_FLXATTRLS); /* Determine how many bytes we need */ @@ -114,6 +111,9 @@ static int mdt_getxattr_pack_reply(struct mdt_thread_info * info) RETURN(rc); } + if (OBD_FAIL_CHECK(OBD_FAIL_MDS_GETXATTR_PACK)) + RETURN(-ENOMEM); + RETURN(size); } diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index b6cd7ec..7ed6ef9 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -14305,6 +14305,29 @@ test_256() { } run_test 256 "Check llog delete for empty and not full state" +test_257() { + remote_mds_nodsh && skip "remote MDS with nodsh" && return + [[ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.8.55) ]] && + skip "Need MDS version at least 2.8.55" && return + + test_mkdir -p $DIR/$tdir + + setfattr -n trusted.name1 -v value1 $DIR/$tdir || + error "setfattr -n trusted.name1=value1 $DIR/$tdir failed" + stat $DIR/$tdir + +#define OBD_FAIL_MDS_XATTR_REP 0x161 + local mdtidx=$($LFS getstripe -M $DIR/$tdir) + local facet=mds$((mdtidx + 1)) + set_nodes_failloc $(facet_active_host $facet) 0x80000161 + getfattr -n trusted.name1 $DIR/$tdir 2> /dev/null + + stop $facet || error "stop MDS failed" + start $facet $(mdsdevname $((mdtidx + 1))) $MDS_MOUNT_OPTS || + error "start MDS fail" +} +run_test 257 "xattr locks are not lost" + test_260() { #define OBD_FAIL_MDC_CLOSE 0x806 $LCTL set_param fail_loc=0x80000806 diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh index 19a7c31..8c332a4 100755 --- a/lustre/tests/sanityn.sh +++ b/lustre/tests/sanityn.sh @@ -3317,6 +3317,34 @@ test_78() { #LU-6673 } run_test 78 "Enable policy and specify tunings right away" +test_79() { + remote_mds_nodsh && skip "remote MDS with nodsh" && return + test_mkdir -p $DIR/$tdir + + # Prevent interference from layout intent RPCs due to + # asynchronous writeback. These will be tested in 130c below. + do_nodes ${CLIENTS:-$HOSTNAME} sync + + setfattr -n trusted.name1 -v value1 $DIR/$tdir || + error "setfattr -n trusted.name1=value1 $DIR/$tdir failed" + +#define OBD_FAIL_MDS_INTENT_DELAY 0x160 + local mdtidx=$($LFS getstripe -M $DIR/$tdir) + local facet=mds$((mdtidx + 1)) + stat $DIR/$tdir + set_nodes_failloc $(facet_active_host $facet) 0x80000160 + getfattr -n trusted.name1 $DIR/$tdir 2> /dev/null & + local pid=$! + sleep 2 + +#define OBD_FAIL_MDS_GETXATTR_PACK 0x131 + set_nodes_failloc $(facet_active_host $facet) 0x80000131 + + wait $pid + return 0 +} +run_test 79 "xattr: intent error" + test_80a() { [ $MDSCOUNT -lt 2 ] && skip "needs >= 2 MDTs" && return local MDTIDX=1 -- 1.8.3.1