* [PATCH-v2 1/3] staging/lustre/llite: Access to released file triggers a restore
2013-11-21 14:24 [PATCH-v2 0/3] staging/lustre: sync with external tree Peng Tao
@ 2013-11-21 14:24 ` Peng Tao
2013-11-21 14:24 ` [PATCH-v2 2/3] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES Peng Tao
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Peng Tao @ 2013-11-21 14:24 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, JC Lafoucriere, Peng Tao, Andreas Dilger
From: JC Lafoucriere <jacques-charles.lafoucriere@cea.fr>
When a client accesses data in a released file,
or truncate it, client must trig a restore request.
During this restore, the client must not glimpse and
must use size from MDT. To bring the "restore is running"
information on the client we add a new t_state bit field
to mdt_info which will be used to carry transient file state.
To memorise this information in the inode we add a new flag
LLIF_FILE_RESTORING.
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3432
Lustre-change: http://review.whamcloud.com/6537
Signed-off-by: JC Lafoucriere <jacques-charles.lafoucriere@cea.fr>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
Tested-by: Oleg Drokin <oleg.drokin@intel.com>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
---
drivers/staging/lustre/lustre/include/cl_object.h | 6 ++-
.../lustre/lustre/include/lustre/lustre_idl.h | 14 +++--
drivers/staging/lustre/lustre/lclient/lcommon_cl.c | 6 +++
drivers/staging/lustre/lustre/llite/file.c | 39 +++++++++++++-
.../staging/lustre/lustre/llite/llite_internal.h | 3 ++
drivers/staging/lustre/lustre/llite/llite_lib.c | 36 +++++++++++++
drivers/staging/lustre/lustre/llite/vvp_io.c | 54 ++++++++++++++++++--
drivers/staging/lustre/lustre/lov/lov_io.c | 15 ++++--
.../staging/lustre/lustre/ptlrpc/pack_generic.c | 2 +-
drivers/staging/lustre/lustre/ptlrpc/wiretest.c | 18 ++++---
10 files changed, 169 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/lustre/lustre/include/cl_object.h b/drivers/staging/lustre/lustre/include/cl_object.h
index c485206..4d692dc 100644
--- a/drivers/staging/lustre/lustre/include/cl_object.h
+++ b/drivers/staging/lustre/lustre/include/cl_object.h
@@ -2388,7 +2388,11 @@ struct cl_io {
* Right now, only two opertaions need to verify layout: glimpse
* and setattr.
*/
- ci_verify_layout:1;
+ ci_verify_layout:1,
+ /**
+ * file is released, restore has to to be triggered by vvp layer
+ */
+ ci_restore_needed:1;
/**
* Number of pages owned by this IO. For invariant checking.
*/
diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h b/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h
index e592a0e..4d8d8c3 100644
--- a/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h
+++ b/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h
@@ -1725,10 +1725,7 @@ static inline __u32 lov_mds_md_size(__u16 stripes, __u32 lmm_magic)
#define OBD_MD_MDS (0x0000000100000000ULL) /* where an inode lives on */
#define OBD_MD_REINT (0x0000000200000000ULL) /* reintegrate oa */
#define OBD_MD_MEA (0x0000000400000000ULL) /* CMD split EA */
-
-/* OBD_MD_MDTIDX is used to get MDT index, but it is never been used overwire,
- * and it is already obsolete since 2.3 */
-/* #define OBD_MD_MDTIDX (0x0000000800000000ULL) */
+#define OBD_MD_TSTATE (0x0000000800000000ULL) /* transient state field */
#define OBD_MD_FLXATTR (0x0000001000000000ULL) /* xattr */
#define OBD_MD_FLXATTRLS (0x0000002000000000ULL) /* xattr list */
@@ -2208,6 +2205,11 @@ static inline int ll_inode_to_ext_flags(int iflags)
((iflags & S_IMMUTABLE) ? LUSTRE_IMMUTABLE_FL : 0));
}
+/* 64 possible states */
+enum md_transient_state {
+ MS_RESTORE = (1 << 0), /* restore is running */
+};
+
struct mdt_body {
struct lu_fid fid1;
struct lu_fid fid2;
@@ -2219,7 +2221,9 @@ struct mdt_body {
obd_time ctime;
__u64 blocks; /* XID, in the case of MDS_READPAGE */
__u64 ioepoch;
- __u64 unused1; /* was "ino" until 2.4.0 */
+ __u64 t_state; /* transient file state defined in
+ * enum md_transient_state
+ * was "ino" until 2.4.0 */
__u32 fsuid;
__u32 fsgid;
__u32 capability;
diff --git a/drivers/staging/lustre/lustre/lclient/lcommon_cl.c b/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
index e60c04d..1c628e3 100644
--- a/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
+++ b/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
@@ -1006,6 +1006,12 @@ again:
cl_io_fini(env, io);
if (unlikely(io->ci_need_restart))
goto again;
+ /* HSM import case: file is released, cannot be restored
+ * no need to fail except if restore registration failed
+ * with -ENODATA */
+ if (result == -ENODATA && io->ci_restore_needed &&
+ io->ci_result != -ENODATA)
+ result = 0;
cl_env_put(env, &refcheck);
return result;
}
diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 971409e..82248e9 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -1107,7 +1107,7 @@ out:
cl_io_fini(env, io);
/* If any bit been read/written (result != 0), we just return
* short read/write instead of restart io. */
- if (result == 0 && io->ci_need_restart) {
+ if ((result == 0 || result == -ENODATA) && io->ci_need_restart) {
CDEBUG(D_VFSTRACE, "Restart %s on %s from %lld, count:%zd\n",
iot == CIT_READ ? "read" : "write",
file->f_dentry->d_name.name, *ppos, count);
@@ -2867,7 +2867,15 @@ int ll_inode_revalidate_it(struct dentry *dentry, struct lookup_intent *it,
LTIME_S(inode->i_mtime) = ll_i2info(inode)->lli_lvb.lvb_mtime;
LTIME_S(inode->i_ctime) = ll_i2info(inode)->lli_lvb.lvb_ctime;
} else {
- rc = ll_glimpse_size(inode);
+ /* In case of restore, the MDT has the right size and has
+ * already send it back without granting the layout lock,
+ * inode is up-to-date so glimpse is useless.
+ * Also to glimpse we need the layout, in case of a running
+ * restore the MDT holds the layout lock so the glimpse will
+ * block up to the end of restore (getattr will block)
+ */
+ if (!(ll_i2info(inode)->lli_flags & LLIF_FILE_RESTORING))
+ rc = ll_glimpse_size(inode);
}
return rc;
}
@@ -3464,3 +3472,30 @@ again:
return rc;
}
+
+/**
+ * This function send a restore request to the MDT
+ */
+int ll_layout_restore(struct inode *inode)
+{
+ struct hsm_user_request *hur;
+ int len, rc;
+
+ len = sizeof(struct hsm_user_request) +
+ sizeof(struct hsm_user_item);
+ OBD_ALLOC(hur, len);
+ if (hur == NULL)
+ return -ENOMEM;
+
+ hur->hur_request.hr_action = HUA_RESTORE;
+ hur->hur_request.hr_archive_id = 0;
+ hur->hur_request.hr_flags = 0;
+ memcpy(&hur->hur_user_item[0].hui_fid, &ll_i2info(inode)->lli_fid,
+ sizeof(hur->hur_user_item[0].hui_fid));
+ hur->hur_user_item[0].hui_extent.length = -1;
+ hur->hur_request.hr_itemcount = 1;
+ rc = obd_iocontrol(LL_IOC_HSM_REQUEST, cl_i2sbi(inode)->ll_md_exp,
+ len, hur, NULL);
+ OBD_FREE(hur, len);
+ return rc;
+}
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 1a13330..c326ff2 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -125,6 +125,8 @@ enum lli_flags {
LLIF_SRVLOCK = (1 << 5),
/* File data is modified. */
LLIF_DATA_MODIFIED = (1 << 6),
+ /* File is being restored */
+ LLIF_FILE_RESTORING = (1 << 7),
};
struct ll_inode_info {
@@ -1588,5 +1590,6 @@ enum {
int ll_layout_conf(struct inode *inode, const struct cl_object_conf *conf);
int ll_layout_refresh(struct inode *inode, __u32 *gen);
+int ll_layout_restore(struct inode *inode);
#endif /* LLITE_INTERNAL_H */
diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index fd584ff..facc391 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -1353,6 +1353,7 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr)
struct ll_inode_info *lli = ll_i2info(inode);
struct md_op_data *op_data = NULL;
struct md_open_data *mod = NULL;
+ bool file_is_released = false;
int rc = 0, rc1 = 0;
CDEBUG(D_VFSTRACE, "%s: setattr inode %p/fid:"DFID" from %llu to %llu, "
@@ -1436,10 +1437,40 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr)
(attr->ia_valid & (ATTR_SIZE | ATTR_MTIME | ATTR_MTIME_SET)))
op_data->op_flags = MF_EPOCH_OPEN;
+ /* truncate on a released file must failed with -ENODATA,
+ * so size must not be set on MDS for released file
+ * but other attributes must be set
+ */
+ if (S_ISREG(inode->i_mode)) {
+ struct lov_stripe_md *lsm;
+ __u32 gen;
+
+ ll_layout_refresh(inode, &gen);
+ lsm = ccc_inode_lsm_get(inode);
+ if (lsm && lsm->lsm_pattern & LOV_PATTERN_F_RELEASED)
+ file_is_released = true;
+ ccc_inode_lsm_put(inode, lsm);
+ }
+
+ /* clear size attr for released file
+ * we clear the attribute send to MDT in op_data, not the original
+ * received from caller in attr which is used later to
+ * decide return code */
+ if (file_is_released && (attr->ia_valid & ATTR_SIZE))
+ op_data->op_attr.ia_valid &= ~ATTR_SIZE;
+
rc = ll_md_setattr(dentry, op_data, &mod);
if (rc)
GOTO(out, rc);
+ /* truncate failed, others succeed */
+ if (file_is_released) {
+ if (attr->ia_valid & ATTR_SIZE)
+ GOTO(out, rc = -ENODATA);
+ else
+ GOTO(out, rc = 0);
+ }
+
/* RPC to MDT is sent, cancel data modification flag */
if (rc == 0 && (op_data->op_bias & MDS_DATA_MODIFIED)) {
spin_lock(&lli->lli_lock);
@@ -1761,6 +1792,11 @@ void ll_update_inode(struct inode *inode, struct lustre_md *md)
LASSERT(md->oss_capa);
ll_add_capa(inode, md->oss_capa);
}
+
+ if (body->valid & OBD_MD_TSTATE) {
+ if (body->t_state & MS_RESTORE)
+ lli->lli_flags |= LLIF_FILE_RESTORING;
+ }
}
void ll_read_inode2(struct inode *inode, void *opaque)
diff --git a/drivers/staging/lustre/lustre/llite/vvp_io.c b/drivers/staging/lustre/lustre/llite/vvp_io.c
index 3ff664c..f69e3aa 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_io.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_io.c
@@ -121,8 +121,38 @@ static void vvp_io_fini(const struct lu_env *env, const struct cl_io_slice *ios)
CLOBINVRNT(env, obj, ccc_object_invariant(obj));
- CDEBUG(D_VFSTRACE, "ignore/verify layout %d/%d, layout version %d.\n",
- io->ci_ignore_layout, io->ci_verify_layout, cio->cui_layout_gen);
+ CDEBUG(D_VFSTRACE, DFID
+ " ignore/verify layout %d/%d, layout version %d restore needed %d\n",
+ PFID(lu_object_fid(&obj->co_lu)),
+ io->ci_ignore_layout, io->ci_verify_layout,
+ cio->cui_layout_gen, io->ci_restore_needed);
+
+ if (io->ci_restore_needed == 1) {
+ int rc;
+
+ /* file was detected release, we need to restore it
+ * before finishing the io
+ */
+ rc = ll_layout_restore(ccc_object_inode(obj));
+ /* if restore registration failed, no restart,
+ * we will return -ENODATA */
+ /* The layout will change after restore, so we need to
+ * block on layout lock hold by the MDT
+ * as MDT will not send new layout in lvb (see LU-3124)
+ * we have to explicitly fetch it, all this will be done
+ * by ll_layout_refresh()
+ */
+ if (rc == 0) {
+ io->ci_restore_needed = 0;
+ io->ci_need_restart = 1;
+ io->ci_verify_layout = 1;
+ } else {
+ io->ci_restore_needed = 1;
+ io->ci_need_restart = 0;
+ io->ci_verify_layout = 0;
+ io->ci_result = rc;
+ }
+ }
if (!io->ci_ignore_layout && io->ci_verify_layout) {
__u32 gen = 0;
@@ -130,9 +160,17 @@ static void vvp_io_fini(const struct lu_env *env, const struct cl_io_slice *ios)
/* check layout version */
ll_layout_refresh(ccc_object_inode(obj), &gen);
io->ci_need_restart = cio->cui_layout_gen != gen;
- if (io->ci_need_restart)
- CDEBUG(D_VFSTRACE, "layout changed from %d to %d.\n",
- cio->cui_layout_gen, gen);
+ if (io->ci_need_restart) {
+ CDEBUG(D_VFSTRACE,
+ DFID" layout changed from %d to %d.\n",
+ PFID(lu_object_fid(&obj->co_lu)),
+ cio->cui_layout_gen, gen);
+ /* today successful restore is the only possible
+ * case */
+ /* restore was done, clear restoring state */
+ ll_i2info(ccc_object_inode(obj))->lli_flags &=
+ ~LLIF_FILE_RESTORING;
+ }
}
}
@@ -1111,6 +1149,12 @@ int vvp_io_init(const struct lu_env *env, struct cl_object *obj,
CLOBINVRNT(env, obj, ccc_object_invariant(obj));
+ CDEBUG(D_VFSTRACE, DFID
+ " ignore/verify layout %d/%d, layout version %d restore needed %d\n",
+ PFID(lu_object_fid(&obj->co_lu)),
+ io->ci_ignore_layout, io->ci_verify_layout,
+ cio->cui_layout_gen, io->ci_restore_needed);
+
CL_IO_SLICE_CLEAN(cio, cui_cl);
cl_io_slice_add(io, &cio->cui_cl, obj, &vvp_io_ops);
vio->cui_ra_window_set = 0;
diff --git a/drivers/staging/lustre/lustre/lov/lov_io.c b/drivers/staging/lustre/lustre/lov/lov_io.c
index 2792fa5..5a6ab70 100644
--- a/drivers/staging/lustre/lustre/lov/lov_io.c
+++ b/drivers/staging/lustre/lustre/lov/lov_io.c
@@ -947,14 +947,23 @@ int lov_io_init_released(const struct lu_env *env, struct cl_object *obj,
LASSERTF(0, "invalid type %d\n", io->ci_type);
case CIT_MISC:
case CIT_FSYNC:
- result = +1;
+ result = 1;
break;
case CIT_SETATTR:
+ /* the truncate to 0 is managed by MDT:
+ * - in open, for open O_TRUNC
+ * - in setattr, for truncate
+ */
+ /* the truncate is for size > 0 so triggers a restore */
+ if (cl_io_is_trunc(io))
+ io->ci_restore_needed = 1;
+ result = -ENODATA;
+ break;
case CIT_READ:
case CIT_WRITE:
case CIT_FAULT:
- /* TODO: need to restore the file. */
- result = -EBADF;
+ io->ci_restore_needed = 1;
+ result = -ENODATA;
break;
}
if (result == 0) {
diff --git a/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c b/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
index 4659314..d831bd7 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
@@ -1873,7 +1873,7 @@ void lustre_swab_mdt_body(struct mdt_body *b)
__swab64s(&b->ctime);
__swab64s(&b->blocks);
__swab64s(&b->ioepoch);
- CLASSERT(offsetof(typeof(*b), unused1) != 0);
+ __swab64s(&b->t_state);
__swab32s(&b->fsuid);
__swab32s(&b->fsgid);
__swab32s(&b->capability);
diff --git a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
index 9890bd9..e3f02c7 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
@@ -49,9 +49,10 @@ void lustre_assert_wire_constants(void)
{
/* Wire protocol assertions generated by 'wirecheck'
* (make -C lustre/utils newwiretest)
- * running on Linux deva 2.6.32.279.lustre #5 SMP Tue Apr 9 22:52:17 CST 2013 x86_64 x86_64 x
- * with gcc version 4.4.4 20100726 (Red Hat 4.4.4-13) (GCC) */
-
+ * running on Linux centos6-bis 2.6.32-358.0.1.el6-head
+ * #3 SMP Wed Apr 17 17:37:43 CEST 2013
+ * with gcc version 4.4.6 20110731 (Red Hat 4.4.6-3) (GCC)
+ */
/* Constants... */
LASSERTF(PTL_RPC_MSG_REQUEST == 4711, "found %lld\n",
@@ -1335,6 +1336,8 @@ void lustre_assert_wire_constants(void)
OBD_MD_REINT);
LASSERTF(OBD_MD_MEA == (0x0000000400000000ULL), "found 0x%.16llxULL\n",
OBD_MD_MEA);
+ LASSERTF(OBD_MD_TSTATE == (0x0000000800000000ULL),
+ "found 0x%.16llxULL\n", OBD_MD_TSTATE);
LASSERTF(OBD_MD_FLXATTR == (0x0000001000000000ULL), "found 0x%.16llxULL\n",
OBD_MD_FLXATTR);
LASSERTF(OBD_MD_FLXATTRLS == (0x0000002000000000ULL), "found 0x%.16llxULL\n",
@@ -1918,10 +1921,11 @@ void lustre_assert_wire_constants(void)
(long long)(int)offsetof(struct mdt_body, blocks));
LASSERTF((int)sizeof(((struct mdt_body *)0)->blocks) == 8, "found %lld\n",
(long long)(int)sizeof(((struct mdt_body *)0)->blocks));
- LASSERTF((int)offsetof(struct mdt_body, unused1) == 96, "found %lld\n",
- (long long)(int)offsetof(struct mdt_body, unused1));
- LASSERTF((int)sizeof(((struct mdt_body *)0)->unused1) == 8, "found %lld\n",
- (long long)(int)sizeof(((struct mdt_body *)0)->unused1));
+ LASSERTF((int)offsetof(struct mdt_body, t_state) == 96, "found %lld\n",
+ (long long)(int)offsetof(struct mdt_body, t_state));
+ LASSERTF((int)sizeof(((struct mdt_body *)0)->t_state) == 8,
+ "found %lld\n",
+ (long long)(int)sizeof(((struct mdt_body *)0)->t_state));
LASSERTF((int)offsetof(struct mdt_body, fsuid) == 104, "found %lld\n",
(long long)(int)offsetof(struct mdt_body, fsuid));
LASSERTF((int)sizeof(((struct mdt_body *)0)->fsuid) == 4, "found %lld\n",
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH-v2 2/3] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES
2013-11-21 14:24 [PATCH-v2 0/3] staging/lustre: sync with external tree Peng Tao
2013-11-21 14:24 ` [PATCH-v2 1/3] staging/lustre/llite: Access to released file triggers a restore Peng Tao
@ 2013-11-21 14:24 ` Peng Tao
2013-11-21 14:24 ` [PATCH-v2 3/3] staging/lustre/lnet: Fix assert on empty group in selftest module Peng Tao
2013-11-21 15:43 ` [PATCH-v2 0/3] staging/lustre: sync with external tree Greg Kroah-Hartman
3 siblings, 0 replies; 5+ messages in thread
From: Peng Tao @ 2013-11-21 14:24 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, Andreas Dilger, Lai Siyao, Peng Tao
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..25e14e1 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH-v2 3/3] staging/lustre/lnet: Fix assert on empty group in selftest module
2013-11-21 14:24 [PATCH-v2 0/3] staging/lustre: sync with external tree Peng Tao
2013-11-21 14:24 ` [PATCH-v2 1/3] staging/lustre/llite: Access to released file triggers a restore Peng Tao
2013-11-21 14:24 ` [PATCH-v2 2/3] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES Peng Tao
@ 2013-11-21 14:24 ` Peng Tao
2013-11-21 15:43 ` [PATCH-v2 0/3] staging/lustre: sync with external tree Greg Kroah-Hartman
3 siblings, 0 replies; 5+ messages in thread
From: Peng Tao @ 2013-11-21 14:24 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, Amir Shehata, Peng Tao, Andreas Dilger
From: Amir Shehata <amir.shehata@intel.com>
The core of the issue is that the selftest module doesn't sanitize its
own API, but it depends on lst utility to do such checks. As a result
this issue manifests itself in this particular LU through an assert
on an empty group. If the NID is misspelled then an empty group is
added. An error output is provided, but if that's never checked in a
batch script, as is the case with this issue, then the script will try
to add an empty group to a test to run in a batch, and that will cause
an assert
The fix is two fold. Ensure that lst utility checks that a group is
added with at least one node. If not the group is subsequently
deleted. And the add_test command would fail, since the group no
longer exists.
The second fix is to ensure that the kernel module itself sanitizes
its own API in this particular case, so that if a different utility is
used other than lst to communicate with the selftest kernel module
then this error would be caught. This fix looks up the batch and the
groups, src and dst, in the ioctl handle and sanitizes that input at
this point. If the group looked up either doesn't exist or doesn't
have at least one ACTIVE node, then the command fails.
NOTE:there are many other cases in the code where the selftest kernel
module doesn't check for sanity of the input, but depends totally on
the lst module to do such checks. Particularly around length of
strings passed in. Thus it is possible to crash the selftest module
if someone tries to create another userspace app to communicate with
the selftest kernel module without ensuring sanity of the params sent
to the kernel module. In effect, it's always assumed that lst is the
front end for selftest and no other front end is to be used.
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3093
Lustre-change: http://review.whamcloud.com/6092
Signed-off-by: Amir Shehata <amir.shehata@intel.com>
Reviewed-by: Isaac Huang <he.huang@intel.com>
Reviewed-by: Liang Zhen <liang.zhen@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
[coding style fix of the original patch is splitted into a new one -- PengTao]
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
---
drivers/staging/lustre/lnet/selftest/console.c | 79 +++++++++++++++++-------
drivers/staging/lustre/lnet/selftest/console.h | 6 +-
2 files changed, 61 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/lustre/lnet/selftest/console.c b/drivers/staging/lustre/lnet/selftest/console.c
index 067a8b5..9556bc0 100644
--- a/drivers/staging/lustre/lnet/selftest/console.c
+++ b/drivers/staging/lustre/lnet/selftest/console.c
@@ -1237,41 +1237,77 @@ again:
goto again;
}
-int
-lstcon_test_add(char *name, int type, int loop, int concur,
- int dist, int span, char *src_name, char * dst_name,
- void *param, int paramlen, int *retp,
- struct list_head *result_up)
+static int
+lstcon_verify_batch(const char *name, lstcon_batch_t **batch)
{
- lstcon_group_t *src_grp = NULL;
- lstcon_group_t *dst_grp = NULL;
- lstcon_test_t *test = NULL;
- lstcon_batch_t *batch;
- int rc;
+ int rc;
- rc = lstcon_batch_find(name, &batch);
+ rc = lstcon_batch_find(name, batch);
if (rc != 0) {
CDEBUG(D_NET, "Can't find batch %s\n", name);
return rc;
}
- if (batch->bat_state != LST_BATCH_IDLE) {
+ if ((*batch)->bat_state != LST_BATCH_IDLE) {
CDEBUG(D_NET, "Can't change running batch %s\n", name);
- return rc;
+ return -EINVAL;
}
- rc = lstcon_group_find(src_name, &src_grp);
+ return 0;
+}
+
+static int
+lstcon_verify_group(const char *name, lstcon_group_t **grp)
+{
+ int rc;
+ lstcon_ndlink_t *ndl;
+
+ rc = lstcon_group_find(name, grp);
if (rc != 0) {
- CDEBUG(D_NET, "Can't find group %s\n", src_name);
- goto out;
+ CDEBUG(D_NET, "can't find group %s\n", name);
+ return rc;
}
- rc = lstcon_group_find(dst_name, &dst_grp);
- if (rc != 0) {
- CDEBUG(D_NET, "Can't find group %s\n", dst_name);
- goto out;
+ list_for_each_entry(ndl, &(*grp)->grp_ndl_list, ndl_link) {
+ if (ndl->ndl_node->nd_state == LST_NODE_ACTIVE)
+ return 0;
}
+ CDEBUG(D_NET, "Group %s has no ACTIVE nodes\n", name);
+
+ return -EINVAL;
+}
+
+int
+lstcon_test_add(char *batch_name, int type, int loop,
+ int concur, int dist, int span,
+ char *src_name, char *dst_name,
+ void *param, int paramlen, int *retp,
+ struct list_head *result_up)
+{
+ lstcon_test_t *test = NULL;
+ int rc;
+ lstcon_group_t *src_grp = NULL;
+ lstcon_group_t *dst_grp = NULL;
+ lstcon_batch_t *batch = NULL;
+
+ /*
+ * verify that a batch of the given name exists, and the groups
+ * that will be part of the batch exist and have at least one
+ * active node
+ */
+ rc = lstcon_verify_batch(batch_name, &batch);
+ if (rc != 0)
+ goto out;
+
+ rc = lstcon_verify_group(src_name, &src_grp);
+ if (rc != 0)
+ goto out;
+
+ rc = lstcon_verify_group(dst_name, &dst_grp);
+ if (rc != 0)
+ goto out;
+
if (dst_grp->grp_userland)
*retp = 1;
@@ -1310,7 +1346,8 @@ lstcon_test_add(char *name, int type, int loop, int concur,
if (lstcon_trans_stat()->trs_rpc_errno != 0 ||
lstcon_trans_stat()->trs_fwk_errno != 0)
- CDEBUG(D_NET, "Failed to add test %d to batch %s\n", type, name);
+ CDEBUG(D_NET, "Failed to add test %d to batch %s\n", type,
+ batch_name);
/* add to test list anyway, so user can check what's going on */
list_add_tail(&test->tes_link, &batch->bat_test_list);
diff --git a/drivers/staging/lustre/lnet/selftest/console.h b/drivers/staging/lustre/lnet/selftest/console.h
index e61b266..b57dbd8 100644
--- a/drivers/staging/lustre/lnet/selftest/console.h
+++ b/drivers/staging/lustre/lnet/selftest/console.h
@@ -224,9 +224,9 @@ extern int lstcon_group_stat(char *grp_name, int timeout,
struct list_head *result_up);
extern int lstcon_nodes_stat(int count, lnet_process_id_t *ids_up,
int timeout, struct list_head *result_up);
-extern int lstcon_test_add(char *name, int type, int loop, int concur,
- int dist, int span, char *src_name, char * dst_name,
+extern int lstcon_test_add(char *batch_name, int type, int loop,
+ int concur, int dist, int span,
+ char *src_name, char *dst_name,
void *param, int paramlen, int *retp,
struct list_head *result_up);
-
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH-v2 0/3] staging/lustre: sync with external tree
2013-11-21 14:24 [PATCH-v2 0/3] staging/lustre: sync with external tree Peng Tao
` (2 preceding siblings ...)
2013-11-21 14:24 ` [PATCH-v2 3/3] staging/lustre/lnet: Fix assert on empty group in selftest module Peng Tao
@ 2013-11-21 15:43 ` Greg Kroah-Hartman
3 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-21 15:43 UTC (permalink / raw)
To: Peng Tao; +Cc: linux-kernel, Andreas Dilger
On Thu, Nov 21, 2013 at 10:24:47PM +0800, Peng Tao wrote:
> Hi Greg,
>
> The first two patches were updated to fix checkpatch warnings.
> The third patch is unchanged as I cannot reproduce the build warning even
> with the specific gcc version. I am resending it in the hope that you can
> point out what warning it was if it is stil there.
Odd, I don't see the warning this time anymore, I don't know what was
up, perhaps an interaction from not having the previous 2 patches
applied?
Anyway, all three now applied, thanks.
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread