All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] staging/lustre: sync with exernal tree
@ 2013-11-19 13:23 Peng Tao
  2013-11-19 13:23 ` [PATCH 1/9] staging/lustre/llite: Access to released file trigs a restore Peng Tao
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Peng Tao, Andreas Dilger

Hi Greg,

Following patches were ported from Lustre external tree.

PATCH 4/9 was splitted into five patches to do separate things that
were done in the original single patch.

PATCh 3/9 is a cleanup patch.

Thanks,
Tao

Cc: Andreas Dilger <andreas.dilger@intel.com>

Amir Shehata (5):
  staging/lustre/lnet: Fix assert on empty group in selftest module
  staging/lustre/lnet: coding style fix for lst_test_add_ioctl
  staging/lustre/lnet: remove extra space in lstcon_rpc_trans_abort
  staging/lustre/lnet: constify name argument of
    lstcon_group_find/lstcon_batch_find
  staging/lustre/lnet: coding style fix for lstcon_test_add

Andreas Dilger (1):
  staging/lustre/ldlm: fix resource/fid check, use DLDLMRES

JC Lafoucriere (1):
  staging/lustre/llite: Access to released file trigs a restore

Jinshan Xiong (1):
  staging/lustre/hsm: Implementation of exclusive open

Peng Tao (1):
  drivers/staging/lustre: indent lustre_ldlm_flags_vals

 drivers/staging/lustre/lnet/selftest/conctl.c      |   56 ++--
 drivers/staging/lustre/lnet/selftest/conrpc.c      |    2 +-
 drivers/staging/lustre/lnet/selftest/console.c     |  103 ++++--
 drivers/staging/lustre/lnet/selftest/console.h     |    6 +-
 drivers/staging/lustre/lustre/include/cl_object.h  |    6 +-
 .../lustre/lustre/include/linux/lustre_intent.h    |    2 +-
 .../lustre/lustre/include/lustre/lustre_idl.h      |   19 +-
 .../lustre/lustre/include/lustre/lustre_user.h     |    3 +
 .../lustre/lustre/include/lustre_dlm_flags.h       |   90 +++---
 drivers/staging/lustre/lustre/include/lustre_lib.h |   11 +-
 drivers/staging/lustre/lustre/lclient/lcommon_cl.c |    6 +
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    5 +
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |   36 +--
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |   19 +-
 drivers/staging/lustre/lustre/llite/file.c         |  336 +++++++++++++++++++-
 .../staging/lustre/lustre/llite/llite_internal.h   |   14 +-
 drivers/staging/lustre/lustre/llite/llite_lib.c    |   36 +++
 drivers/staging/lustre/lustre/llite/namei.c        |    5 +-
 drivers/staging/lustre/lustre/llite/vvp_io.c       |   54 +++-
 drivers/staging/lustre/lustre/lov/lov_io.c         |   15 +-
 drivers/staging/lustre/lustre/mdc/mdc_internal.h   |    2 +-
 drivers/staging/lustre/lustre/mdc/mdc_lib.c        |    7 +-
 drivers/staging/lustre/lustre/mdc/mdc_locks.c      |   49 +--
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |    4 +-
 .../staging/lustre/lustre/ptlrpc/pack_generic.c    |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/wiretest.c    |   17 +-
 26 files changed, 699 insertions(+), 206 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] staging/lustre/llite: Access to released file trigs a restore
  2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
@ 2013-11-19 13:23 ` Peng Tao
  2013-11-19 18:29   ` Greg Kroah-Hartman
  2013-11-19 13:23 ` [PATCH 2/9] staging/lustre/hsm: Implementation of exclusive open Peng Tao
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 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    |   17 +++---
 10 files changed, 168 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 5ca18d0..7d60896 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 */
@@ -2207,6 +2204,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;
@@ -2218,7 +2220,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 17064b4..22f2e73 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -897,7 +897,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);
@@ -2572,7 +2572,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;
 }
@@ -3169,3 +3177,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 d37c183..22fb6ba 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 {
@@ -1579,5 +1581,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..7053cc8 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..6f62277 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,10 @@ 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] 19+ messages in thread

* [PATCH 2/9] staging/lustre/hsm: Implementation of exclusive open
  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 13:23 ` Peng Tao
  2013-11-19 13:23 ` [PATCH 3/9] drivers/staging/lustre: indent lustre_ldlm_flags_vals Peng Tao
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Jinshan Xiong, John L. Hammond, Peng Tao, Andreas Dilger

From: Jinshan Xiong <jinshan.xiong@intel.com>

Proposed way to do exclusive open:

0. First of all, we have to perform most of the work in kernel space;

1. Client exclusively opens the file with IT_RELEASE_OPEN.
1.1 exclusive open means the open will fail if the file is being opened by somebody else;

2. the MDT will handle IT_RELEASE as follows:
2.1 Revoke OPEN_LOCK on this file, just request EX mode of MDS_INODELOCK_OPEN lock and
release it;
2.2 Acquire a rwsem, say open_rwsem, with write mode before trying to the file; for the
normal open, it should acquire read mode of open_rwsem;
2.3 Check if the file is already being opened by others, if not return with -EBUSY;
2.4 Check if the file can be released;
2.5 Set a special flag in struct mdt_file_data to mark this open is exclusive;
2.5 Release open rwsem.

>From now on, if the file is opened by others, it will mark mdt_file_data that the
exclusive open is broken.

3. Client: if IT_RELEASE_OPEN is finished successfully, and do followings:
3.1 Acquire full PW or EX extent lock to flush dirty cache;
3.2 Pack the handle of layout lock, data version and other attars;
3.3 Close the file with IT_RELEASE_CLOSE.

4. Back to MDT to handle IT_RELEASE CLOSE:
4.1 Grab the open_rwsem
4.2 Check if the exclusive open has ever been broken, in that case, the RELEASE process
will fail;
4.3 Verify the data version matches archive;
4.4 Grab EX layout lock
4.5 Swap the layout
4.6 Release EX layout lock
4.7 Close the exclusive open

Basically we avoid granting EX layout lock back to client and introduce exclusive open so
that we know it if the file has ever being accessed. I hope this can simplify things
a little bit. Also, exclusive open can be used to implement file lease.

In this patch, a framework of lease is implemented. However,
only exclusive lease is supported right now.

To apply a lease, MDS_OPEN_LEASE must be set to open the file, EX
mode open lock is returned to the client side to hold a lease. From
that time on, if this file is opened again by other processes, the
open lock will be revoked so the client who holds the lease will
know the lease is already broken by checking that open lock.

To release a lease, normal close is used. The client will revoke the
open lock before sending CLOSE request.

Lease can be applied in two ways. ll_lease_open()/close() can be
called directly if the lease holder is in kernel space; or if the
lease holder lives in user space, it has to open the file first and
then use ioctl() with command LL_IOC_SET_LEASE to apply a lease. The
lease holder has to poll the lease status itself.

Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-2919
Lustre-change: http://review.whamcloud.com/6730
Signed-off-by: Jinshan Xiong <jinshan.xiong@intel.com>
Signed-off-by: John L. Hammond <john.hammond@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
---
 .../lustre/lustre/include/linux/lustre_intent.h    |    2 +-
 .../lustre/lustre/include/lustre/lustre_idl.h      |    5 +
 .../lustre/lustre/include/lustre/lustre_user.h     |    3 +
 .../lustre/lustre/include/lustre_dlm_flags.h       |   12 +-
 drivers/staging/lustre/lustre/include/lustre_lib.h |   11 +-
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    5 +
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |    4 +-
 drivers/staging/lustre/lustre/llite/file.c         |  297 +++++++++++++++++++-
 .../staging/lustre/lustre/llite/llite_internal.h   |   11 +-
 drivers/staging/lustre/lustre/mdc/mdc_internal.h   |    2 +-
 drivers/staging/lustre/lustre/mdc/mdc_lib.c        |    7 +-
 drivers/staging/lustre/lustre/mdc/mdc_locks.c      |   40 ++-
 12 files changed, 370 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/linux/lustre_intent.h b/drivers/staging/lustre/lustre/include/linux/lustre_intent.h
index b10ddfa..c491d52 100644
--- a/drivers/staging/lustre/lustre/include/linux/lustre_intent.h
+++ b/drivers/staging/lustre/lustre/include/linux/lustre_intent.h
@@ -52,8 +52,8 @@ struct lustre_intent_data {
 
 struct lookup_intent {
 	int     it_op;
-	int     it_flags;
 	int     it_create_mode;
+	__u64   it_flags;
 	union {
 		struct lustre_intent_data lustre;
 	} d;
diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h b/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h
index 7d60896..4d8d8c3 100644
--- a/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h
+++ b/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h
@@ -2117,6 +2117,7 @@ extern void lustre_swab_generic_32s (__u32 *val);
 #define DISP_ENQ_OPEN_REF    0x00800000
 #define DISP_ENQ_CREATE_REF  0x01000000
 #define DISP_OPEN_LOCK       0x02000000
+#define DISP_OPEN_LEASE      0x04000000
 
 /* INODE LOCK PARTS */
 #define MDS_INODELOCK_LOOKUP 0x000001       /* dentry, mode, owner, group */
@@ -2377,6 +2378,10 @@ extern void lustre_swab_mdt_rec_setattr (struct mdt_rec_setattr *sa);
 					      * hsm restore) */
 #define MDS_OPEN_VOLATILE   0400000000000ULL /* File is volatile = created
 						unlinked */
+#define MDS_OPEN_LEASE	   01000000000000ULL /* Open the file and grant lease
+					      * delegation, succeed if it's not
+					      * being opened with conflict mode.
+					      */
 
 /* permission for create non-directory file */
 #define MAY_CREATE      (1 << 7)
diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
index c7bd447..9fd1d3b 100644
--- a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
+++ b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
@@ -245,6 +245,9 @@ struct ost_id {
 #define LL_IOC_LMV_GETSTRIPE	    _IOWR('f', 241, struct lmv_user_md)
 #define LL_IOC_REMOVE_ENTRY	    _IOWR('f', 242, __u64)
 
+#define LL_IOC_SET_LEASE		_IOWR('f', 243, long)
+#define LL_IOC_GET_LEASE		_IO('f', 244)
+
 #define LL_STATFS_LMV	   1
 #define LL_STATFS_LOV	   2
 #define LL_STATFS_NODELAY	4
diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h b/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
index 8c34d9d..f8b4ba7 100644
--- a/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
+++ b/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
@@ -35,7 +35,7 @@
 #ifndef LDLM_ALL_FLAGS_MASK
 
 /** l_flags bits marked as "all_flags" bits */
-#define LDLM_FL_ALL_FLAGS_MASK          0x007FFFFFC08F132FULL
+#define LDLM_FL_ALL_FLAGS_MASK          0x00FFFFFFC08F132FULL
 
 /** l_flags bits marked as "ast" bits */
 #define LDLM_FL_AST_MASK                0x0000000080000000ULL
@@ -53,7 +53,7 @@
 #define LDLM_FL_INHERIT_MASK            0x0000000000800000ULL
 
 /** l_flags bits marked as "local_only" bits */
-#define LDLM_FL_LOCAL_ONLY_MASK         0x007FFFFF00000000ULL
+#define LDLM_FL_LOCAL_ONLY_MASK         0x00FFFFFF00000000ULL
 
 /** l_flags bits marked as "on_wire" bits */
 #define LDLM_FL_ON_WIRE_MASK            0x00000000C08F132FULL
@@ -358,6 +358,12 @@
 #define ldlm_set_ns_srv(_l)             LDLM_SET_FLAG((  _l), 1ULL << 54)
 #define ldlm_clear_ns_srv(_l)           LDLM_CLEAR_FLAG((_l), 1ULL << 54)
 
+/** Flag whether this lock can be reused. Used by exclusive open. */
+#define LDLM_FL_EXCL                    0x0080000000000000ULL /* bit  55 */
+#define ldlm_is_excl(_l)                LDLM_TEST_FLAG((_l), 1ULL << 55)
+#define ldlm_set_excl(_l)               LDLM_SET_FLAG((_l), 1ULL << 55)
+#define ldlm_clear_excl(_l)             LDLM_CLEAR_FLAG((_l), 1ULL << 55)
+
 /** test for ldlm_lock flag bit set */
 #define LDLM_TEST_FLAG(_l, _b)        (((_l)->l_flags & (_b)) != 0)
 
@@ -414,6 +420,7 @@ static int hf_lustre_ldlm_fl_server_lock         = -1;
 static int hf_lustre_ldlm_fl_res_locked          = -1;
 static int hf_lustre_ldlm_fl_waited              = -1;
 static int hf_lustre_ldlm_fl_ns_srv              = -1;
+static int hf_lustre_ldlm_fl_excl                = -1;
 
 const value_string lustre_ldlm_flags_vals[] = {
   {LDLM_FL_LOCK_CHANGED,        "LDLM_FL_LOCK_CHANGED"},
@@ -454,6 +461,7 @@ const value_string lustre_ldlm_flags_vals[] = {
   {LDLM_FL_RES_LOCKED,          "LDLM_FL_RES_LOCKED"},
   {LDLM_FL_WAITED,              "LDLM_FL_WAITED"},
   {LDLM_FL_NS_SRV,              "LDLM_FL_NS_SRV"},
+  {LDLM_FL_EXCL,                "LDLM_FL_EXCL"},
   { 0, NULL }
 };
 #endif /*  WIRESHARK_COMPILE */
diff --git a/drivers/staging/lustre/lustre/include/lustre_lib.h b/drivers/staging/lustre/lustre/include/lustre_lib.h
index b6fd03e..609a090 100644
--- a/drivers/staging/lustre/lustre/include/lustre_lib.h
+++ b/drivers/staging/lustre/lustre/include/lustre_lib.h
@@ -81,11 +81,12 @@ struct client_obd *client_conn2cli(struct lustre_handle *conn);
 
 struct md_open_data;
 struct obd_client_handle {
-	struct lustre_handle  och_fh;
-	struct lu_fid	 och_fid;
-	struct md_open_data  *och_mod;
-	__u32 och_magic;
-	fmode_t och_flags;
+	struct lustre_handle	 och_fh;
+	struct lu_fid		 och_fid;
+	struct md_open_data	*och_mod;
+	struct lustre_handle	 och_lease_handle; /* open lock for lease */
+	__u32			 och_magic;
+	fmode_t			 och_flags;
 };
 #define OBD_CLIENT_HANDLE_MAGIC 0xd15ea5ed
 
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 3900a69..ef826e9 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1129,6 +1129,11 @@ static struct ldlm_lock *search_queue(struct list_head *queue,
 		if (lock == old_lock)
 			break;
 
+		/* Check if this lock can be matched.
+		 * Used by LU-2919(exclusive open) for open lease lock */
+		if (ldlm_is_excl(lock))
+			continue;
+
 		/* llite sometimes wants to match locks that will be
 		 * canceled when their users drop, but we allow it to match
 		 * if it passes in CBPENDING and the lock still has users.
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
index dcc2784..1e2c0dd 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
@@ -910,7 +910,7 @@ int ldlm_cli_enqueue(struct obd_export *exp, struct ptlrpc_request **reqp,
 	lock->l_conn_export = exp;
 	lock->l_export = NULL;
 	lock->l_blocking_ast = einfo->ei_cb_bl;
-	lock->l_flags |= (*flags & LDLM_FL_NO_LRU);
+	lock->l_flags |= (*flags & (LDLM_FL_NO_LRU | LDLM_FL_EXCL));
 
 	/* lock not sent to server yet */
 
@@ -1333,7 +1333,7 @@ int ldlm_cli_cancel(struct lustre_handle *lockh,
 	}
 
 	rc = ldlm_cli_cancel_local(lock);
-	if (rc == LDLM_FL_LOCAL_ONLY) {
+	if (rc == LDLM_FL_LOCAL_ONLY || cancel_flags & LCF_LOCAL) {
 		LDLM_LOCK_RELEASE(lock);
 		return 0;
 	}
diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 22f2e73..82248e9 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -241,6 +241,24 @@ int ll_md_close(struct obd_export *md_exp, struct inode *inode,
 	if (unlikely(fd->fd_flags & LL_FILE_GROUP_LOCKED))
 		ll_put_grouplock(inode, file, fd->fd_grouplock.cg_gid);
 
+	if (fd->fd_lease_och != NULL) {
+		bool lease_broken;
+
+		/* Usually the lease is not released when the
+		 * application crashed, we need to release here. */
+		rc = ll_lease_close(fd->fd_lease_och, inode, &lease_broken);
+		CDEBUG(rc ? D_ERROR : D_INODE, "Clean up lease "DFID" %d/%d\n",
+			PFID(&lli->lli_fid), rc, lease_broken);
+
+		fd->fd_lease_och = NULL;
+	}
+
+	if (fd->fd_och != NULL) {
+		rc = ll_close_inode_openhandle(md_exp, inode, fd->fd_och);
+		fd->fd_och = NULL;
+		GOTO(out, rc);
+	}
+
 	/* Let's see if we have good enough OPEN lock on the file and if
 	   we can skip talking to MDS */
 	if (file->f_dentry->d_inode) { /* Can this ever be false? */
@@ -277,6 +295,7 @@ int ll_md_close(struct obd_export *md_exp, struct inode *inode,
 		       file, file->f_dentry, file->f_dentry->d_name.name);
 	}
 
+out:
 	LUSTRE_FPRIVATE(file) = NULL;
 	ll_file_data_put(fd);
 	ll_capa_close(inode);
@@ -440,6 +459,7 @@ static int ll_och_fill(struct obd_export *md_exp, struct lookup_intent *it,
 	body = req_capsule_server_get(&req->rq_pill, &RMF_MDT_BODY);
 	och->och_fh = body->handle;
 	och->och_fid = body->fid1;
+	och->och_lease_handle.cookie = it->d.lustre.it_lock_handle;
 	och->och_magic = OBD_CLIENT_HANDLE_MAGIC;
 	och->och_flags = it->it_flags;
 
@@ -471,7 +491,7 @@ int ll_local_open(struct file *file, struct lookup_intent *it,
 
 	LUSTRE_FPRIVATE(file) = fd;
 	ll_readahead_init(inode, &fd->fd_ras);
-	fd->fd_omode = it->it_flags;
+	fd->fd_omode = it->it_flags & (FMODE_READ | FMODE_WRITE | FMODE_EXEC);
 	return 0;
 }
 
@@ -673,6 +693,196 @@ out_openerr:
 	return rc;
 }
 
+static int ll_md_blocking_lease_ast(struct ldlm_lock *lock,
+			struct ldlm_lock_desc *desc, void *data, int flag)
+{
+	int rc;
+	struct lustre_handle lockh;
+
+	switch (flag) {
+	case LDLM_CB_BLOCKING:
+		ldlm_lock2handle(lock, &lockh);
+		rc = ldlm_cli_cancel(&lockh, LCF_ASYNC);
+		if (rc < 0) {
+			CDEBUG(D_INODE, "ldlm_cli_cancel: %d\n", rc);
+			return rc;
+		}
+		break;
+	case LDLM_CB_CANCELING:
+		/* do nothing */
+		break;
+	}
+	return 0;
+}
+
+/**
+ * Acquire a lease and open the file.
+ */
+struct obd_client_handle *ll_lease_open(struct inode *inode, struct file *file,
+					fmode_t fmode)
+{
+	struct lookup_intent it = { .it_op = IT_OPEN };
+	struct ll_sb_info *sbi = ll_i2sbi(inode);
+	struct md_op_data *op_data;
+	struct ptlrpc_request *req;
+	struct lustre_handle old_handle = { 0 };
+	struct obd_client_handle *och = NULL;
+	int rc;
+	int rc2;
+
+	if (fmode != FMODE_WRITE && fmode != FMODE_READ)
+		return ERR_PTR(-EINVAL);
+
+	if (file != NULL) {
+		struct ll_inode_info *lli = ll_i2info(inode);
+		struct ll_file_data *fd = LUSTRE_FPRIVATE(file);
+		struct obd_client_handle **och_p;
+		__u64 *och_usecount;
+
+		if (!(fmode & file->f_mode) || (file->f_mode & FMODE_EXEC))
+			return ERR_PTR(-EPERM);
+
+		/* Get the openhandle of the file */
+		rc = -EBUSY;
+		mutex_lock(&lli->lli_och_mutex);
+		if (fd->fd_lease_och != NULL) {
+			mutex_unlock(&lli->lli_och_mutex);
+			return ERR_PTR(rc);
+		}
+
+		if (fd->fd_och == NULL) {
+			if (file->f_mode & FMODE_WRITE) {
+				LASSERT(lli->lli_mds_write_och != NULL);
+				och_p = &lli->lli_mds_write_och;
+				och_usecount = &lli->lli_open_fd_write_count;
+			} else {
+				LASSERT(lli->lli_mds_read_och != NULL);
+				och_p = &lli->lli_mds_read_och;
+				och_usecount = &lli->lli_open_fd_read_count;
+			}
+			if (*och_usecount == 1) {
+				fd->fd_och = *och_p;
+				*och_p = NULL;
+				*och_usecount = 0;
+				rc = 0;
+			}
+		}
+		mutex_unlock(&lli->lli_och_mutex);
+		if (rc < 0) /* more than 1 opener */
+			return ERR_PTR(rc);
+
+		LASSERT(fd->fd_och != NULL);
+		old_handle = fd->fd_och->och_fh;
+	}
+
+	OBD_ALLOC_PTR(och);
+	if (och == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	op_data = ll_prep_md_op_data(NULL, inode, inode, NULL, 0, 0,
+					LUSTRE_OPC_ANY, NULL);
+	if (IS_ERR(op_data))
+		GOTO(out, rc = PTR_ERR(op_data));
+
+	/* To tell the MDT this openhandle is from the same owner */
+	op_data->op_handle = old_handle;
+
+	it.it_flags = fmode | MDS_OPEN_LOCK | MDS_OPEN_BY_FID | MDS_OPEN_LEASE;
+	rc = md_intent_lock(sbi->ll_md_exp, op_data, NULL, 0, &it, 0, &req,
+				ll_md_blocking_lease_ast,
+	/* LDLM_FL_NO_LRU: To not put the lease lock into LRU list, otherwise
+	 * it can be cancelled which may mislead applications that the lease is
+	 * broken;
+	 * LDLM_FL_EXCL: Set this flag so that it won't be matched by normal
+	 * open in ll_md_blocking_ast(). Otherwise as ll_md_blocking_lease_ast
+	 * doesn't deal with openhandle, so normal openhandle will be leaked. */
+				LDLM_FL_NO_LRU | LDLM_FL_EXCL);
+	ll_finish_md_op_data(op_data);
+	if (req != NULL) {
+		ptlrpc_req_finished(req);
+		it_clear_disposition(&it, DISP_ENQ_COMPLETE);
+	}
+	if (rc < 0)
+		GOTO(out_release_it, rc);
+
+	if (it_disposition(&it, DISP_LOOKUP_NEG))
+		GOTO(out_release_it, rc = -ENOENT);
+
+	rc = it_open_error(DISP_OPEN_OPEN, &it);
+	if (rc)
+		GOTO(out_release_it, rc);
+
+	LASSERT(it_disposition(&it, DISP_ENQ_OPEN_REF));
+	ll_och_fill(sbi->ll_md_exp, &it, och);
+
+	if (!it_disposition(&it, DISP_OPEN_LEASE)) /* old server? */
+		GOTO(out_close, rc = -EOPNOTSUPP);
+
+	/* already get lease, handle lease lock */
+	ll_set_lock_data(sbi->ll_md_exp, inode, &it, NULL);
+	if (it.d.lustre.it_lock_mode == 0 ||
+	    it.d.lustre.it_lock_bits != MDS_INODELOCK_OPEN) {
+		/* open lock must return for lease */
+		CERROR(DFID "lease granted but no open lock, %d/%llu.\n",
+			PFID(ll_inode2fid(inode)), it.d.lustre.it_lock_mode,
+			it.d.lustre.it_lock_bits);
+		GOTO(out_close, rc = -EPROTO);
+	}
+
+	ll_intent_release(&it);
+	return och;
+
+out_close:
+	rc2 = ll_close_inode_openhandle(sbi->ll_md_exp, inode, och);
+	if (rc2)
+		CERROR("Close openhandle returned %d\n", rc2);
+
+	/* cancel open lock */
+	if (it.d.lustre.it_lock_mode != 0) {
+		ldlm_lock_decref_and_cancel(&och->och_lease_handle,
+						it.d.lustre.it_lock_mode);
+		it.d.lustre.it_lock_mode = 0;
+	}
+out_release_it:
+	ll_intent_release(&it);
+out:
+	OBD_FREE_PTR(och);
+	return ERR_PTR(rc);
+}
+EXPORT_SYMBOL(ll_lease_open);
+
+/**
+ * Release lease and close the file.
+ * It will check if the lease has ever broken.
+ */
+int ll_lease_close(struct obd_client_handle *och, struct inode *inode,
+			bool *lease_broken)
+{
+	struct ldlm_lock *lock;
+	bool cancelled = true;
+	int rc;
+
+	lock = ldlm_handle2lock(&och->och_lease_handle);
+	if (lock != NULL) {
+		lock_res_and_lock(lock);
+		cancelled = ldlm_is_cancel(lock);
+		unlock_res_and_lock(lock);
+		ldlm_lock_put(lock);
+	}
+
+	CDEBUG(D_INODE, "lease for "DFID" broken? %d\n",
+		PFID(&ll_i2info(inode)->lli_fid), cancelled);
+
+	if (!cancelled)
+		ldlm_cli_cancel(&och->och_lease_handle, 0);
+	if (lease_broken != NULL)
+		*lease_broken = cancelled;
+
+	rc = ll_close_inode_openhandle(ll_i2sbi(inode)->ll_md_exp, inode, och);
+	return rc;
+}
+EXPORT_SYMBOL(ll_lease_close);
+
 /* Fills the obdo with the attributes for the lsm */
 static int ll_lsm_getattr(struct lov_stripe_md *lsm, struct obd_export *exp,
 			  struct obd_capa *capa, struct obdo *obdo,
@@ -2066,6 +2276,91 @@ long ll_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		OBD_FREE_PTR(hca);
 		return rc;
 	}
+	case LL_IOC_SET_LEASE: {
+		struct ll_inode_info *lli = ll_i2info(inode);
+		struct obd_client_handle *och = NULL;
+		bool lease_broken;
+		fmode_t mode = 0;
+
+		switch (arg) {
+		case F_WRLCK:
+			if (!(file->f_mode & FMODE_WRITE))
+				return -EPERM;
+			mode = FMODE_WRITE;
+			break;
+		case F_RDLCK:
+			if (!(file->f_mode & FMODE_READ))
+				return -EPERM;
+			mode = FMODE_READ;
+			break;
+		case F_UNLCK:
+			mutex_lock(&lli->lli_och_mutex);
+			if (fd->fd_lease_och != NULL) {
+				och = fd->fd_lease_och;
+				fd->fd_lease_och = NULL;
+			}
+			mutex_unlock(&lli->lli_och_mutex);
+
+			if (och != NULL) {
+				mode = och->och_flags &
+				       (FMODE_READ|FMODE_WRITE);
+				rc = ll_lease_close(och, inode, &lease_broken);
+				if (rc == 0 && lease_broken)
+					mode = 0;
+			} else {
+				rc = -ENOLCK;
+			}
+
+			/* return the type of lease or error */
+			return rc < 0 ? rc : (int)mode;
+		default:
+			return -EINVAL;
+		}
+
+		CDEBUG(D_INODE, "Set lease with mode %d\n", mode);
+
+		/* apply for lease */
+		och = ll_lease_open(inode, file, mode);
+		if (IS_ERR(och))
+			return PTR_ERR(och);
+
+		rc = 0;
+		mutex_lock(&lli->lli_och_mutex);
+		if (fd->fd_lease_och == NULL) {
+			fd->fd_lease_och = och;
+			och = NULL;
+		}
+		mutex_unlock(&lli->lli_och_mutex);
+		if (och != NULL) {
+			/* impossible now that only excl is supported for now */
+			ll_lease_close(och, inode, &lease_broken);
+			rc = -EBUSY;
+		}
+		return rc;
+	}
+	case LL_IOC_GET_LEASE: {
+		struct ll_inode_info *lli = ll_i2info(inode);
+		struct ldlm_lock *lock = NULL;
+
+		rc = 0;
+		mutex_lock(&lli->lli_och_mutex);
+		if (fd->fd_lease_och != NULL) {
+			struct obd_client_handle *och = fd->fd_lease_och;
+
+			lock = ldlm_handle2lock(&och->och_lease_handle);
+			if (lock != NULL) {
+				lock_res_and_lock(lock);
+				if (!ldlm_is_cancel(lock))
+					rc = och->och_flags &
+						(FMODE_READ | FMODE_WRITE);
+				unlock_res_and_lock(lock);
+				ldlm_lock_put(lock);
+			}
+		}
+		mutex_unlock(&lli->lli_och_mutex);
+
+		return rc;
+	}
 	default: {
 		int err;
 
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 22fb6ba..c326ff2 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -610,10 +610,14 @@ extern struct kmem_cache *ll_file_data_slab;
 struct lustre_handle;
 struct ll_file_data {
 	struct ll_readahead_state fd_ras;
-	int fd_omode;
 	struct ccc_grouplock fd_grouplock;
 	__u64 lfd_pos;
 	__u32 fd_flags;
+	fmode_t fd_omode;
+	/* openhandle if lease exists for this file.
+	 * Borrow lli->lli_och_mutex to protect assignment */
+	struct obd_client_handle *fd_lease_och;
+	struct obd_client_handle *fd_och;
 	struct file *fd_file;
 	/* Indicate whether need to report failure when close.
 	 * true: failure is known, not report again.
@@ -779,6 +783,11 @@ int ll_put_grouplock(struct inode *inode, struct file *file, unsigned long arg);
 int ll_fid2path(struct inode *inode, void *arg);
 int ll_data_version(struct inode *inode, __u64 *data_version, int extent_lock);
 
+struct obd_client_handle *ll_lease_open(struct inode *inode, struct file *file,
+					fmode_t mode);
+int ll_lease_close(struct obd_client_handle *och, struct inode *inode,
+		   bool *lease_broken);
+
 /* llite/dcache.c */
 
 int ll_dops_init(struct dentry *de, int block, int init_sa);
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_internal.h b/drivers/staging/lustre/lustre/mdc/mdc_internal.h
index 2aeff0e..b995af6 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_internal.h
+++ b/drivers/staging/lustre/lustre/mdc/mdc_internal.h
@@ -69,7 +69,7 @@ void mdc_create_pack(struct ptlrpc_request *req, struct md_op_data *op_data,
 		     const void *data, int datalen, __u32 mode, __u32 uid,
 		     __u32 gid, cfs_cap_t capability, __u64 rdev);
 void mdc_open_pack(struct ptlrpc_request *req, struct md_op_data *op_data,
-		   __u32 mode, __u64 rdev, __u32 flags, const void *data,
+		   __u32 mode, __u64 rdev, __u64 flags, const void *data,
 		   int datalen);
 void mdc_unlink_pack(struct ptlrpc_request *req, struct md_op_data *op_data);
 void mdc_link_pack(struct ptlrpc_request *req, struct md_op_data *op_data);
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_lib.c b/drivers/staging/lustre/lustre/mdc/mdc_lib.c
index b2de478..3e77020 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_lib.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_lib.c
@@ -174,12 +174,12 @@ void mdc_create_pack(struct ptlrpc_request *req, struct md_op_data *op_data,
 	}
 }
 
-static __u64 mds_pack_open_flags(__u32 flags, __u32 mode)
+static __u64 mds_pack_open_flags(__u64 flags, __u32 mode)
 {
 	__u64 cr_flags = (flags & (FMODE_READ | FMODE_WRITE |
 				   MDS_OPEN_HAS_EA | MDS_OPEN_HAS_OBJS |
 				   MDS_OPEN_OWNEROVERRIDE | MDS_OPEN_LOCK |
-				   MDS_OPEN_BY_FID));
+				   MDS_OPEN_BY_FID | MDS_OPEN_LEASE));
 	if (flags & O_CREAT)
 		cr_flags |= MDS_OPEN_CREAT;
 	if (flags & O_EXCL)
@@ -207,7 +207,7 @@ static __u64 mds_pack_open_flags(__u32 flags, __u32 mode)
 
 /* packing of MDS records */
 void mdc_open_pack(struct ptlrpc_request *req, struct md_op_data *op_data,
-		   __u32 mode, __u64 rdev, __u32 flags, const void *lmm,
+		   __u32 mode, __u64 rdev, __u64 flags, const void *lmm,
 		   int lmmlen)
 {
 	struct mdt_rec_create *rec;
@@ -234,6 +234,7 @@ void mdc_open_pack(struct ptlrpc_request *req, struct md_op_data *op_data,
 	rec->cr_suppgid2 = op_data->op_suppgids[1];
 	rec->cr_bias     = op_data->op_bias;
 	rec->cr_umask    = current_umask();
+	rec->cr_old_handle = op_data->op_handle;
 
 	mdc_pack_capa(req, &RMF_CAPA1, op_data->op_capa1);
 	/* the next buffer is child capa, which is used for replay,
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_locks.c b/drivers/staging/lustre/lustre/mdc/mdc_locks.c
index fb5a995..eccbab7 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_locks.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_locks.c
@@ -75,6 +75,12 @@ EXPORT_SYMBOL(it_clear_disposition);
 
 int it_open_error(int phase, struct lookup_intent *it)
 {
+	if (it_disposition(it, DISP_OPEN_LEASE)) {
+		if (phase >= DISP_OPEN_LEASE)
+			return it->d.lustre.it_status;
+		else
+			return 0;
+	}
 	if (it_disposition(it, DISP_OPEN_OPEN)) {
 		if (phase >= DISP_OPEN_OPEN)
 			return it->d.lustre.it_status;
@@ -281,14 +287,21 @@ static struct ptlrpc_request *mdc_intent_open_pack(struct obd_export *exp,
 	/* XXX: openlock is not cancelled for cross-refs. */
 	/* If inode is known, cancel conflicting OPEN locks. */
 	if (fid_is_sane(&op_data->op_fid2)) {
-		if (it->it_flags & (FMODE_WRITE|MDS_OPEN_TRUNC))
-			mode = LCK_CW;
+		if (it->it_flags & MDS_OPEN_LEASE) { /* try to get lease */
+			if (it->it_flags & FMODE_WRITE)
+				mode = LCK_EX;
+			else
+				mode = LCK_PR;
+		} else {
+			if (it->it_flags & (FMODE_WRITE|MDS_OPEN_TRUNC))
+				mode = LCK_CW;
 #ifdef FMODE_EXEC
-		else if (it->it_flags & FMODE_EXEC)
-			mode = LCK_PR;
+			else if (it->it_flags & FMODE_EXEC)
+				mode = LCK_PR;
 #endif
-		else
-			mode = LCK_CR;
+			else
+				mode = LCK_CR;
+		}
 		count = mdc_resource_get_unused(exp, &op_data->op_fid2,
 						&cancels, mode,
 						MDS_INODELOCK_OPEN);
@@ -1065,10 +1078,10 @@ int mdc_intent_lock(struct obd_export *exp, struct md_op_data *op_data,
 	LASSERT(it);
 
 	CDEBUG(D_DLMTRACE, "(name: %.*s,"DFID") in obj "DFID
-	       ", intent: %s flags %#o\n", op_data->op_namelen,
-	       op_data->op_name, PFID(&op_data->op_fid2),
-	       PFID(&op_data->op_fid1), ldlm_it2str(it->it_op),
-	       it->it_flags);
+		", intent: %s flags %#Lo\n", op_data->op_namelen,
+		op_data->op_name, PFID(&op_data->op_fid2),
+		PFID(&op_data->op_fid1), ldlm_it2str(it->it_op),
+		it->it_flags);
 
 	lockh.cookie = 0;
 	if (fid_is_sane(&op_data->op_fid2) &&
@@ -1194,9 +1207,10 @@ int mdc_intent_getattr_async(struct obd_export *exp,
 	int		      rc = 0;
 	__u64		    flags = LDLM_FL_HAS_INTENT;
 
-	CDEBUG(D_DLMTRACE,"name: %.*s in inode "DFID", intent: %s flags %#o\n",
-	       op_data->op_namelen, op_data->op_name, PFID(&op_data->op_fid1),
-	       ldlm_it2str(it->it_op), it->it_flags);
+	CDEBUG(D_DLMTRACE,
+		"name: %.*s in inode "DFID", intent: %s flags %#Lo\n",
+		op_data->op_namelen, op_data->op_name, PFID(&op_data->op_fid1),
+		ldlm_it2str(it->it_op), it->it_flags);
 
 	fid_build_reg_res_name(&op_data->op_fid1, &res_id);
 	req = mdc_intent_getattr_pack(exp, it, op_data);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/9] drivers/staging/lustre: indent lustre_ldlm_flags_vals
  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 13:23 ` [PATCH 2/9] staging/lustre/hsm: Implementation of exclusive open Peng Tao
@ 2013-11-19 13:23 ` Peng Tao
  2013-11-19 13:23 ` [PATCH 4/9] staging/lustre/lnet: Fix assert on empty group in selftest module Peng Tao
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Peng Tao, Peng Tao

From: Peng Tao <tao.peng@emc.com>

To follow kernel's "no spaces at the start of a line" rule.

Signed-off-by: Peng Tao <bergwolf@gmail.com>
---
 .../lustre/lustre/include/lustre_dlm_flags.h       |   80 ++++++++++----------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h b/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
index f8b4ba7..75716f1 100644
--- a/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
+++ b/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
@@ -423,46 +423,46 @@ static int hf_lustre_ldlm_fl_ns_srv              = -1;
 static int hf_lustre_ldlm_fl_excl                = -1;
 
 const value_string lustre_ldlm_flags_vals[] = {
-  {LDLM_FL_LOCK_CHANGED,        "LDLM_FL_LOCK_CHANGED"},
-  {LDLM_FL_BLOCK_GRANTED,       "LDLM_FL_BLOCK_GRANTED"},
-  {LDLM_FL_BLOCK_CONV,          "LDLM_FL_BLOCK_CONV"},
-  {LDLM_FL_BLOCK_WAIT,          "LDLM_FL_BLOCK_WAIT"},
-  {LDLM_FL_AST_SENT,            "LDLM_FL_AST_SENT"},
-  {LDLM_FL_REPLAY,              "LDLM_FL_REPLAY"},
-  {LDLM_FL_INTENT_ONLY,         "LDLM_FL_INTENT_ONLY"},
-  {LDLM_FL_HAS_INTENT,          "LDLM_FL_HAS_INTENT"},
-  {LDLM_FL_DISCARD_DATA,        "LDLM_FL_DISCARD_DATA"},
-  {LDLM_FL_NO_TIMEOUT,          "LDLM_FL_NO_TIMEOUT"},
-  {LDLM_FL_BLOCK_NOWAIT,        "LDLM_FL_BLOCK_NOWAIT"},
-  {LDLM_FL_TEST_LOCK,           "LDLM_FL_TEST_LOCK"},
-  {LDLM_FL_CANCEL_ON_BLOCK,     "LDLM_FL_CANCEL_ON_BLOCK"},
-  {LDLM_FL_DENY_ON_CONTENTION,  "LDLM_FL_DENY_ON_CONTENTION"},
-  {LDLM_FL_AST_DISCARD_DATA,    "LDLM_FL_AST_DISCARD_DATA"},
-  {LDLM_FL_FAIL_LOC,            "LDLM_FL_FAIL_LOC"},
-  {LDLM_FL_SKIPPED,             "LDLM_FL_SKIPPED"},
-  {LDLM_FL_CBPENDING,           "LDLM_FL_CBPENDING"},
-  {LDLM_FL_WAIT_NOREPROC,       "LDLM_FL_WAIT_NOREPROC"},
-  {LDLM_FL_CANCEL,              "LDLM_FL_CANCEL"},
-  {LDLM_FL_LOCAL_ONLY,          "LDLM_FL_LOCAL_ONLY"},
-  {LDLM_FL_FAILED,              "LDLM_FL_FAILED"},
-  {LDLM_FL_CANCELING,           "LDLM_FL_CANCELING"},
-  {LDLM_FL_LOCAL,               "LDLM_FL_LOCAL"},
-  {LDLM_FL_LVB_READY,           "LDLM_FL_LVB_READY"},
-  {LDLM_FL_KMS_IGNORE,          "LDLM_FL_KMS_IGNORE"},
-  {LDLM_FL_CP_REQD,             "LDLM_FL_CP_REQD"},
-  {LDLM_FL_CLEANED,             "LDLM_FL_CLEANED"},
-  {LDLM_FL_ATOMIC_CB,           "LDLM_FL_ATOMIC_CB"},
-  {LDLM_FL_BL_AST,              "LDLM_FL_BL_AST"},
-  {LDLM_FL_BL_DONE,             "LDLM_FL_BL_DONE"},
-  {LDLM_FL_NO_LRU,              "LDLM_FL_NO_LRU"},
-  {LDLM_FL_FAIL_NOTIFIED,       "LDLM_FL_FAIL_NOTIFIED"},
-  {LDLM_FL_DESTROYED,           "LDLM_FL_DESTROYED"},
-  {LDLM_FL_SERVER_LOCK,         "LDLM_FL_SERVER_LOCK"},
-  {LDLM_FL_RES_LOCKED,          "LDLM_FL_RES_LOCKED"},
-  {LDLM_FL_WAITED,              "LDLM_FL_WAITED"},
-  {LDLM_FL_NS_SRV,              "LDLM_FL_NS_SRV"},
-  {LDLM_FL_EXCL,                "LDLM_FL_EXCL"},
-  { 0, NULL }
+	{LDLM_FL_LOCK_CHANGED,        "LDLM_FL_LOCK_CHANGED"},
+	{LDLM_FL_BLOCK_GRANTED,       "LDLM_FL_BLOCK_GRANTED"},
+	{LDLM_FL_BLOCK_CONV,          "LDLM_FL_BLOCK_CONV"},
+	{LDLM_FL_BLOCK_WAIT,          "LDLM_FL_BLOCK_WAIT"},
+	{LDLM_FL_AST_SENT,            "LDLM_FL_AST_SENT"},
+	{LDLM_FL_REPLAY,              "LDLM_FL_REPLAY"},
+	{LDLM_FL_INTENT_ONLY,         "LDLM_FL_INTENT_ONLY"},
+	{LDLM_FL_HAS_INTENT,          "LDLM_FL_HAS_INTENT"},
+	{LDLM_FL_DISCARD_DATA,        "LDLM_FL_DISCARD_DATA"},
+	{LDLM_FL_NO_TIMEOUT,          "LDLM_FL_NO_TIMEOUT"},
+	{LDLM_FL_BLOCK_NOWAIT,        "LDLM_FL_BLOCK_NOWAIT"},
+	{LDLM_FL_TEST_LOCK,           "LDLM_FL_TEST_LOCK"},
+	{LDLM_FL_CANCEL_ON_BLOCK,     "LDLM_FL_CANCEL_ON_BLOCK"},
+	{LDLM_FL_DENY_ON_CONTENTION,  "LDLM_FL_DENY_ON_CONTENTION"},
+	{LDLM_FL_AST_DISCARD_DATA,    "LDLM_FL_AST_DISCARD_DATA"},
+	{LDLM_FL_FAIL_LOC,            "LDLM_FL_FAIL_LOC"},
+	{LDLM_FL_SKIPPED,             "LDLM_FL_SKIPPED"},
+	{LDLM_FL_CBPENDING,           "LDLM_FL_CBPENDING"},
+	{LDLM_FL_WAIT_NOREPROC,       "LDLM_FL_WAIT_NOREPROC"},
+	{LDLM_FL_CANCEL,              "LDLM_FL_CANCEL"},
+	{LDLM_FL_LOCAL_ONLY,          "LDLM_FL_LOCAL_ONLY"},
+	{LDLM_FL_FAILED,              "LDLM_FL_FAILED"},
+	{LDLM_FL_CANCELING,           "LDLM_FL_CANCELING"},
+	{LDLM_FL_LOCAL,               "LDLM_FL_LOCAL"},
+	{LDLM_FL_LVB_READY,           "LDLM_FL_LVB_READY"},
+	{LDLM_FL_KMS_IGNORE,          "LDLM_FL_KMS_IGNORE"},
+	{LDLM_FL_CP_REQD,             "LDLM_FL_CP_REQD"},
+	{LDLM_FL_CLEANED,             "LDLM_FL_CLEANED"},
+	{LDLM_FL_ATOMIC_CB,           "LDLM_FL_ATOMIC_CB"},
+	{LDLM_FL_BL_AST,              "LDLM_FL_BL_AST"},
+	{LDLM_FL_BL_DONE,             "LDLM_FL_BL_DONE"},
+	{LDLM_FL_NO_LRU,              "LDLM_FL_NO_LRU"},
+	{LDLM_FL_FAIL_NOTIFIED,       "LDLM_FL_FAIL_NOTIFIED"},
+	{LDLM_FL_DESTROYED,           "LDLM_FL_DESTROYED"},
+	{LDLM_FL_SERVER_LOCK,         "LDLM_FL_SERVER_LOCK"},
+	{LDLM_FL_RES_LOCKED,          "LDLM_FL_RES_LOCKED"},
+	{LDLM_FL_WAITED,              "LDLM_FL_WAITED"},
+	{LDLM_FL_NS_SRV,              "LDLM_FL_NS_SRV"},
+	{LDLM_FL_EXCL,                "LDLM_FL_EXCL"},
+	{ 0, NULL }
 };
 #endif /*  WIRESHARK_COMPILE */
 #endif /* LDLM_ALL_FLAGS_MASK */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/9] staging/lustre/lnet: Fix assert on empty group in selftest module
  2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
                   ` (2 preceding siblings ...)
  2013-11-19 13:23 ` [PATCH 3/9] drivers/staging/lustre: indent lustre_ldlm_flags_vals Peng Tao
@ 2013-11-19 13:23 ` Peng Tao
  2013-11-19 18:37   ` Greg Kroah-Hartman
  2013-11-19 13:23 ` [PATCH 5/9] staging/lustre/lnet: coding style fix for lst_test_add_ioctl Peng Tao
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 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 f1152e4..73d60e9 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] 19+ messages in thread

* [PATCH 5/9] staging/lustre/lnet: coding style fix for lst_test_add_ioctl
  2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
                   ` (3 preceding siblings ...)
  2013-11-19 13:23 ` [PATCH 4/9] staging/lustre/lnet: Fix assert on empty group in selftest module Peng Tao
@ 2013-11-19 13:23 ` Peng Tao
  2013-11-19 13:23 ` [PATCH 6/9] staging/lustre/lnet: remove extra space in lstcon_rpc_trans_abort Peng Tao
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Amir Shehata, Peng Tao, Andreas Dilger

From: Amir Shehata <amir.shehata@intel.com>

To make the function a bit easier to read.

This is coding style fix part of original Lustre commit in external tree.

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>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
---
 drivers/staging/lustre/lnet/selftest/conctl.c |   56 ++++++++++++-------------
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/lustre/lnet/selftest/conctl.c b/drivers/staging/lustre/lnet/selftest/conctl.c
index bce3d3b..cbc416d 100644
--- a/drivers/staging/lustre/lnet/selftest/conctl.c
+++ b/drivers/staging/lustre/lnet/selftest/conctl.c
@@ -723,12 +723,12 @@ lst_stat_query_ioctl(lstio_stat_args_t *args)
 
 int lst_test_add_ioctl(lstio_test_args_t *args)
 {
-	char	   *name;
-	char	   *srcgrp = NULL;
-	char	   *dstgrp = NULL;
-	void	   *param = NULL;
-	int	     ret = 0;
-	int	     rc = -ENOMEM;
+	char		*batch_name;
+	char		*src_name = NULL;
+	char		*dst_name = NULL;
+	void		*param = NULL;
+	int		ret = 0;
+	int		rc = -ENOMEM;
 
 	if (args->lstio_tes_resultp == NULL ||
 	    args->lstio_tes_retp == NULL ||
@@ -755,16 +755,16 @@ int lst_test_add_ioctl(lstio_test_args_t *args)
 	     args->lstio_tes_param_len > PAGE_CACHE_SIZE - sizeof(lstcon_test_t)))
 		return -EINVAL;
 
-	LIBCFS_ALLOC(name, args->lstio_tes_bat_nmlen + 1);
-	if (name == NULL)
+	LIBCFS_ALLOC(batch_name, args->lstio_tes_bat_nmlen + 1);
+	if (batch_name == NULL)
 		return rc;
 
-	LIBCFS_ALLOC(srcgrp, args->lstio_tes_sgrp_nmlen + 1);
-	if (srcgrp == NULL)
+	LIBCFS_ALLOC(src_name, args->lstio_tes_sgrp_nmlen + 1);
+	if (src_name == NULL)
 		goto out;
 
-	LIBCFS_ALLOC(dstgrp, args->lstio_tes_dgrp_nmlen + 1);
-	 if (dstgrp == NULL)
+	LIBCFS_ALLOC(dst_name, args->lstio_tes_dgrp_nmlen + 1);
+	 if (dst_name == NULL)
 		goto out;
 
 	if (args->lstio_tes_param != NULL) {
@@ -774,39 +774,37 @@ int lst_test_add_ioctl(lstio_test_args_t *args)
 	}
 
 	rc = -EFAULT;
-	if (copy_from_user(name,
-			      args->lstio_tes_bat_name,
-			      args->lstio_tes_bat_nmlen) ||
-	    copy_from_user(srcgrp,
-			      args->lstio_tes_sgrp_name,
-			      args->lstio_tes_sgrp_nmlen) ||
-	    copy_from_user(dstgrp,
-			      args->lstio_tes_dgrp_name,
-			      args->lstio_tes_dgrp_nmlen) ||
+	if (copy_from_user(batch_name, args->lstio_tes_bat_name,
+			   args->lstio_tes_bat_nmlen) ||
+	    copy_from_user(src_name, args->lstio_tes_sgrp_name,
+			   args->lstio_tes_sgrp_nmlen) ||
+	    copy_from_user(dst_name, args->lstio_tes_dgrp_name,
+			   args->lstio_tes_dgrp_nmlen) ||
 	    copy_from_user(param, args->lstio_tes_param,
 			      args->lstio_tes_param_len))
 		goto out;
 
-	rc = lstcon_test_add(name,
+	rc = lstcon_test_add(batch_name,
 			    args->lstio_tes_type,
 			    args->lstio_tes_loop,
 			    args->lstio_tes_concur,
 			    args->lstio_tes_dist, args->lstio_tes_span,
-			    srcgrp, dstgrp, param, args->lstio_tes_param_len,
+			    src_name, dst_name, param,
+			    args->lstio_tes_param_len,
 			    &ret, args->lstio_tes_resultp);
 
 	if (ret != 0)
 		rc = (copy_to_user(args->lstio_tes_retp, &ret,
 				       sizeof(ret))) ? -EFAULT : 0;
 out:
-	if (name != NULL)
-		LIBCFS_FREE(name, args->lstio_tes_bat_nmlen + 1);
+	if (batch_name != NULL)
+		LIBCFS_FREE(batch_name, args->lstio_tes_bat_nmlen + 1);
 
-	if (srcgrp != NULL)
-		LIBCFS_FREE(srcgrp, args->lstio_tes_sgrp_nmlen + 1);
+	if (src_name != NULL)
+		LIBCFS_FREE(src_name, args->lstio_tes_sgrp_nmlen + 1);
 
-	if (dstgrp != NULL)
-		LIBCFS_FREE(dstgrp, args->lstio_tes_dgrp_nmlen + 1);
+	if (dst_name != NULL)
+		LIBCFS_FREE(dst_name, args->lstio_tes_dgrp_nmlen + 1);
 
 	if (param != NULL)
 		LIBCFS_FREE(param, args->lstio_tes_param_len);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/9] staging/lustre/lnet: remove extra space in lstcon_rpc_trans_abort
  2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
                   ` (4 preceding siblings ...)
  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 ` 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
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Amir Shehata, Peng Tao, Andreas Dilger

From: Amir Shehata <amir.shehata@intel.com>

This is coding style fix part of original Lustre commit in external tree.

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>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
---
 drivers/staging/lustre/lnet/selftest/conrpc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c
index 9a52f25..53d5892 100644
--- a/drivers/staging/lustre/lnet/selftest/conrpc.c
+++ b/drivers/staging/lustre/lnet/selftest/conrpc.c
@@ -311,7 +311,7 @@ lstcon_rpc_trans_abort(lstcon_rpc_trans_t *trans, int error)
 
 		sfw_abort_rpc(rpc);
 
-		if  (error != ETIMEDOUT)
+		if (error != ETIMEDOUT)
 			continue;
 
 		nd = crpc->crp_node;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/9] staging/lustre/lnet: constify name argument of lstcon_group_find/lstcon_batch_find
  2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
                   ` (5 preceding siblings ...)
  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 ` Peng Tao
  2013-11-19 13:23 ` [PATCH 8/9] staging/lustre/lnet: coding style fix for lstcon_test_add Peng Tao
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Amir Shehata, Peng Tao, Andreas Dilger

From: Amir Shehata <amir.shehata@intel.com>

This is part of original Lustre commit in external tree.

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>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
---
 drivers/staging/lustre/lnet/selftest/console.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/selftest/console.c b/drivers/staging/lustre/lnet/selftest/console.c
index 73d60e9..a27dfa6 100644
--- a/drivers/staging/lustre/lnet/selftest/console.c
+++ b/drivers/staging/lustre/lnet/selftest/console.c
@@ -265,7 +265,7 @@ lstcon_group_decref(lstcon_group_t *grp)
 }
 
 static int
-lstcon_group_find(char *name, lstcon_group_t **grpp)
+lstcon_group_find(const char *name, lstcon_group_t **grpp)
 {
 	lstcon_group_t   *grp;
 
@@ -831,7 +831,7 @@ lstcon_group_info(char *name, lstcon_ndlist_ent_t *gents_p,
 }
 
 int
-lstcon_batch_find(char *name, lstcon_batch_t **batpp)
+lstcon_batch_find(const char *name, lstcon_batch_t **batpp)
 {
 	lstcon_batch_t   *bat;
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/9] staging/lustre/lnet: coding style fix for lstcon_test_add
  2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
                   ` (6 preceding siblings ...)
  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 ` Peng Tao
  2013-11-19 13:23 ` [PATCH 9/9] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES Peng Tao
  2013-11-19 18:41 ` [PATCH 0/9] staging/lustre: sync with exernal tree Greg Kroah-Hartman
  9 siblings, 0 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Amir Shehata, Peng Tao, Andreas Dilger

From: Amir Shehata <amir.shehata@intel.com>

To make the function a bit easier to read.

This is coding style fix part of original Lustre commit in external tree.

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>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
---
 drivers/staging/lustre/lnet/selftest/console.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/lustre/lnet/selftest/console.c b/drivers/staging/lustre/lnet/selftest/console.c
index a27dfa6..9556bc0 100644
--- a/drivers/staging/lustre/lnet/selftest/console.c
+++ b/drivers/staging/lustre/lnet/selftest/console.c
@@ -1320,18 +1320,18 @@ lstcon_test_add(char *batch_name, int type, int loop,
 	}
 
 	memset(test, 0, offsetof(lstcon_test_t, tes_param[paramlen]));
-	test->tes_hdr.tsb_id    = batch->bat_hdr.tsb_id;
-	test->tes_batch	 = batch;
-	test->tes_type	  = type;
-	test->tes_oneside       = 0; /* TODO */
-	test->tes_loop	  = loop;
+	test->tes_hdr.tsb_id	= batch->bat_hdr.tsb_id;
+	test->tes_batch		= batch;
+	test->tes_type		= type;
+	test->tes_oneside	= 0; /* TODO */
+	test->tes_loop		= loop;
 	test->tes_concur	= concur;
-	test->tes_stop_onerr    = 1; /* TODO */
-	test->tes_span	  = span;
-	test->tes_dist	  = dist;
+	test->tes_stop_onerr	= 1; /* TODO */
+	test->tes_span		= span;
+	test->tes_dist		= dist;
 	test->tes_cliidx	= 0; /* just used for creating RPC */
-	test->tes_src_grp       = src_grp;
-	test->tes_dst_grp       = dst_grp;
+	test->tes_src_grp	= src_grp;
+	test->tes_dst_grp	= dst_grp;
 	INIT_LIST_HEAD(&test->tes_trans_list);
 
 	if (param != NULL) {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 9/9] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES
  2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
                   ` (7 preceding siblings ...)
  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
  2013-11-19 18:40   ` Greg Kroah-Hartman
  2013-11-19 18:41 ` [PATCH 0/9] staging/lustre: sync with exernal tree Greg Kroah-Hartman
  9 siblings, 1 reply; 19+ messages in thread
From: Peng Tao @ 2013-11-19 13:23 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..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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/9] staging/lustre/llite: Access to released file trigs a restore
  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
  0 siblings, 1 reply; 19+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-19 18:29 UTC (permalink / raw)
  To: Peng Tao; +Cc: linux-kernel, JC Lafoucriere, Andreas Dilger

"trigs"?  Come on, we don't have a lack of characters here...

On Tue, Nov 19, 2013 at 09:23:40PM +0800, Peng Tao wrote:
> 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    |   17 +++---
>  10 files changed, 168 insertions(+), 24 deletions(-)

This patch has checkpatch errors, sorry, please fix them before sending
them again.

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] staging/lustre/lnet: Fix assert on empty group in selftest module
  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
  0 siblings, 1 reply; 19+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-19 18:37 UTC (permalink / raw)
  To: Peng Tao; +Cc: linux-kernel, Amir Shehata, Andreas Dilger

On Tue, Nov 19, 2013 at 09:23:43PM +0800, Peng Tao wrote:
> 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.

This patch adds build warnings to the kernel build process, so I can't
apply it, sorry.  Please fix that up before sending it again.

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 9/9] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES
  2013-11-19 13:23 ` [PATCH 9/9] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES Peng Tao
@ 2013-11-19 18:40   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 19+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-19 18:40 UTC (permalink / raw)
  To: Peng Tao; +Cc: linux-kernel, Andreas Dilger, Lai Siyao

On Tue, Nov 19, 2013 at 09:23:48PM +0800, Peng Tao wrote:
> 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.

There are codingstyle warnings in this patch, please fix before
resending.

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] staging/lustre: sync with exernal tree
  2013-11-19 13:23 [PATCH 0/9] staging/lustre: sync with exernal tree Peng Tao
                   ` (8 preceding siblings ...)
  2013-11-19 13:23 ` [PATCH 9/9] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES Peng Tao
@ 2013-11-19 18:41 ` Greg Kroah-Hartman
  9 siblings, 0 replies; 19+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-19 18:41 UTC (permalink / raw)
  To: Peng Tao; +Cc: linux-kernel, Andreas Dilger

On Tue, Nov 19, 2013 at 09:23:39PM +0800, Peng Tao wrote:
> Hi Greg,
> 
> Following patches were ported from Lustre external tree.
> 
> PATCH 4/9 was splitted into five patches to do separate things that
> were done in the original single patch.
> 
> PATCh 3/9 is a cleanup patch.

I've applied some of these, the ones I have not, I have commented on
why.

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/9] staging/lustre/llite: Access to released file trigs a restore
  2013-11-19 18:29   ` Greg Kroah-Hartman
@ 2013-11-20  7:35     ` Peng Tao
  2013-11-20  9:40       ` Peng Tao
  0 siblings, 1 reply; 19+ messages in thread
From: Peng Tao @ 2013-11-20  7:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Linux Kernel Mailing List, JC Lafoucriere, Andreas Dilger

On Wed, Nov 20, 2013 at 2:29 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> "trigs"?  Come on, we don't have a lack of characters here...
>
Will fix up.

> On Tue, Nov 19, 2013 at 09:23:40PM +0800, Peng Tao wrote:
>> 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    |   17 +++---
>>  10 files changed, 168 insertions(+), 24 deletions(-)
>
> This patch has checkpatch errors, sorry, please fix them before sending
> them again.
>
The "quoted string split across lines" warning is a bit confusing.

WARNING: quoted string split across lines
#265: FILE: drivers/staging/lustre/lustre/llite/vvp_io.c:125:
+       CDEBUG(D_VFSTRACE, DFID" ignore/verify layout %d/%d, layout version %d "
+                          "restore needed %d\n",

If I put the quoted string in the same line, I got

WARNING: line over 80 characters
#264: FILE: drivers/staging/lustre/lustre/llite/vvp_io.c:124:
+       CDEBUG(D_VFSTRACE, DFID" ignore/verify layout %d/%d, layout
version %d restore needed %d\n",

So one way or another, the patch breaks a rule.

Looking at scripts/checkpatch.pl commit log, I saw

commit ca56dc098caf93b5437cd6c4ee49f02aa18f84d6
Author: Josh Triplett <josh@joshtriplett.org>
Date:   Fri Mar 23 15:02:21 2012 -0700

    checkpatch: check for quoted strings broken across lines

    checkpatch already makes an exception to the 80-column rule for quoted
    strings, and Documentation/CodingStyle recommends not splitting quoted
    strings across lines, because it breaks the ability to grep for the
    string.  Rather than just permitting this, actively warn about quoted
    strings split across lines.

But it seems that the exception for quoted strings to the 80-column
rule no longer holds?

Thanks,
Tao

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] staging/lustre/lnet: Fix assert on empty group in selftest module
  2013-11-19 18:37   ` Greg Kroah-Hartman
@ 2013-11-20  9:26     ` Peng Tao
  2013-11-20 16:27       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 19+ messages in thread
From: Peng Tao @ 2013-11-20  9:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Linux Kernel Mailing List, Amir Shehata, Andreas Dilger

On Wed, Nov 20, 2013 at 2:37 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Nov 19, 2013 at 09:23:43PM +0800, Peng Tao wrote:
>> 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.
>
> This patch adds build warnings to the kernel build process, so I can't
> apply it, sorry.  Please fix that up before sending it again.
>
Hi Greg,

Can you please be explicit about what build warning you saw?

I tried to reproduce it with gcc version 4.1.2 and 4.6.3 on my
machine, but didn't see any build warnings with this patch applied.

Thanks,
Tao

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/9] staging/lustre/llite: Access to released file trigs a restore
  2013-11-20  7:35     ` Peng Tao
@ 2013-11-20  9:40       ` Peng Tao
  0 siblings, 0 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-20  9:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Linux Kernel Mailing List, JC Lafoucriere, Andreas Dilger

On Wed, Nov 20, 2013 at 3:35 PM, Peng Tao <bergwolf@gmail.com> wrote:
> On Wed, Nov 20, 2013 at 2:29 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> "trigs"?  Come on, we don't have a lack of characters here...
>>
> Will fix up.
>
>> On Tue, Nov 19, 2013 at 09:23:40PM +0800, Peng Tao wrote:
>>> 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    |   17 +++---
>>>  10 files changed, 168 insertions(+), 24 deletions(-)
>>
>> This patch has checkpatch errors, sorry, please fix them before sending
>> them again.
>>
> The "quoted string split across lines" warning is a bit confusing.
>
> WARNING: quoted string split across lines
> #265: FILE: drivers/staging/lustre/lustre/llite/vvp_io.c:125:
> +       CDEBUG(D_VFSTRACE, DFID" ignore/verify layout %d/%d, layout version %d "
> +                          "restore needed %d\n",
>
> If I put the quoted string in the same line, I got
>
> WARNING: line over 80 characters
> #264: FILE: drivers/staging/lustre/lustre/llite/vvp_io.c:124:
> +       CDEBUG(D_VFSTRACE, DFID" ignore/verify layout %d/%d, layout
> version %d restore needed %d\n",
>
> So one way or another, the patch breaks a rule.
>
> Looking at scripts/checkpatch.pl commit log, I saw
>
> commit ca56dc098caf93b5437cd6c4ee49f02aa18f84d6
> Author: Josh Triplett <josh@joshtriplett.org>
> Date:   Fri Mar 23 15:02:21 2012 -0700
>
>     checkpatch: check for quoted strings broken across lines
>
>     checkpatch already makes an exception to the 80-column rule for quoted
>     strings, and Documentation/CodingStyle recommends not splitting quoted
>     strings across lines, because it breaks the ability to grep for the
>     string.  Rather than just permitting this, actively warn about quoted
>     strings split across lines.
>
> But it seems that the exception for quoted strings to the 80-column
> rule no longer holds?
>
I found out that the quoted strings exception only applies to lines
that have only quoted strings. I'll fix up.

Thanks,
Tao

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] staging/lustre/lnet: Fix assert on empty group in selftest module
  2013-11-20  9:26     ` Peng Tao
@ 2013-11-20 16:27       ` Greg Kroah-Hartman
  2013-11-20 17:34         ` Peng Tao
  0 siblings, 1 reply; 19+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-20 16:27 UTC (permalink / raw)
  To: Peng Tao; +Cc: Linux Kernel Mailing List, Amir Shehata, Andreas Dilger

On Wed, Nov 20, 2013 at 05:26:57PM +0800, Peng Tao wrote:
> On Wed, Nov 20, 2013 at 2:37 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Nov 19, 2013 at 09:23:43PM +0800, Peng Tao wrote:
> >> 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.
> >
> > This patch adds build warnings to the kernel build process, so I can't
> > apply it, sorry.  Please fix that up before sending it again.
> >
> Hi Greg,
> 
> Can you please be explicit about what build warning you saw?

I don't remember what it was at the moment, sorry.

> I tried to reproduce it with gcc version 4.1.2 and 4.6.3 on my
> machine, but didn't see any build warnings with this patch applied.

I have 4.7.3 here, and x86-64.  Try that and see what happens.

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] staging/lustre/lnet: Fix assert on empty group in selftest module
  2013-11-20 16:27       ` Greg Kroah-Hartman
@ 2013-11-20 17:34         ` Peng Tao
  0 siblings, 0 replies; 19+ messages in thread
From: Peng Tao @ 2013-11-20 17:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Linux Kernel Mailing List, Amir Shehata, Andreas Dilger

On 11/21/2013 12:27 AM, Greg Kroah-Hartman wrote:
> On Wed, Nov 20, 2013 at 05:26:57PM +0800, Peng Tao wrote:
>> On Wed, Nov 20, 2013 at 2:37 AM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>> On Tue, Nov 19, 2013 at 09:23:43PM +0800, Peng Tao wrote:
>>>> 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.
>>> This patch adds build warnings to the kernel build process, so I can't
>>> apply it, sorry.  Please fix that up before sending it again.
>>>
>> Hi Greg,
>>
>> Can you please be explicit about what build warning you saw?
> I don't remember what it was at the moment, sorry.
>
>> I tried to reproduce it with gcc version 4.1.2 and 4.6.3 on my
>> machine, but didn't see any build warnings with this patch applied.
> I have 4.7.3 here, and x86-64.  Try that and see what happens.
>
Could you please share you .config? I just tried gcc 4.7.3 on x86_64 but 
still no luck. Guess it might have something to do with certain Kconfig 
combinations.

[X61@linux-lustre]$gcc --version
gcc (Ubuntu/Linaro 4.7.3-1ubuntu1) 4.7.3
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[X61@linux-lustre]$uname -a
Linux X61 3.11.0 #1 SMP Wed Sep 4 23:16:15 CST 2013 x86_64 x86_64 x86_64 
GNU/Linux

Thanks,
Tao

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-11-20 17:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 9/9] staging/lustre/ldlm: fix resource/fid check, use DLDLMRES Peng Tao
2013-11-19 18:40   ` Greg Kroah-Hartman
2013-11-19 18:41 ` [PATCH 0/9] staging/lustre: sync with exernal tree Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.