From: Peng Tao <bergwolf@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org,
Andreas Dilger <andreas.dilger@intel.com>,
Lai Siyao <lai.siyao@intel.com>, Peng Tao <bergwolf@gmail.com>
Subject: [PATCH 9/9] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES
Date: Tue, 19 Nov 2013 21:23:48 +0800 [thread overview]
Message-ID: <1384867428-8395-10-git-send-email-bergwolf@gmail.com> (raw)
In-Reply-To: <1384867428-8395-1-git-send-email-bergwolf@gmail.com>
From: Andreas Dilger <andreas.dilger@intel.com>
In ll_md_blocking_ast() the FID/resource comparison is incorrectly
checking for fid_ver() stored in res_id.name[2] instead of name[1]
changed since http://review.whamcloud.com/2271 (commit 4f91d5161d00)
landed. This does not impact current clients, since name[2] and
fid_ver() are always zero, but it could cause problems in the future.
In ldlm_cli_enqueue_fini() use ldlm_res_eq() instead of comparing
each of the resource fields separately.
Use DLDLMRES/PLDLMRES when printing resource names everywhere.
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-2901
Lustre-change: http://review.whamcloud.com/6592
Signed-off-by: Lai Siyao <lai.siyao@intel.com>
Reviewed-by: Johann Lombardi <johann.lombardi@intel.com>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
---
drivers/staging/lustre/lustre/ldlm/ldlm_request.c | 32 +++++++-------------
drivers/staging/lustre/lustre/ldlm/ldlm_resource.c | 19 +++---------
drivers/staging/lustre/lustre/llite/namei.c | 5 +--
drivers/staging/lustre/lustre/mdc/mdc_locks.c | 9 ++----
drivers/staging/lustre/lustre/mgc/mgc_request.c | 4 +--
5 files changed, 21 insertions(+), 48 deletions(-)
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
index 1e2c0dd..1ddcca3 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
@@ -610,18 +610,12 @@ int ldlm_cli_enqueue_fini(struct obd_export *exp, struct ptlrpc_request *req,
lock->l_req_mode = newmode;
}
- if (memcmp(reply->lock_desc.l_resource.lr_name.name,
- lock->l_resource->lr_name.name,
- sizeof(struct ldlm_res_id))) {
- CDEBUG(D_INFO, "remote intent success, locking "
- "(%ld,%ld,%ld) instead of "
- "(%ld,%ld,%ld)\n",
- (long)reply->lock_desc.l_resource.lr_name.name[0],
- (long)reply->lock_desc.l_resource.lr_name.name[1],
- (long)reply->lock_desc.l_resource.lr_name.name[2],
- (long)lock->l_resource->lr_name.name[0],
- (long)lock->l_resource->lr_name.name[1],
- (long)lock->l_resource->lr_name.name[2]);
+ if (!ldlm_res_eq(&reply->lock_desc.l_resource.lr_name,
+ &lock->l_resource->lr_name)) {
+ CDEBUG(D_INFO, "remote intent success, locking "DLDLMRES
+ " instead of "DLDLMRES"\n",
+ PLDLMRES(&reply->lock_desc.l_resource),
+ PLDLMRES(lock->l_resource));
rc = ldlm_lock_change_resource(ns, lock,
&reply->lock_desc.l_resource.lr_name);
@@ -1912,7 +1906,8 @@ int ldlm_cli_cancel_unused_resource(struct ldlm_namespace *ns,
0, flags | LCF_BL_AST, opaque);
rc = ldlm_cli_cancel_list(&cancels, count, NULL, flags);
if (rc != ELDLM_OK)
- CERROR("ldlm_cli_cancel_unused_resource: %d\n", rc);
+ CERROR("canceling unused lock "DLDLMRES": rc = %d\n",
+ PLDLMRES(res), rc);
LDLM_RESOURCE_DELREF(res);
ldlm_resource_putref(res);
@@ -1930,15 +1925,10 @@ static int ldlm_cli_hash_cancel_unused(struct cfs_hash *hs, struct cfs_hash_bd *
{
struct ldlm_resource *res = cfs_hash_object(hs, hnode);
struct ldlm_cli_cancel_arg *lc = arg;
- int rc;
- rc = ldlm_cli_cancel_unused_resource(ldlm_res_to_ns(res), &res->lr_name,
- NULL, LCK_MINMODE,
- lc->lc_flags, lc->lc_opaque);
- if (rc != 0) {
- CERROR("ldlm_cli_cancel_unused ("LPU64"): %d\n",
- res->lr_name.name[0], rc);
- }
+ ldlm_cli_cancel_unused_resource(ldlm_res_to_ns(res), &res->lr_name,
+ NULL, LCK_MINMODE,
+ lc->lc_flags, lc->lc_opaque);
/* must return 0 for hash iteration */
return 0;
}
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
index 77e022b..2fdd938 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
@@ -762,16 +762,9 @@ static int ldlm_resource_complain(struct cfs_hash *hs, struct cfs_hash_bd *bd,
struct ldlm_resource *res = cfs_hash_object(hs, hnode);
lock_res(res);
- CERROR("Namespace %s resource refcount nonzero "
- "(%d) after lock cleanup; forcing "
- "cleanup.\n",
- ldlm_ns_name(ldlm_res_to_ns(res)),
- atomic_read(&res->lr_refcount) - 1);
-
- CERROR("Resource: %p ("LPU64"/"LPU64"/"LPU64"/"
- LPU64") (rc: %d)\n", res,
- res->lr_name.name[0], res->lr_name.name[1],
- res->lr_name.name[2], res->lr_name.name[3],
+ CERROR("%s: namespace resource "DLDLMRES" (%p) refcount nonzero "
+ "(%d) after lock cleanup; forcing cleanup.\n",
+ ldlm_ns_name(ldlm_res_to_ns(res)), PLDLMRES(res), res,
atomic_read(&res->lr_refcount) - 1);
ldlm_resource_dump(D_ERROR, res);
@@ -1403,10 +1396,8 @@ void ldlm_resource_dump(int level, struct ldlm_resource *res)
if (!((libcfs_debug | D_ERROR) & level))
return;
- CDEBUG(level, "--- Resource: %p ("LPU64"/"LPU64"/"LPU64"/"LPU64
- ") (rc: %d)\n", res, res->lr_name.name[0], res->lr_name.name[1],
- res->lr_name.name[2], res->lr_name.name[3],
- atomic_read(&res->lr_refcount));
+ CDEBUG(level, "--- Resource: "DLDLMRES" (%p) refcount = %d\n",
+ PLDLMRES(res), res, atomic_read(&res->lr_refcount));
if (!list_empty(&res->lr_granted)) {
CDEBUG(level, "Granted locks (in reverse order):\n");
diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 34815b5..8377468 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -233,12 +233,9 @@ int ll_md_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc,
ll_have_md_lock(inode, &bits, mode);
fid = ll_inode2fid(inode);
- if (lock->l_resource->lr_name.name[0] != fid_seq(fid) ||
- lock->l_resource->lr_name.name[1] != fid_oid(fid) ||
- lock->l_resource->lr_name.name[2] != fid_ver(fid)) {
+ if (!fid_res_name_eq(fid, &lock->l_resource->lr_name))
LDLM_ERROR(lock, "data mismatch with object "
DFID" (%p)", PFID(fid), inode);
- }
if (bits & MDS_INODELOCK_OPEN) {
int flags = 0;
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_locks.c b/drivers/staging/lustre/lustre/mdc/mdc_locks.c
index eccbab7..09dee11 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_locks.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_locks.c
@@ -971,13 +971,8 @@ static int mdc_finish_intent_lock(struct obd_export *exp,
LASSERTF(fid_res_name_eq(&mdt_body->fid1,
&lock->l_resource->lr_name),
- "Lock res_id: %lu/%lu/%lu, fid: %lu/%lu/%lu.\n",
- (unsigned long)lock->l_resource->lr_name.name[0],
- (unsigned long)lock->l_resource->lr_name.name[1],
- (unsigned long)lock->l_resource->lr_name.name[2],
- (unsigned long)fid_seq(&mdt_body->fid1),
- (unsigned long)fid_oid(&mdt_body->fid1),
- (unsigned long)fid_ver(&mdt_body->fid1));
+ "Lock res_id: "DLDLMRES", fid: "DFID"\n",
+ PLDLMRES(lock->l_resource), PFID(&mdt_body->fid1));
LDLM_LOCK_PUT(lock);
memcpy(&old_lock, lockh, sizeof(*lockh));
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 12a9ede..93b601d 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -788,8 +788,8 @@ static int mgc_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc,
/* We've given up the lock, prepare ourselves to update. */
LDLM_DEBUG(lock, "MGC cancel CB");
- CDEBUG(D_MGC, "Lock res "LPX64" (%.8s)\n",
- lock->l_resource->lr_name.name[0],
+ CDEBUG(D_MGC, "Lock res "DLDLMRES" (%.8s)\n",
+ PLDLMRES(lock->l_resource),
(char *)&lock->l_resource->lr_name.name[0]);
if (!cld) {
--
1.7.9.5
next prev parent reply other threads:[~2013-11-19 13:26 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
2013-11-19 13:23 ` [PATCH 1/9] staging/lustre/llite: Access to released file trigs a restore Peng Tao
2013-11-19 18:29 ` Greg Kroah-Hartman
2013-11-20 7:35 ` Peng Tao
2013-11-20 9:40 ` Peng Tao
2013-11-19 13:23 ` [PATCH 2/9] staging/lustre/hsm: Implementation of exclusive open Peng Tao
2013-11-19 13:23 ` [PATCH 3/9] drivers/staging/lustre: indent lustre_ldlm_flags_vals Peng Tao
2013-11-19 13:23 ` [PATCH 4/9] staging/lustre/lnet: Fix assert on empty group in selftest module Peng Tao
2013-11-19 18:37 ` Greg Kroah-Hartman
2013-11-20 9:26 ` Peng Tao
2013-11-20 16:27 ` Greg Kroah-Hartman
2013-11-20 17:34 ` Peng Tao
2013-11-19 13:23 ` [PATCH 5/9] staging/lustre/lnet: coding style fix for lst_test_add_ioctl Peng Tao
2013-11-19 13:23 ` [PATCH 6/9] staging/lustre/lnet: remove extra space in lstcon_rpc_trans_abort Peng Tao
2013-11-19 13:23 ` [PATCH 7/9] staging/lustre/lnet: constify name argument of lstcon_group_find/lstcon_batch_find Peng Tao
2013-11-19 13:23 ` [PATCH 8/9] staging/lustre/lnet: coding style fix for lstcon_test_add Peng Tao
2013-11-19 13:23 ` Peng Tao [this message]
2013-11-19 18:40 ` [PATCH 9/9] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES Greg Kroah-Hartman
2013-11-19 18:41 ` [PATCH 0/9] staging/lustre: sync with exernal tree Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1384867428-8395-10-git-send-email-bergwolf@gmail.com \
--to=bergwolf@gmail.com \
--cc=andreas.dilger@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=lai.siyao@intel.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).