All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/12] NFS support for async intra COPY
@ 2017-09-28 17:28 Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 01/12] fs: Don't copy beyond the end of the file Olga Kornievskaia
                   ` (11 more replies)
  0 siblings, 12 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Change the client side to always request an asynchronous copy.

If server replies to the COPY with the copy stateid, client will go
wait on the CB_OFFLOAD. To fend off the race between CB_OFFLOAD and
COPY reply, we check the list of pending callbacks before going to
wait. Client adds the copy to the global list of copy stateids for the
callback to look thru and signal the waiting copy.

When the client receives reply from the CB_OFFLOAD with some bytes and
committed how is UNSTABLE, then COMMIT is sent to the server. The results
arep propagated to the VFS and application. Assuming that application
will deal with a partial result and continue from the new offset if needed.

If server errs with ERR_OFFLOAD_NOREQS the copy will be re-sent as a
synchronous COPY. If application cancels an in-flight COPY,
OFFLOAD_CANCEL is sent to the server (sending it as an async RPC in the
context of the nfsid_workqueue).

v4:
separated async patches from the async+inter series
update the code based on the constifying patches that went for 4.13
addressed minor comments provided by Anna

Anna Schumaker (1):
  fs: Don't copy beyond the end of the file

Olga Kornievskaia (11):
  NFS CB_OFFLOAD xdr
  NFS OFFLOAD_STATUS xdr
  NFS OFFLOAD_STATUS op
  NFS OFFLOAD_CANCEL xdr
  NFS COPY xdr handle async reply
  NFS add support for asynchronous COPY
  NFS handle COPY reply CB_OFFLOAD call race
  NFS export nfs4_async_handle_error
  NFS send OFFLOAD_CANCEL when COPY killed
  NFS handle COPY ERR_OFFLOAD_NO_REQS
  NFS add a simple sync nfs4_proc_commit after async COPY

 fs/nfs/callback.h         |  12 +++
 fs/nfs/callback_proc.c    |  54 +++++++++++
 fs/nfs/callback_xdr.c     |  81 +++++++++++++++-
 fs/nfs/client.c           |   1 +
 fs/nfs/nfs42.h            |   5 +-
 fs/nfs/nfs42proc.c        | 233 ++++++++++++++++++++++++++++++++++++++++++++--
 fs/nfs/nfs42xdr.c         | 187 ++++++++++++++++++++++++++++++++++---
 fs/nfs/nfs4_fs.h          |   5 +-
 fs/nfs/nfs4client.c       |  15 +++
 fs/nfs/nfs4proc.c         |  37 +++++++-
 fs/nfs/nfs4xdr.c          |   2 +
 fs/read_write.c           |   3 +
 include/linux/nfs4.h      |   2 +
 include/linux/nfs_fs.h    |   9 ++
 include/linux/nfs_fs_sb.h |   4 +
 include/linux/nfs_xdr.h   |  14 +++
 16 files changed, 641 insertions(+), 23 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v4 01/12] fs: Don't copy beyond the end of the file
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 02/12] NFS CB_OFFLOAD xdr Olga Kornievskaia
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/read_write.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/read_write.c b/fs/read_write.c
index a2b9a47..47aec8e 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1563,6 +1563,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 	if (unlikely(ret))
 		return ret;
 
+	if (pos_in >= i_size_read(inode_in))
+		return -EINVAL;
+
 	if (!(file_in->f_mode & FMODE_READ) ||
 	    !(file_out->f_mode & FMODE_WRITE) ||
 	    (file_out->f_flags & O_APPEND))
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 02/12] NFS CB_OFFLOAD xdr
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 01/12] fs: Don't copy beyond the end of the file Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 03/12] NFS OFFLOAD_STATUS xdr Olga Kornievskaia
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/callback.h      | 12 ++++++++
 fs/nfs/callback_proc.c |  7 +++++
 fs/nfs/callback_xdr.c  | 81 +++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
index 3dc54d7..cbc5c4c 100644
--- a/fs/nfs/callback.h
+++ b/fs/nfs/callback.h
@@ -183,6 +183,18 @@ struct cb_notify_lock_args {
 extern __be32 nfs4_callback_notify_lock(void *argp, void *resp,
 					 struct cb_process_state *cps);
 #endif /* CONFIG_NFS_V4_1 */
+#ifdef CONFIG_NFS_V4_2
+struct cb_offloadargs {
+	struct nfs_fh		coa_fh;
+	nfs4_stateid		coa_stateid;
+	uint32_t		error;
+	uint64_t		wr_count;
+	struct nfs_writeverf	wr_writeverf;
+};
+
+extern __be32 nfs4_callback_offload(void *args, void *dummy,
+				    struct cb_process_state *cps);
+#endif /* CONFIG_NFS_V4_2 */
 extern int check_gss_callback_principal(struct nfs_client *, struct svc_rqst *);
 extern __be32 nfs4_callback_getattr(void *argp, void *resp,
 				    struct cb_process_state *cps);
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 14358de..eef2728 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -656,3 +656,10 @@ __be32 nfs4_callback_notify_lock(void *argp, void *resp,
 	return htonl(NFS4_OK);
 }
 #endif /* CONFIG_NFS_V4_1 */
+#ifdef CONFIG_NFS_V4_2
+__be32 nfs4_callback_offload(void *args, void *dummy,
+			     struct cb_process_state *cps)
+{
+	return 0;
+}
+#endif /* CONFIG_NFS_V4_2 */
diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 681dd64..ef34639 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -37,6 +37,9 @@
 #define CB_OP_RECALLSLOT_RES_MAXSZ	(CB_OP_HDR_RES_MAXSZ)
 #define CB_OP_NOTIFY_LOCK_RES_MAXSZ	(CB_OP_HDR_RES_MAXSZ)
 #endif /* CONFIG_NFS_V4_1 */
+#ifdef CONFIG_NFS_V4_2
+#define CB_OP_OFFLOAD_RES_MAXSZ		(CB_OP_HDR_RES_MAXSZ)
+#endif /* CONFIG_NFS_V4_2 */
 
 #define NFSDBG_FACILITY NFSDBG_CALLBACK
 
@@ -526,7 +529,73 @@ static __be32 decode_notify_lock_args(struct svc_rqst *rqstp,
 }
 
 #endif /* CONFIG_NFS_V4_1 */
+#ifdef CONFIG_NFS_V4_2
+static __be32 decode_write_response(struct xdr_stream *xdr,
+					struct cb_offloadargs *args)
+{
+	__be32 *p;
+	__be32 dummy;
+
+	/* skip the always zero field */
+	p = read_buf(xdr, 4);
+	if (unlikely(!p))
+		goto out;
+	dummy = ntohl(*p++);
+
+	/* decode count, stable_how, verifier */
+	p = xdr_inline_decode(xdr, 8 + 4);
+	if (unlikely(!p))
+		goto out;
+	p = xdr_decode_hyper(p, &args->wr_count);
+	args->wr_writeverf.committed = be32_to_cpup(p);
+	p = xdr_inline_decode(xdr, NFS4_VERIFIER_SIZE);
+	if (likely(p)) {
+		memcpy(&args->wr_writeverf.verifier.data[0], p,
+			NFS4_VERIFIER_SIZE);
+		return 0;
+	}
+out:
+	return htonl(NFS4ERR_RESOURCE);
+}
+
+static __be32 decode_offload_args(struct svc_rqst *rqstp,
+					struct xdr_stream *xdr,
+					void *data)
+{
+	struct cb_offloadargs *args = data;
+	__be32 *p;
+	__be32 status;
+
+	/* decode fh */
+	status = decode_fh(xdr, &args->coa_fh);
+	if (unlikely(status != 0))
+		return status;
+
+	/* decode stateid */
+	status = decode_stateid(xdr, &args->coa_stateid);
+	if (unlikely(status != 0))
+		return status;
 
+	/* decode status */
+	p = read_buf(xdr, 4);
+	if (unlikely(!p))
+		goto out;
+	args->error = ntohl(*p++);
+	if (!args->error) {
+		status = decode_write_response(xdr, args);
+		if (unlikely(status != 0))
+			return status;
+	} else {
+		p = xdr_inline_decode(xdr, 8);
+		if (unlikely(!p))
+			goto out;
+		p = xdr_decode_hyper(p, &args->wr_count);
+	}
+	return 0;
+out:
+	return htonl(NFS4ERR_RESOURCE);
+}
+#endif /* CONFIG_NFS_V4_2 */
 static __be32 encode_string(struct xdr_stream *xdr, unsigned int len, const char *str)
 {
 	if (unlikely(xdr_stream_encode_opaque(xdr, str, len) < 0))
@@ -793,7 +862,10 @@ static void nfs4_cb_free_slot(struct cb_process_state *cps)
 	if (status != htonl(NFS4ERR_OP_ILLEGAL))
 		return status;
 
-	if (op_nr == OP_CB_OFFLOAD)
+	if (op_nr == OP_CB_OFFLOAD) {
+		*op = &callback_ops[op_nr];
+		return htonl(NFS_OK);
+	} else
 		return htonl(NFS4ERR_NOTSUPP);
 	return htonl(NFS4ERR_OP_ILLEGAL);
 }
@@ -989,6 +1061,13 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp)
 		.res_maxsize = CB_OP_NOTIFY_LOCK_RES_MAXSZ,
 	},
 #endif /* CONFIG_NFS_V4_1 */
+#ifdef CONFIG_NFS_V4_2
+	[OP_CB_OFFLOAD] = {
+		.process_op = nfs4_callback_offload,
+		.decode_args = decode_offload_args,
+		.res_maxsize = CB_OP_OFFLOAD_RES_MAXSZ,
+	},
+#endif /* CONFIG_NFS_V4_2 */
 };
 
 /*
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 03/12] NFS OFFLOAD_STATUS xdr
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 01/12] fs: Don't copy beyond the end of the file Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 02/12] NFS CB_OFFLOAD xdr Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 04/12] NFS OFFLOAD_STATUS op Olga Kornievskaia
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs42xdr.c         | 89 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/nfs4proc.c         |  1 +
 fs/nfs/nfs4xdr.c          |  1 +
 include/linux/nfs4.h      |  1 +
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   | 12 +++++++
 6 files changed, 105 insertions(+)

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index 5ee1b0f..3ebbb41 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -25,6 +25,11 @@
 					 NFS42_WRITE_RES_SIZE + \
 					 1 /* cr_consecutive */ + \
 					 1 /* cr_synchronous */)
+#define encode_offload_status_maxsz	(op_encode_hdr_maxsz + \
+					 XDR_QUADLEN(NFS4_STATEID_SIZE))
+#define decode_offload_status_maxsz	(op_decode_hdr_maxsz + \
+					 2 /* osr_count */ + \
+					 1 + 1 /* osr_complete<1> */)
 #define encode_deallocate_maxsz		(op_encode_hdr_maxsz + \
 					 encode_fallocate_maxsz)
 #define decode_deallocate_maxsz		(op_decode_hdr_maxsz)
@@ -74,6 +79,12 @@
 					 decode_putfh_maxsz + \
 					 decode_copy_maxsz + \
 					 decode_commit_maxsz)
+#define NFS4_enc_offload_status_sz	(compound_encode_hdr_maxsz + \
+					 encode_putfh_maxsz + \
+					 encode_offload_status_maxsz)
+#define NFS4_dec_offload_status_sz	(compound_decode_hdr_maxsz + \
+					 decode_putfh_maxsz + \
+					 decode_offload_status_maxsz)
 #define NFS4_enc_deallocate_sz		(compound_encode_hdr_maxsz + \
 					 encode_putfh_maxsz + \
 					 encode_deallocate_maxsz + \
@@ -144,6 +155,14 @@ static void encode_copy(struct xdr_stream *xdr,
 	encode_uint32(xdr, 0); /* src server list */
 }
 
+static void encode_offload_status(struct xdr_stream *xdr,
+				  const struct nfs42_offload_status_args *args,
+				  struct compound_hdr *hdr)
+{
+	encode_op_hdr(xdr, OP_OFFLOAD_STATUS, decode_offload_status_maxsz, hdr);
+	encode_nfs4_stateid(xdr, &args->osa_stateid);
+}
+
 static void encode_deallocate(struct xdr_stream *xdr,
 			      const struct nfs42_falloc_args *args,
 			      struct compound_hdr *hdr)
@@ -260,6 +279,25 @@ static void nfs4_xdr_enc_copy(struct rpc_rqst *req,
 }
 
 /*
+ * Encode OFFLOAD_STATUS request
+ */
+static void nfs4_xdr_enc_offload_status(struct rpc_rqst *req,
+					struct xdr_stream *xdr,
+					const void *data)
+{
+	const struct nfs42_offload_status_args *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->osa_seq_args),
+	};
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->osa_seq_args, &hdr);
+	encode_putfh(xdr, args->osa_src_fh, &hdr);
+	encode_offload_status(xdr, args, &hdr);
+	encode_nops(&hdr);
+}
+
+/*
  * Encode DEALLOCATE request
  */
 static void nfs4_xdr_enc_deallocate(struct rpc_rqst *req,
@@ -412,6 +450,31 @@ static int decode_copy(struct xdr_stream *xdr, struct nfs42_copy_res *res)
 	return decode_copy_requirements(xdr, res);
 }
 
+static int decode_offload_status(struct xdr_stream *xdr,
+				 struct nfs42_offload_status_res *res)
+{
+	__be32 *p;
+	uint32_t count;
+	int status;
+
+	status = decode_op_hdr(xdr, OP_OFFLOAD_STATUS);
+	if (status)
+		return status;
+	p = xdr_inline_decode(xdr, 8 + 4);
+	if (unlikely(!p))
+		goto out_overflow;
+	p = xdr_decode_hyper(p, &res->osr_count);
+	count = be32_to_cpup(p++);
+	if (count) {
+		p = xdr_inline_decode(xdr, 4);
+		res->osr_status = nfs4_stat_to_errno(be32_to_cpup(p));
+	}
+	return 0;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
 static int decode_deallocate(struct xdr_stream *xdr, struct nfs42_falloc_res *res)
 {
 	return decode_op_hdr(xdr, OP_DEALLOCATE);
@@ -512,6 +575,32 @@ static int nfs4_xdr_dec_copy(struct rpc_rqst *rqstp,
 }
 
 /*
+ * Decode OFFLOAD_STATUS response
+ */
+static int nfs4_xdr_dec_offload_status(struct rpc_rqst *rqstp,
+				       struct xdr_stream *xdr,
+				       void *data)
+{
+	struct nfs42_offload_status_res *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->osr_seq_res, rqstp);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+	status = decode_offload_status(xdr, res);
+
+out:
+	return status;
+}
+
+/*
  * Decode DEALLOCATE request
  */
 static int nfs4_xdr_dec_deallocate(struct rpc_rqst *rqstp,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6c61e2b..7d59593 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -9295,6 +9295,7 @@ static bool nfs4_match_stateid(const nfs4_stateid *s1,
 		| NFS_CAP_ATOMIC_OPEN_V1
 		| NFS_CAP_ALLOCATE
 		| NFS_CAP_COPY
+		| NFS_CAP_OFFLOAD_STATUS
 		| NFS_CAP_DEALLOCATE
 		| NFS_CAP_SEEK
 		| NFS_CAP_LAYOUTSTATS
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 37c8af0..5406f1d 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -7733,6 +7733,7 @@ int nfs4_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
 	PROC(LAYOUTSTATS,	enc_layoutstats,	dec_layoutstats),
 	PROC(CLONE,		enc_clone,		dec_clone),
 	PROC(COPY,		enc_copy,		dec_copy),
+	PROC(OFFLOAD_STATUS,	enc_offload_status,	dec_offload_status),
 #endif /* CONFIG_NFS_V4_2 */
 };
 
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 47239c3..0f37785 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -524,6 +524,7 @@ enum {
 	NFSPROC4_CLNT_LAYOUTSTATS,
 	NFSPROC4_CLNT_CLONE,
 	NFSPROC4_CLNT_COPY,
+	NFSPROC4_CLNT_OFFLOAD_STATUS,
 };
 
 /* nfs41 types */
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 74c4466..fb4e2b8 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -252,5 +252,6 @@ struct nfs_server {
 #define NFS_CAP_LAYOUTSTATS	(1U << 22)
 #define NFS_CAP_CLONE		(1U << 23)
 #define NFS_CAP_COPY		(1U << 24)
+#define NFS_CAP_OFFLOAD_STATUS	(1U << 25)
 
 #endif
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 164d535..5ffd745 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1399,6 +1399,18 @@ struct nfs42_copy_res {
 	struct nfs_commitres		commit_res;
 };
 
+struct nfs42_offload_status_args {
+	struct nfs4_sequence_args	osa_seq_args;
+	struct nfs_fh			*osa_src_fh;
+	nfs4_stateid			osa_stateid;
+};
+
+struct nfs42_offload_status_res {
+	struct nfs4_sequence_res	osr_seq_res;
+	uint64_t			osr_count;
+	int				osr_status;
+};
+
 struct nfs42_seek_args {
 	struct nfs4_sequence_args	seq_args;
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 04/12] NFS OFFLOAD_STATUS op
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (2 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 03/12] NFS OFFLOAD_STATUS xdr Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 20:32   ` Anna Schumaker
  2017-09-28 17:28 ` [PATCH v4 05/12] NFS OFFLOAD_CANCEL xdr Olga Kornievskaia
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs42.h     |  5 ++++-
 fs/nfs/nfs42proc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h
index b6cd153..6b9decf 100644
--- a/fs/nfs/nfs42.h
+++ b/fs/nfs/nfs42.h
@@ -11,6 +11,7 @@
  */
 #define PNFS_LAYOUTSTATS_MAXDEV (4)
 
+#if defined(CONFIG_NFS_V4_2)
 /* nfs4.2proc.c */
 int nfs42_proc_allocate(struct file *, loff_t, loff_t);
 ssize_t nfs42_proc_copy(struct file *, loff_t, struct file *, loff_t, size_t);
@@ -19,5 +20,7 @@
 int nfs42_proc_layoutstats_generic(struct nfs_server *,
 				   struct nfs42_layoutstat_data *);
 int nfs42_proc_clone(struct file *, struct file *, loff_t, loff_t, loff_t);
-
+int nfs42_proc_offload_status(struct file *, nfs4_stateid *,
+			      struct nfs42_offload_status_res *);
+#endif /* CONFIG_NFS_V4_2) */
 #endif /* __LINUX_FS_NFS_NFS4_2_H */
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 6c2db51..5c42e71 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -263,6 +263,48 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
 	return err;
 }
 
+int _nfs42_proc_offload_status(struct file *dst, nfs4_stateid *stateid,
+				struct nfs42_offload_status_res *res)
+{
+	struct nfs42_offload_status_args args;
+	struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
+	struct rpc_message msg = {
+		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OFFLOAD_STATUS],
+		.rpc_resp = res,
+		.rpc_argp = &args,
+	};
+	int status;
+
+	args.osa_src_fh = NFS_FH(file_inode(dst));
+	memcpy(&args.osa_stateid, stateid, sizeof(args.osa_stateid));
+	status = nfs4_call_sync(dst_server->client, dst_server, &msg,
+				&args.osa_seq_args, &res->osr_seq_res, 0);
+	if (status == -ENOTSUPP)
+		dst_server->caps &= ~NFS_CAP_OFFLOAD_STATUS;
+
+	return status;
+}
+
+int nfs42_proc_offload_status(struct file *dst, nfs4_stateid *stateid,
+				struct nfs42_offload_status_res *res)
+{
+	struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
+	struct nfs4_exception exception = { };
+	int status;
+
+	if (!(dst_server->caps & NFS_CAP_OFFLOAD_STATUS))
+		return -EOPNOTSUPP;
+
+	do {
+		status = _nfs42_proc_offload_status(dst, stateid, res);
+		if (status == -ENOTSUPP)
+			return -EOPNOTSUPP;
+		status = nfs4_handle_exception(dst_server, status, &exception);
+	} while (exception.retry);
+
+	return status;
+}
+
 static loff_t _nfs42_proc_llseek(struct file *filep,
 		struct nfs_lock_context *lock, loff_t offset, int whence)
 {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 05/12] NFS OFFLOAD_CANCEL xdr
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (3 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 04/12] NFS OFFLOAD_STATUS op Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 06/12] NFS COPY xdr handle async reply Olga Kornievskaia
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs42xdr.c         | 68 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/nfs4proc.c         |  1 +
 fs/nfs/nfs4xdr.c          |  1 +
 include/linux/nfs4.h      |  1 +
 include/linux/nfs_fs_sb.h |  1 +
 5 files changed, 72 insertions(+)

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index 3ebbb41..940621a 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -30,6 +30,9 @@
 #define decode_offload_status_maxsz	(op_decode_hdr_maxsz + \
 					 2 /* osr_count */ + \
 					 1 + 1 /* osr_complete<1> */)
+#define encode_offload_cancel_maxsz	(op_encode_hdr_maxsz + \
+					 XDR_QUADLEN(NFS4_STATEID_SIZE))
+#define decode_offload_cancel_maxsz	(op_decode_hdr_maxsz)
 #define encode_deallocate_maxsz		(op_encode_hdr_maxsz + \
 					 encode_fallocate_maxsz)
 #define decode_deallocate_maxsz		(op_decode_hdr_maxsz)
@@ -85,6 +88,12 @@
 #define NFS4_dec_offload_status_sz	(compound_decode_hdr_maxsz + \
 					 decode_putfh_maxsz + \
 					 decode_offload_status_maxsz)
+#define NFS4_enc_offload_cancel_sz	(compound_encode_hdr_maxsz + \
+					 encode_putfh_maxsz + \
+					 encode_offload_cancel_maxsz)
+#define NFS4_dec_offload_cancel_sz	(compound_decode_hdr_maxsz + \
+					 decode_putfh_maxsz + \
+					 decode_offload_cancel_maxsz)
 #define NFS4_enc_deallocate_sz		(compound_encode_hdr_maxsz + \
 					 encode_putfh_maxsz + \
 					 encode_deallocate_maxsz + \
@@ -163,6 +172,14 @@ static void encode_offload_status(struct xdr_stream *xdr,
 	encode_nfs4_stateid(xdr, &args->osa_stateid);
 }
 
+static void encode_offload_cancel(struct xdr_stream *xdr,
+				  const struct nfs42_offload_status_args *args,
+				  struct compound_hdr *hdr)
+{
+	encode_op_hdr(xdr, OP_OFFLOAD_CANCEL, decode_offload_cancel_maxsz, hdr);
+	encode_nfs4_stateid(xdr, &args->osa_stateid);
+}
+
 static void encode_deallocate(struct xdr_stream *xdr,
 			      const struct nfs42_falloc_args *args,
 			      struct compound_hdr *hdr)
@@ -298,6 +315,25 @@ static void nfs4_xdr_enc_offload_status(struct rpc_rqst *req,
 }
 
 /*
+ * Encode OFFLOAD_CANEL request
+ */
+static void nfs4_xdr_enc_offload_cancel(struct rpc_rqst *req,
+					struct xdr_stream *xdr,
+					const void *data)
+{
+	const struct nfs42_offload_status_args *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->osa_seq_args),
+	};
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->osa_seq_args, &hdr);
+	encode_putfh(xdr, args->osa_src_fh, &hdr);
+	encode_offload_cancel(xdr, args, &hdr);
+	encode_nops(&hdr);
+}
+
+/*
  * Encode DEALLOCATE request
  */
 static void nfs4_xdr_enc_deallocate(struct rpc_rqst *req,
@@ -475,6 +511,12 @@ static int decode_offload_status(struct xdr_stream *xdr,
 	return -EIO;
 }
 
+static int decode_offload_cancel(struct xdr_stream *xdr,
+				 struct nfs42_offload_status_res *res)
+{
+	return decode_op_hdr(xdr, OP_OFFLOAD_CANCEL);
+}
+
 static int decode_deallocate(struct xdr_stream *xdr, struct nfs42_falloc_res *res)
 {
 	return decode_op_hdr(xdr, OP_DEALLOCATE);
@@ -601,6 +643,32 @@ static int nfs4_xdr_dec_offload_status(struct rpc_rqst *rqstp,
 }
 
 /*
+ * Decode OFFLOAD_CANCEL response
+ */
+static int nfs4_xdr_dec_offload_cancel(struct rpc_rqst *rqstp,
+				       struct xdr_stream *xdr,
+				       void *data)
+{
+	struct nfs42_offload_status_res *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->osr_seq_res, rqstp);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+	status = decode_offload_cancel(xdr, res);
+
+out:
+	return status;
+}
+
+/*
  * Decode DEALLOCATE request
  */
 static int nfs4_xdr_dec_deallocate(struct rpc_rqst *rqstp,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7d59593..3926f11 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -9296,6 +9296,7 @@ static bool nfs4_match_stateid(const nfs4_stateid *s1,
 		| NFS_CAP_ALLOCATE
 		| NFS_CAP_COPY
 		| NFS_CAP_OFFLOAD_STATUS
+		| NFS_CAP_OFFLOAD_CANCEL
 		| NFS_CAP_DEALLOCATE
 		| NFS_CAP_SEEK
 		| NFS_CAP_LAYOUTSTATS
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 5406f1d..e45f74b 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -7734,6 +7734,7 @@ int nfs4_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
 	PROC(CLONE,		enc_clone,		dec_clone),
 	PROC(COPY,		enc_copy,		dec_copy),
 	PROC(OFFLOAD_STATUS,	enc_offload_status,	dec_offload_status),
+	PROC(OFFLOAD_CANCEL,    enc_offload_cancel,     dec_offload_cancel),
 #endif /* CONFIG_NFS_V4_2 */
 };
 
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 0f37785..fde1670 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -525,6 +525,7 @@ enum {
 	NFSPROC4_CLNT_CLONE,
 	NFSPROC4_CLNT_COPY,
 	NFSPROC4_CLNT_OFFLOAD_STATUS,
+	NFSPROC4_CLNT_OFFLOAD_CANCEL,
 };
 
 /* nfs41 types */
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index fb4e2b8..de1eafe 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -253,5 +253,6 @@ struct nfs_server {
 #define NFS_CAP_CLONE		(1U << 23)
 #define NFS_CAP_COPY		(1U << 24)
 #define NFS_CAP_OFFLOAD_STATUS	(1U << 25)
+#define NFS_CAP_OFFLOAD_CANCEL	(1U << 26)
 
 #endif
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 06/12] NFS COPY xdr handle async reply
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (4 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 05/12] NFS OFFLOAD_CANCEL xdr Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 07/12] NFS add support for asynchronous COPY Olga Kornievskaia
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

If server returns async reply, it must include a callback stateid,
wr_callback_id in the write_response4.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs42xdr.c       | 22 ++++++++++++----------
 include/linux/nfs_xdr.h |  1 +
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index 940621a..9e557ad 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -426,21 +426,23 @@ static int decode_write_response(struct xdr_stream *xdr,
 				 struct nfs42_write_res *res)
 {
 	__be32 *p;
+	int status, count;
 
-	p = xdr_inline_decode(xdr, 4 + 8 + 4);
+	p = xdr_inline_decode(xdr, 4);
 	if (unlikely(!p))
 		goto out_overflow;
-
-	/*
-	 * We never use asynchronous mode, so warn if a server returns
-	 * a stateid.
-	 */
-	if (unlikely(*p != 0)) {
-		pr_err_once("%s: server has set unrequested "
-				"asynchronous mode\n", __func__);
+	count = be32_to_cpup(p);
+	if (count > 1)
 		return -EREMOTEIO;
+	else if (count == 1) {
+		status = decode_opaque_fixed(xdr, &res->stateid,
+				NFS4_STATEID_SIZE);
+		if (unlikely(status))
+			goto out_overflow;
 	}
-	p++;
+	p = xdr_inline_decode(xdr, 8 + 4);
+	if (unlikely(!p))
+		goto out_overflow;
 	p = xdr_decode_hyper(p, &res->count);
 	res->verifier.committed = be32_to_cpup(p);
 	return decode_verifier(xdr, &res->verifier.verifier);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 5ffd745..784ec8d 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1387,6 +1387,7 @@ struct nfs42_copy_args {
 };
 
 struct nfs42_write_res {
+	nfs4_stateid		stateid;
 	u64			count;
 	struct nfs_writeverf	verifier;
 };
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 07/12] NFS add support for asynchronous COPY
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (5 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 06/12] NFS COPY xdr handle async reply Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 19:33   ` Anna Schumaker
  2017-09-28 17:28 ` [PATCH v4 08/12] NFS handle COPY reply CB_OFFLOAD call race Olga Kornievskaia
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Change xdr to always send COPY asynchronously.

Keep the list copies send in a list under a server structure.
Once copy is sent, it waits on a completion structure that will
be signalled by the callback thread that receives CB_OFFLOAD.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/callback_proc.c    | 38 ++++++++++++++++++++++++++++++-
 fs/nfs/client.c           |  1 +
 fs/nfs/nfs42proc.c        | 57 ++++++++++++++++++++++++++++++++++++++++++-----
 fs/nfs/nfs42xdr.c         |  8 ++++---
 include/linux/nfs_fs.h    |  9 ++++++++
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   |  1 +
 7 files changed, 106 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index eef2728..d3e7b61 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -657,9 +657,45 @@ __be32 nfs4_callback_notify_lock(void *argp, void *resp,
 }
 #endif /* CONFIG_NFS_V4_1 */
 #ifdef CONFIG_NFS_V4_2
-__be32 nfs4_callback_offload(void *args, void *dummy,
+static void nfs4_copy_cb_args(struct nfs4_copy_state *cp_state,
+				struct cb_offloadargs *args)
+{
+	cp_state->count = args->wr_count;
+	cp_state->error = args->error;
+	if (!args->error) {
+		cp_state->verf.committed = args->wr_writeverf.committed;
+		memcpy(&cp_state->verf.verifier.data[0],
+			&args->wr_writeverf.verifier.data[0],
+			NFS4_VERIFIER_SIZE);
+	}
+}
+
+__be32 nfs4_callback_offload(void *data, void *dummy,
 			     struct cb_process_state *cps)
 {
+	struct cb_offloadargs *args = data;
+	struct nfs_server *server;
+	struct nfs4_copy_state *copy;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
+				client_link) {
+		spin_lock(&server->nfs_client->cl_lock);
+		list_for_each_entry(copy, &server->ss_copies, copies) {
+			if (memcmp(args->coa_stateid.other,
+					copy->stateid.other,
+					sizeof(args->coa_stateid.other)))
+				continue;
+			nfs4_copy_cb_args(copy, args);
+			complete(&copy->completion);
+			spin_unlock(&server->nfs_client->cl_lock);
+			goto out;
+		}
+		spin_unlock(&server->nfs_client->cl_lock);
+	}
+out:
+	rcu_read_unlock();
+
 	return 0;
 }
 #endif /* CONFIG_NFS_V4_2 */
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index efebe6c..7b338be 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -876,6 +876,7 @@ struct nfs_server *nfs_alloc_server(void)
 	INIT_LIST_HEAD(&server->delegations);
 	INIT_LIST_HEAD(&server->layouts);
 	INIT_LIST_HEAD(&server->state_owners_lru);
+	INIT_LIST_HEAD(&server->ss_copies);
 
 	atomic_set(&server->active, 0);
 
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 5c42e71..c8ca353 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -129,6 +129,39 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
 	return err;
 }
 
+static int handle_async_copy(struct nfs42_copy_res *res,
+			     struct nfs_server *server,
+			     struct file *src,
+			     struct file *dst,
+			     nfs4_stateid *src_stateid,
+			     uint64_t *ret_count)
+{
+	struct nfs4_copy_state *copy;
+	int status;
+
+	copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
+	if (!copy)
+		return -ENOMEM;
+	memcpy(&copy->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
+	init_completion(&copy->completion);
+
+	spin_lock(&server->nfs_client->cl_lock);
+	list_add_tail(&copy->copies, &server->ss_copies);
+	spin_unlock(&server->nfs_client->cl_lock);
+
+	wait_for_completion_interruptible(&copy->completion);
+	spin_lock(&server->nfs_client->cl_lock);
+	list_del_init(&copy->copies);
+	spin_unlock(&server->nfs_client->cl_lock);
+	*ret_count = copy->count;
+	memcpy(&res->write_res.verifier, &copy->verf, sizeof(copy->verf));
+	if (copy->count <= 0)
+		status = -copy->error;
+
+	kfree(copy);
+	return status;
+}
+
 static ssize_t _nfs42_proc_copy(struct file *src,
 				struct nfs_lock_context *src_lock,
 				struct file *dst,
@@ -167,9 +200,13 @@ static ssize_t _nfs42_proc_copy(struct file *src,
 	if (status)
 		return status;
 
-	res->commit_res.verf = kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
-	if (!res->commit_res.verf)
-		return -ENOMEM;
+	res->commit_res.verf = NULL;
+	if (args->sync) {
+		res->commit_res.verf =
+			kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
+		if (!res->commit_res.verf)
+			return -ENOMEM;
+	}
 	status = nfs4_call_sync(server->client, server, &msg,
 				&args->seq_args, &res->seq_res, 0);
 	if (status == -ENOTSUPP)
@@ -177,18 +214,27 @@ static ssize_t _nfs42_proc_copy(struct file *src,
 	if (status)
 		goto out;
 
-	if (nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
+	if (args->sync &&
+		!nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
 				    &res->commit_res.verf->verifier)) {
 		status = -EAGAIN;
 		goto out;
 	}
 
+	if (!res->synchronous) {
+		status = handle_async_copy(res, server, src, dst,
+				&args->src_stateid, &res->write_res.count);
+		if (status)
+			return status;
+	}
+
 	truncate_pagecache_range(dst_inode, pos_dst,
 				 pos_dst + res->write_res.count);
 
 	status = res->write_res.count;
 out:
-	kfree(res->commit_res.verf);
+	if (args->sync)
+		kfree(res->commit_res.verf);
 	return status;
 }
 
@@ -205,6 +251,7 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
 		.dst_fh		= NFS_FH(file_inode(dst)),
 		.dst_pos	= pos_dst,
 		.count		= count,
+		.sync		= false,
 	};
 	struct nfs42_copy_res res;
 	struct nfs4_exception src_exception = {
diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index 9e557ad..2c37c66 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -160,7 +160,7 @@ static void encode_copy(struct xdr_stream *xdr,
 	encode_uint64(xdr, args->count);
 
 	encode_uint32(xdr, 1); /* consecutive = true */
-	encode_uint32(xdr, 1); /* synchronous = true */
+	encode_uint32(xdr, args->sync);
 	encode_uint32(xdr, 0); /* src server list */
 }
 
@@ -291,7 +291,8 @@ static void nfs4_xdr_enc_copy(struct rpc_rqst *req,
 	encode_savefh(xdr, &hdr);
 	encode_putfh(xdr, args->dst_fh, &hdr);
 	encode_copy(xdr, args, &hdr);
-	encode_copy_commit(xdr, args, &hdr);
+	if (args->sync)
+		encode_copy_commit(xdr, args, &hdr);
 	encode_nops(&hdr);
 }
 
@@ -613,7 +614,8 @@ static int nfs4_xdr_dec_copy(struct rpc_rqst *rqstp,
 	status = decode_copy(xdr, res);
 	if (status)
 		goto out;
-	status = decode_commit(xdr, &res->commit_res);
+	if (res->commit_res.verf)
+		status = decode_commit(xdr, &res->commit_res);
 out:
 	return status;
 }
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index a0282ce..580a8d9 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -183,6 +183,15 @@ struct nfs_inode {
 	struct inode		vfs_inode;
 };
 
+struct nfs4_copy_state {
+	struct list_head	copies;
+	nfs4_stateid		stateid;
+	struct completion	completion;
+	uint64_t		count;
+	struct nfs_writeverf	verf;
+	int			error;
+};
+
 /*
  * Cache validity bit flags
  */
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index de1eafe..511eefb 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -206,6 +206,7 @@ struct nfs_server {
 	struct list_head	state_owners_lru;
 	struct list_head	layouts;
 	struct list_head	delegations;
+	struct list_head	ss_copies;
 
 	unsigned long		mig_gen;
 	unsigned long		mig_status;
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 784ec8d..020d958 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1384,6 +1384,7 @@ struct nfs42_copy_args {
 	u64				dst_pos;
 
 	u64				count;
+	bool				sync;
 };
 
 struct nfs42_write_res {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 08/12] NFS handle COPY reply CB_OFFLOAD call race
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (6 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 07/12] NFS add support for asynchronous COPY Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 19:50   ` Anna Schumaker
  2017-09-28 17:28 ` [PATCH v4 09/12] NFS export nfs4_async_handle_error Olga Kornievskaia
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

It's possible that server replies back with CB_OFFLOAD call and
COPY reply at the same time such that client will process
CB_OFFLOAD before reply to COPY. For that keep a list of pending
callback stateids received and them before waiting on completion
check the pending list.

Cleanup any pending copies on the client shutdown.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/callback_proc.c    | 17 ++++++++++++++---
 fs/nfs/nfs42proc.c        | 24 +++++++++++++++++++++---
 fs/nfs/nfs4client.c       | 15 +++++++++++++++
 include/linux/nfs_fs_sb.h |  1 +
 4 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index d3e7b61..d546c9f 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -676,11 +676,12 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
 	struct cb_offloadargs *args = data;
 	struct nfs_server *server;
 	struct nfs4_copy_state *copy;
+	bool found = false;
 
+	spin_lock(&cps->clp->cl_lock);
 	rcu_read_lock();
 	list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
 				client_link) {
-		spin_lock(&server->nfs_client->cl_lock);
 		list_for_each_entry(copy, &server->ss_copies, copies) {
 			if (memcmp(args->coa_stateid.other,
 					copy->stateid.other,
@@ -688,13 +689,23 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
 				continue;
 			nfs4_copy_cb_args(copy, args);
 			complete(&copy->completion);
-			spin_unlock(&server->nfs_client->cl_lock);
+			found = true;
 			goto out;
 		}
-		spin_unlock(&server->nfs_client->cl_lock);
 	}
 out:
 	rcu_read_unlock();
+	if (!found) {
+		copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
+		if (!copy) {
+			spin_unlock(&cps->clp->cl_lock);
+			return -ENOMEM;
+		}
+		memcpy(&copy->stateid, &args->coa_stateid, NFS4_STATEID_SIZE);
+		nfs4_copy_cb_args(copy, args);
+		list_add_tail(&copy->copies, &cps->clp->pending_cb_stateids);
+	}
+	spin_unlock(&cps->clp->cl_lock);
 
 	return 0;
 }
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index c8ca353..d0f454e 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -137,15 +137,32 @@ static int handle_async_copy(struct nfs42_copy_res *res,
 			     uint64_t *ret_count)
 {
 	struct nfs4_copy_state *copy;
-	int status;
+	int status = NFS4_OK;
+	bool found_pending = false;
+
+	spin_lock(&server->nfs_client->cl_lock);
+	list_for_each_entry(copy, &server->nfs_client->pending_cb_stateids,
+				copies) {
+		if (memcmp(&res->write_res.stateid, &copy->stateid,
+				NFS4_STATEID_SIZE))
+			continue;
+		found_pending = true;
+		list_del(&copy->copies);
+		break;
+	}
+	if (found_pending) {
+		spin_unlock(&server->nfs_client->cl_lock);
+		goto out;
+	}
 
 	copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
-	if (!copy)
+	if (!copy) {
+		spin_unlock(&server->nfs_client->cl_lock);
 		return -ENOMEM;
+	}
 	memcpy(&copy->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
 	init_completion(&copy->completion);
 
-	spin_lock(&server->nfs_client->cl_lock);
 	list_add_tail(&copy->copies, &server->ss_copies);
 	spin_unlock(&server->nfs_client->cl_lock);
 
@@ -153,6 +170,7 @@ static int handle_async_copy(struct nfs42_copy_res *res,
 	spin_lock(&server->nfs_client->cl_lock);
 	list_del_init(&copy->copies);
 	spin_unlock(&server->nfs_client->cl_lock);
+out:
 	*ret_count = copy->count;
 	memcpy(&res->write_res.verifier, &copy->verf, sizeof(copy->verf));
 	if (copy->count <= 0)
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index e9bea90..46d4649 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -156,9 +156,23 @@ struct rpc_clnt *
 	}
 }
 
+static void
+nfs4_cleanup_callback(struct nfs_client *clp)
+{
+	struct nfs4_copy_state *cp_state;
+
+	while (!list_empty(&clp->pending_cb_stateids)) {
+		cp_state = list_entry(clp->pending_cb_stateids.next,
+					struct nfs4_copy_state, copies);
+		list_del(&cp_state->copies);
+		kfree(cp_state);
+	}
+}
+
 void nfs41_shutdown_client(struct nfs_client *clp)
 {
 	if (nfs4_has_session(clp)) {
+		nfs4_cleanup_callback(clp);
 		nfs4_shutdown_ds_clients(clp);
 		nfs4_destroy_session(clp->cl_session);
 		nfs4_destroy_clientid(clp);
@@ -202,6 +216,7 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
 #if IS_ENABLED(CONFIG_NFS_V4_1)
 	init_waitqueue_head(&clp->cl_lock_waitq);
 #endif
+	INIT_LIST_HEAD(&clp->pending_cb_stateids);
 	return clp;
 
 error:
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 511eefb..72f159e 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -119,6 +119,7 @@ struct nfs_client {
 #endif
 
 	struct net		*cl_net;
+	struct list_head	pending_cb_stateids;
 };
 
 /*
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 09/12] NFS export nfs4_async_handle_error
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (7 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 08/12] NFS handle COPY reply CB_OFFLOAD call race Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 10/12] NFS send OFFLOAD_CANCEL when COPY killed Olga Kornievskaia
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Make this function available to nfs42proc.c

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4_fs.h  | 3 +++
 fs/nfs/nfs4proc.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index ac4f10b..c7225bb 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -244,6 +244,9 @@ int nfs4_replace_transport(struct nfs_server *server,
 
 /* nfs4proc.c */
 extern int nfs4_handle_exception(struct nfs_server *, int, struct nfs4_exception *);
+extern int nfs4_async_handle_error(struct rpc_task *task,
+				   struct nfs_server *server,
+				   struct nfs4_state *state, long *timeout);
 extern int nfs4_call_sync(struct rpc_clnt *, struct nfs_server *,
 			  struct rpc_message *, struct nfs4_sequence_args *,
 			  struct nfs4_sequence_res *, int);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 3926f11..f1bf19e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -551,7 +551,7 @@ int nfs4_handle_exception(struct nfs_server *server, int errorcode, struct nfs4_
 	return ret;
 }
 
-static int
+int
 nfs4_async_handle_error(struct rpc_task *task, struct nfs_server *server,
 			struct nfs4_state *state, long *timeout)
 {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 10/12] NFS send OFFLOAD_CANCEL when COPY killed
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (8 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 09/12] NFS export nfs4_async_handle_error Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 11/12] NFS handle COPY ERR_OFFLOAD_NO_REQS Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY Olga Kornievskaia
  11 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

When COPY is killed by the user send OFFLOAD_CANCEL to server
processing the copy.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs42proc.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index d0f454e..f2ba4c4 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -16,6 +16,7 @@
 #include "internal.h"
 
 #define NFSDBG_FACILITY NFSDBG_PROC
+static int nfs42_do_offload_cancel_async(struct file *dst, nfs4_stateid *std);
 
 static int _nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
 		struct nfs_lock_context *lock, loff_t offset, loff_t len)
@@ -166,10 +167,15 @@ static int handle_async_copy(struct nfs42_copy_res *res,
 	list_add_tail(&copy->copies, &server->ss_copies);
 	spin_unlock(&server->nfs_client->cl_lock);
 
-	wait_for_completion_interruptible(&copy->completion);
+	status = wait_for_completion_interruptible(&copy->completion);
 	spin_lock(&server->nfs_client->cl_lock);
 	list_del_init(&copy->copies);
 	spin_unlock(&server->nfs_client->cl_lock);
+	if (status == -ERESTARTSYS) {
+		nfs42_do_offload_cancel_async(dst, &copy->stateid);
+		kfree(copy);
+		return status;
+	}
 out:
 	*ret_count = copy->count;
 	memcpy(&res->write_res.verifier, &copy->verf, sizeof(copy->verf));
@@ -328,6 +334,88 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
 	return err;
 }
 
+struct nfs42_offloadcancel_data {
+	struct nfs_server *seq_server;
+	struct nfs42_offload_status_args args;
+	struct nfs42_offload_status_res res;
+};
+
+static void nfs42_offload_cancel_prepare(struct rpc_task *task, void *calldata)
+{
+	struct nfs42_offloadcancel_data *data = calldata;
+
+	nfs4_setup_sequence(data->seq_server->nfs_client,
+				&data->args.osa_seq_args,
+				&data->res.osr_seq_res, task);
+}
+
+static void nfs42_offload_cancel_done(struct rpc_task *task, void *calldata)
+{
+	struct nfs42_offloadcancel_data *data = calldata;
+
+	nfs41_sequence_done(task, &data->res.osr_seq_res);
+	if (task->tk_status &&
+		nfs4_async_handle_error(task, data->seq_server, NULL,
+			NULL) == -EAGAIN)
+		rpc_restart_call_prepare(task);
+}
+
+static void nfs42_free_offloadcancel_data(void *data)
+{
+	kfree(data);
+}
+
+static const struct rpc_call_ops nfs42_offload_cancel_ops = {
+	.rpc_call_prepare = nfs42_offload_cancel_prepare,
+	.rpc_call_done = nfs42_offload_cancel_done,
+	.rpc_release = nfs42_free_offloadcancel_data,
+};
+
+static int nfs42_do_offload_cancel_async(struct file *dst,
+				nfs4_stateid *stateid)
+{
+	struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
+	struct nfs42_offloadcancel_data *data = NULL;
+	struct nfs_open_context *ctx = nfs_file_open_context(dst);
+	struct rpc_task *task;
+	struct rpc_message msg = {
+		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OFFLOAD_CANCEL],
+		.rpc_cred = ctx->cred,
+	};
+	struct rpc_task_setup task_setup_data = {
+		.rpc_client = dst_server->client,
+		.rpc_message = &msg,
+		.callback_ops = &nfs42_offload_cancel_ops,
+		.workqueue = nfsiod_workqueue,
+		.flags = RPC_TASK_ASYNC,
+	};
+	int status;
+
+	if (!(dst_server->caps & NFS_CAP_OFFLOAD_CANCEL))
+		return -EOPNOTSUPP;
+
+	data = kzalloc(sizeof(struct nfs42_offloadcancel_data), GFP_NOFS);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->seq_server = dst_server;
+	data->args.osa_src_fh = NFS_FH(file_inode(dst));
+	memcpy(&data->args.osa_stateid, stateid,
+		sizeof(data->args.osa_stateid));
+	msg.rpc_argp = &data->args;
+	msg.rpc_resp = &data->res;
+	task_setup_data.callback_data = data;
+	nfs4_init_sequence(&data->args.osa_seq_args, &data->res.osr_seq_res, 1);
+	task = rpc_run_task(&task_setup_data);
+	if (IS_ERR(task))
+		return PTR_ERR(task);
+	status = rpc_wait_for_completion_task(task);
+	if (status == -ENOTSUPP)
+		dst_server->caps &= ~NFS_CAP_OFFLOAD_CANCEL;
+	rpc_put_task(task);
+	return status;
+}
+
 int _nfs42_proc_offload_status(struct file *dst, nfs4_stateid *stateid,
 				struct nfs42_offload_status_res *res)
 {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 11/12] NFS handle COPY ERR_OFFLOAD_NO_REQS
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (9 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 10/12] NFS send OFFLOAD_CANCEL when COPY killed Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 17:28 ` [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY Olga Kornievskaia
  11 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

If client sent async COPY and server replied with
ERR_OFFLOAD_NO_REQS, client should retry with a synchronous copy.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs42proc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index f2ba4c4..b9e47a2 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -317,7 +317,11 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
 		if (err == -ENOTSUPP) {
 			err = -EOPNOTSUPP;
 			break;
-		} if (err == -EAGAIN) {
+		} else if (err == -EAGAIN) {
+			dst_exception.retry = 1;
+			continue;
+		} else if (err == -NFS4ERR_OFFLOAD_NO_REQS && !args.sync) {
+			args.sync = true;
 			dst_exception.retry = 1;
 			continue;
 		}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY
  2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
                   ` (10 preceding siblings ...)
  2017-09-28 17:28 ` [PATCH v4 11/12] NFS handle COPY ERR_OFFLOAD_NO_REQS Olga Kornievskaia
@ 2017-09-28 17:28 ` Olga Kornievskaia
  2017-09-28 20:13   ` Anna Schumaker
  11 siblings, 1 reply; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 17:28 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

A COPY with unstable write data needs a simple commit that doesn't
deal with inodes

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs42proc.c | 22 ++++++++++++++++++++++
 fs/nfs/nfs4_fs.h   |  2 +-
 fs/nfs/nfs4proc.c  | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index b9e47a2..2064d11 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -252,6 +252,28 @@ static ssize_t _nfs42_proc_copy(struct file *src,
 			return status;
 	}
 
+	if ((!res->synchronous || !args->sync) &&
+			res->write_res.verifier.committed != NFS_FILE_SYNC) {
+		struct nfs_commitres cres;
+
+		cres.verf = kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
+		if (!cres.verf)
+			return -ENOMEM;
+
+		status = nfs4_proc_commit(dst, pos_dst, res->write_res.count,
+					  &cres);
+		if (status) {
+			kfree(cres.verf);
+			return status;
+		}
+		if (!nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
+					    &cres.verf->verifier)) {
+			/* what are we suppose to do here ? */
+			dprintk("commit verf differs from copy verf\n");
+		}
+		kfree(cres.verf);
+	}
+
 	truncate_pagecache_range(dst_inode, pos_dst,
 				 pos_dst + res->write_res.count);
 
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index c7225bb..5edb161 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -475,7 +475,7 @@ extern int nfs4_sequence_done(struct rpc_task *task,
 			      struct nfs4_sequence_res *res);
 
 extern void nfs4_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp);
-
+extern int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res);
 extern const nfs4_stateid zero_stateid;
 
 /* nfs4super.c */
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index f1bf19e..30829ce 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4829,6 +4829,39 @@ static void nfs4_proc_commit_setup(struct nfs_commit_data *data, struct rpc_mess
 	nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
 }
 
+static int _nfs4_proc_commit(struct file *dst, struct nfs_commitargs *args,
+				struct nfs_commitres *res)
+{
+	struct inode *dst_inode = file_inode(dst);
+	struct nfs_server *server = NFS_SERVER(dst_inode);
+	struct rpc_message msg = {
+		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_COMMIT],
+		.rpc_argp = args,
+		.rpc_resp = res,
+	};
+	return nfs4_call_sync(server->client, server, &msg,
+			&args->seq_args, &res->seq_res, 1);
+}
+
+int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res)
+{
+	struct nfs_commitargs args = {
+		.fh = NFS_FH(file_inode(dst)),
+		.offset = offset,
+		.count = count,
+	};
+	struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
+	struct nfs4_exception exception = { };
+	int status;
+
+	do {
+		status = _nfs4_proc_commit(dst, &args, res);
+		status = nfs4_handle_exception(dst_server, status, &exception);
+	} while (exception.retry);
+
+	return status;
+}
+
 struct nfs4_renewdata {
 	struct nfs_client	*client;
 	unsigned long		timestamp;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 07/12] NFS add support for asynchronous COPY
  2017-09-28 17:28 ` [PATCH v4 07/12] NFS add support for asynchronous COPY Olga Kornievskaia
@ 2017-09-28 19:33   ` Anna Schumaker
  2017-09-28 19:36     ` Olga Kornievskaia
  0 siblings, 1 reply; 24+ messages in thread
From: Anna Schumaker @ 2017-09-28 19:33 UTC (permalink / raw)
  To: Olga Kornievskaia, Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Hi Olga,

On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
> Change xdr to always send COPY asynchronously.
> 
> Keep the list copies send in a list under a server structure.
> Once copy is sent, it waits on a completion structure that will
> be signalled by the callback thread that receives CB_OFFLOAD.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/callback_proc.c    | 38 ++++++++++++++++++++++++++++++-
>  fs/nfs/client.c           |  1 +
>  fs/nfs/nfs42proc.c        | 57 ++++++++++++++++++++++++++++++++++++++++++-----
>  fs/nfs/nfs42xdr.c         |  8 ++++---
>  include/linux/nfs_fs.h    |  9 ++++++++
>  include/linux/nfs_fs_sb.h |  1 +
>  include/linux/nfs_xdr.h   |  1 +
>  7 files changed, 106 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index eef2728..d3e7b61 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -657,9 +657,45 @@ __be32 nfs4_callback_notify_lock(void *argp, void *resp,
>  }
>  #endif /* CONFIG_NFS_V4_1 */
>  #ifdef CONFIG_NFS_V4_2
> -__be32 nfs4_callback_offload(void *args, void *dummy,
> +static void nfs4_copy_cb_args(struct nfs4_copy_state *cp_state,
> +				struct cb_offloadargs *args)
> +{
> +	cp_state->count = args->wr_count;
> +	cp_state->error = args->error;
> +	if (!args->error) {
> +		cp_state->verf.committed = args->wr_writeverf.committed;
> +		memcpy(&cp_state->verf.verifier.data[0],
> +			&args->wr_writeverf.verifier.data[0],
> +			NFS4_VERIFIER_SIZE);
> +	}
> +}
> +
> +__be32 nfs4_callback_offload(void *data, void *dummy,
>  			     struct cb_process_state *cps)
>  {
> +	struct cb_offloadargs *args = data;
> +	struct nfs_server *server;
> +	struct nfs4_copy_state *copy;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
> +				client_link) {
> +		spin_lock(&server->nfs_client->cl_lock);
> +		list_for_each_entry(copy, &server->ss_copies, copies) {
> +			if (memcmp(args->coa_stateid.other,
> +					copy->stateid.other,
> +					sizeof(args->coa_stateid.other)))
> +				continue;
> +			nfs4_copy_cb_args(copy, args);
> +			complete(&copy->completion);
> +			spin_unlock(&server->nfs_client->cl_lock);
> +			goto out;
> +		}
> +		spin_unlock(&server->nfs_client->cl_lock);
> +	}
> +out:
> +	rcu_read_unlock();
> +
>  	return 0;
>  }
>  #endif /* CONFIG_NFS_V4_2 */
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index efebe6c..7b338be 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -876,6 +876,7 @@ struct nfs_server *nfs_alloc_server(void)
>  	INIT_LIST_HEAD(&server->delegations);
>  	INIT_LIST_HEAD(&server->layouts);
>  	INIT_LIST_HEAD(&server->state_owners_lru);
> +	INIT_LIST_HEAD(&server->ss_copies);
>  
>  	atomic_set(&server->active, 0);
>  
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 5c42e71..c8ca353 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -129,6 +129,39 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
>  	return err;
>  }
>  
> +static int handle_async_copy(struct nfs42_copy_res *res,
> +			     struct nfs_server *server,
> +			     struct file *src,
> +			     struct file *dst,
> +			     nfs4_stateid *src_stateid,
> +			     uint64_t *ret_count)
> +{
> +	struct nfs4_copy_state *copy;
> +	int status;> +
> +	copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
> +	if (!copy)
> +		return -ENOMEM;
> +	memcpy(&copy->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
> +	init_completion(&copy->completion);
> +
> +	spin_lock(&server->nfs_client->cl_lock);
> +	list_add_tail(&copy->copies, &server->ss_copies);
> +	spin_unlock(&server->nfs_client->cl_lock);
> +
> +	wait_for_completion_interruptible(&copy->completion);
> +	spin_lock(&server->nfs_client->cl_lock);
> +	list_del_init(&copy->copies);
> +	spin_unlock(&server->nfs_client->cl_lock);
> +	*ret_count = copy->count;
> +	memcpy(&res->write_res.verifier, &copy->verf, sizeof(copy->verf));
> +	if (copy->count <= 0)
> +		status = -copy->error;

If copy->count > 0 then status won't be set before this function returns.  GCC complains about this when I try compiling.

Anna

> +
> +	kfree(copy);
> +	return status;
> +}
> +
>  static ssize_t _nfs42_proc_copy(struct file *src,
>  				struct nfs_lock_context *src_lock,
>  				struct file *dst,
> @@ -167,9 +200,13 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>  	if (status)
>  		return status;
>  
> -	res->commit_res.verf = kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
> -	if (!res->commit_res.verf)
> -		return -ENOMEM;
> +	res->commit_res.verf = NULL;
> +	if (args->sync) {
> +		res->commit_res.verf =
> +			kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
> +		if (!res->commit_res.verf)
> +			return -ENOMEM;
> +	}
>  	status = nfs4_call_sync(server->client, server, &msg,
>  				&args->seq_args, &res->seq_res, 0);
>  	if (status == -ENOTSUPP)
> @@ -177,18 +214,27 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>  	if (status)
>  		goto out;
>  
> -	if (nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
> +	if (args->sync &&
> +		!nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
>  				    &res->commit_res.verf->verifier)) {
>  		status = -EAGAIN;
>  		goto out;
>  	}
>  
> +	if (!res->synchronous) {
> +		status = handle_async_copy(res, server, src, dst,
> +				&args->src_stateid, &res->write_res.count);
> +		if (status)
> +			return status;
> +	}
> +
>  	truncate_pagecache_range(dst_inode, pos_dst,
>  				 pos_dst + res->write_res.count);
>  
>  	status = res->write_res.count;
>  out:
> -	kfree(res->commit_res.verf);
> +	if (args->sync)
> +		kfree(res->commit_res.verf);
>  	return status;
>  }
>  
> @@ -205,6 +251,7 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
>  		.dst_fh		= NFS_FH(file_inode(dst)),
>  		.dst_pos	= pos_dst,
>  		.count		= count,
> +		.sync		= false,
>  	};
>  	struct nfs42_copy_res res;
>  	struct nfs4_exception src_exception = {
> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> index 9e557ad..2c37c66 100644
> --- a/fs/nfs/nfs42xdr.c
> +++ b/fs/nfs/nfs42xdr.c
> @@ -160,7 +160,7 @@ static void encode_copy(struct xdr_stream *xdr,
>  	encode_uint64(xdr, args->count);
>  
>  	encode_uint32(xdr, 1); /* consecutive = true */
> -	encode_uint32(xdr, 1); /* synchronous = true */
> +	encode_uint32(xdr, args->sync);
>  	encode_uint32(xdr, 0); /* src server list */
>  }
>  
> @@ -291,7 +291,8 @@ static void nfs4_xdr_enc_copy(struct rpc_rqst *req,
>  	encode_savefh(xdr, &hdr);
>  	encode_putfh(xdr, args->dst_fh, &hdr);
>  	encode_copy(xdr, args, &hdr);
> -	encode_copy_commit(xdr, args, &hdr);
> +	if (args->sync)
> +		encode_copy_commit(xdr, args, &hdr);
>  	encode_nops(&hdr);
>  }
>  
> @@ -613,7 +614,8 @@ static int nfs4_xdr_dec_copy(struct rpc_rqst *rqstp,
>  	status = decode_copy(xdr, res);
>  	if (status)
>  		goto out;
> -	status = decode_commit(xdr, &res->commit_res);
> +	if (res->commit_res.verf)
> +		status = decode_commit(xdr, &res->commit_res);
>  out:
>  	return status;
>  }
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index a0282ce..580a8d9 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -183,6 +183,15 @@ struct nfs_inode {
>  	struct inode		vfs_inode;
>  };
>  
> +struct nfs4_copy_state {
> +	struct list_head	copies;
> +	nfs4_stateid		stateid;
> +	struct completion	completion;
> +	uint64_t		count;
> +	struct nfs_writeverf	verf;
> +	int			error;
> +};
> +
>  /*
>   * Cache validity bit flags
>   */
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index de1eafe..511eefb 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -206,6 +206,7 @@ struct nfs_server {
>  	struct list_head	state_owners_lru;
>  	struct list_head	layouts;
>  	struct list_head	delegations;
> +	struct list_head	ss_copies;
>  
>  	unsigned long		mig_gen;
>  	unsigned long		mig_status;
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 784ec8d..020d958 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1384,6 +1384,7 @@ struct nfs42_copy_args {
>  	u64				dst_pos;
>  
>  	u64				count;
> +	bool				sync;
>  };
>  
>  struct nfs42_write_res {
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 07/12] NFS add support for asynchronous COPY
  2017-09-28 19:33   ` Anna Schumaker
@ 2017-09-28 19:36     ` Olga Kornievskaia
  0 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 19:36 UTC (permalink / raw)
  To: Anna Schumaker
  Cc: Olga Kornievskaia, Trond Myklebust, Anna Schumaker, linux-nfs

On Thu, Sep 28, 2017 at 3:33 PM, Anna Schumaker
<schumaker.anna@gmail.com> wrote:
> Hi Olga,
>
> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>> Change xdr to always send COPY asynchronously.
>>
>> Keep the list copies send in a list under a server structure.
>> Once copy is sent, it waits on a completion structure that will
>> be signalled by the callback thread that receives CB_OFFLOAD.
>>
>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>> ---
>>  fs/nfs/callback_proc.c    | 38 ++++++++++++++++++++++++++++++-
>>  fs/nfs/client.c           |  1 +
>>  fs/nfs/nfs42proc.c        | 57 ++++++++++++++++++++++++++++++++++++++++++-----
>>  fs/nfs/nfs42xdr.c         |  8 ++++---
>>  include/linux/nfs_fs.h    |  9 ++++++++
>>  include/linux/nfs_fs_sb.h |  1 +
>>  include/linux/nfs_xdr.h   |  1 +
>>  7 files changed, 106 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>> index eef2728..d3e7b61 100644
>> --- a/fs/nfs/callback_proc.c
>> +++ b/fs/nfs/callback_proc.c
>> @@ -657,9 +657,45 @@ __be32 nfs4_callback_notify_lock(void *argp, void *resp,
>>  }
>>  #endif /* CONFIG_NFS_V4_1 */
>>  #ifdef CONFIG_NFS_V4_2
>> -__be32 nfs4_callback_offload(void *args, void *dummy,
>> +static void nfs4_copy_cb_args(struct nfs4_copy_state *cp_state,
>> +                             struct cb_offloadargs *args)
>> +{
>> +     cp_state->count = args->wr_count;
>> +     cp_state->error = args->error;
>> +     if (!args->error) {
>> +             cp_state->verf.committed = args->wr_writeverf.committed;
>> +             memcpy(&cp_state->verf.verifier.data[0],
>> +                     &args->wr_writeverf.verifier.data[0],
>> +                     NFS4_VERIFIER_SIZE);
>> +     }
>> +}
>> +
>> +__be32 nfs4_callback_offload(void *data, void *dummy,
>>                            struct cb_process_state *cps)
>>  {
>> +     struct cb_offloadargs *args = data;
>> +     struct nfs_server *server;
>> +     struct nfs4_copy_state *copy;
>> +
>> +     rcu_read_lock();
>> +     list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
>> +                             client_link) {
>> +             spin_lock(&server->nfs_client->cl_lock);
>> +             list_for_each_entry(copy, &server->ss_copies, copies) {
>> +                     if (memcmp(args->coa_stateid.other,
>> +                                     copy->stateid.other,
>> +                                     sizeof(args->coa_stateid.other)))
>> +                             continue;
>> +                     nfs4_copy_cb_args(copy, args);
>> +                     complete(&copy->completion);
>> +                     spin_unlock(&server->nfs_client->cl_lock);
>> +                     goto out;
>> +             }
>> +             spin_unlock(&server->nfs_client->cl_lock);
>> +     }
>> +out:
>> +     rcu_read_unlock();
>> +
>>       return 0;
>>  }
>>  #endif /* CONFIG_NFS_V4_2 */
>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>> index efebe6c..7b338be 100644
>> --- a/fs/nfs/client.c
>> +++ b/fs/nfs/client.c
>> @@ -876,6 +876,7 @@ struct nfs_server *nfs_alloc_server(void)
>>       INIT_LIST_HEAD(&server->delegations);
>>       INIT_LIST_HEAD(&server->layouts);
>>       INIT_LIST_HEAD(&server->state_owners_lru);
>> +     INIT_LIST_HEAD(&server->ss_copies);
>>
>>       atomic_set(&server->active, 0);
>>
>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>> index 5c42e71..c8ca353 100644
>> --- a/fs/nfs/nfs42proc.c
>> +++ b/fs/nfs/nfs42proc.c
>> @@ -129,6 +129,39 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
>>       return err;
>>  }
>>
>> +static int handle_async_copy(struct nfs42_copy_res *res,
>> +                          struct nfs_server *server,
>> +                          struct file *src,
>> +                          struct file *dst,
>> +                          nfs4_stateid *src_stateid,
>> +                          uint64_t *ret_count)
>> +{
>> +     struct nfs4_copy_state *copy;
>> +     int status;> +
>> +     copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
>> +     if (!copy)
>> +             return -ENOMEM;
>> +     memcpy(&copy->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
>> +     init_completion(&copy->completion);
>> +
>> +     spin_lock(&server->nfs_client->cl_lock);
>> +     list_add_tail(&copy->copies, &server->ss_copies);
>> +     spin_unlock(&server->nfs_client->cl_lock);
>> +
>> +     wait_for_completion_interruptible(&copy->completion);
>> +     spin_lock(&server->nfs_client->cl_lock);
>> +     list_del_init(&copy->copies);
>> +     spin_unlock(&server->nfs_client->cl_lock);
>> +     *ret_count = copy->count;
>> +     memcpy(&res->write_res.verifier, &copy->verf, sizeof(copy->verf));
>> +     if (copy->count <= 0)
>> +             status = -copy->error;
>
> If copy->count > 0 then status won't be set before this function returns.  GCC complains about this when I try compiling.

Got it. I'll initialize it.

>
> Anna
>
>> +
>> +     kfree(copy);
>> +     return status;
>> +}
>> +
>>  static ssize_t _nfs42_proc_copy(struct file *src,
>>                               struct nfs_lock_context *src_lock,
>>                               struct file *dst,
>> @@ -167,9 +200,13 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>>       if (status)
>>               return status;
>>
>> -     res->commit_res.verf = kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
>> -     if (!res->commit_res.verf)
>> -             return -ENOMEM;
>> +     res->commit_res.verf = NULL;
>> +     if (args->sync) {
>> +             res->commit_res.verf =
>> +                     kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
>> +             if (!res->commit_res.verf)
>> +                     return -ENOMEM;
>> +     }
>>       status = nfs4_call_sync(server->client, server, &msg,
>>                               &args->seq_args, &res->seq_res, 0);
>>       if (status == -ENOTSUPP)
>> @@ -177,18 +214,27 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>>       if (status)
>>               goto out;
>>
>> -     if (nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
>> +     if (args->sync &&
>> +             !nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
>>                                   &res->commit_res.verf->verifier)) {
>>               status = -EAGAIN;
>>               goto out;
>>       }
>>
>> +     if (!res->synchronous) {
>> +             status = handle_async_copy(res, server, src, dst,
>> +                             &args->src_stateid, &res->write_res.count);
>> +             if (status)
>> +                     return status;
>> +     }
>> +
>>       truncate_pagecache_range(dst_inode, pos_dst,
>>                                pos_dst + res->write_res.count);
>>
>>       status = res->write_res.count;
>>  out:
>> -     kfree(res->commit_res.verf);
>> +     if (args->sync)
>> +             kfree(res->commit_res.verf);
>>       return status;
>>  }
>>
>> @@ -205,6 +251,7 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
>>               .dst_fh         = NFS_FH(file_inode(dst)),
>>               .dst_pos        = pos_dst,
>>               .count          = count,
>> +             .sync           = false,
>>       };
>>       struct nfs42_copy_res res;
>>       struct nfs4_exception src_exception = {
>> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
>> index 9e557ad..2c37c66 100644
>> --- a/fs/nfs/nfs42xdr.c
>> +++ b/fs/nfs/nfs42xdr.c
>> @@ -160,7 +160,7 @@ static void encode_copy(struct xdr_stream *xdr,
>>       encode_uint64(xdr, args->count);
>>
>>       encode_uint32(xdr, 1); /* consecutive = true */
>> -     encode_uint32(xdr, 1); /* synchronous = true */
>> +     encode_uint32(xdr, args->sync);
>>       encode_uint32(xdr, 0); /* src server list */
>>  }
>>
>> @@ -291,7 +291,8 @@ static void nfs4_xdr_enc_copy(struct rpc_rqst *req,
>>       encode_savefh(xdr, &hdr);
>>       encode_putfh(xdr, args->dst_fh, &hdr);
>>       encode_copy(xdr, args, &hdr);
>> -     encode_copy_commit(xdr, args, &hdr);
>> +     if (args->sync)
>> +             encode_copy_commit(xdr, args, &hdr);
>>       encode_nops(&hdr);
>>  }
>>
>> @@ -613,7 +614,8 @@ static int nfs4_xdr_dec_copy(struct rpc_rqst *rqstp,
>>       status = decode_copy(xdr, res);
>>       if (status)
>>               goto out;
>> -     status = decode_commit(xdr, &res->commit_res);
>> +     if (res->commit_res.verf)
>> +             status = decode_commit(xdr, &res->commit_res);
>>  out:
>>       return status;
>>  }
>> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
>> index a0282ce..580a8d9 100644
>> --- a/include/linux/nfs_fs.h
>> +++ b/include/linux/nfs_fs.h
>> @@ -183,6 +183,15 @@ struct nfs_inode {
>>       struct inode            vfs_inode;
>>  };
>>
>> +struct nfs4_copy_state {
>> +     struct list_head        copies;
>> +     nfs4_stateid            stateid;
>> +     struct completion       completion;
>> +     uint64_t                count;
>> +     struct nfs_writeverf    verf;
>> +     int                     error;
>> +};
>> +
>>  /*
>>   * Cache validity bit flags
>>   */
>> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>> index de1eafe..511eefb 100644
>> --- a/include/linux/nfs_fs_sb.h
>> +++ b/include/linux/nfs_fs_sb.h
>> @@ -206,6 +206,7 @@ struct nfs_server {
>>       struct list_head        state_owners_lru;
>>       struct list_head        layouts;
>>       struct list_head        delegations;
>> +     struct list_head        ss_copies;
>>
>>       unsigned long           mig_gen;
>>       unsigned long           mig_status;
>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> index 784ec8d..020d958 100644
>> --- a/include/linux/nfs_xdr.h
>> +++ b/include/linux/nfs_xdr.h
>> @@ -1384,6 +1384,7 @@ struct nfs42_copy_args {
>>       u64                             dst_pos;
>>
>>       u64                             count;
>> +     bool                            sync;
>>  };
>>
>>  struct nfs42_write_res {
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 08/12] NFS handle COPY reply CB_OFFLOAD call race
  2017-09-28 17:28 ` [PATCH v4 08/12] NFS handle COPY reply CB_OFFLOAD call race Olga Kornievskaia
@ 2017-09-28 19:50   ` Anna Schumaker
  2017-09-28 19:57     ` Olga Kornievskaia
  0 siblings, 1 reply; 24+ messages in thread
From: Anna Schumaker @ 2017-09-28 19:50 UTC (permalink / raw)
  To: Olga Kornievskaia, Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Hi Olga,

On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
> It's possible that server replies back with CB_OFFLOAD call and
> COPY reply at the same time such that client will process
> CB_OFFLOAD before reply to COPY. For that keep a list of pending
> callback stateids received and them before waiting on completion
                    ^^^^^^^^^^^^^^^^^^^^^^^^
What are you doing with the stateids?

> check the pending list.
> 
> Cleanup any pending copies on the client shutdown.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/callback_proc.c    | 17 ++++++++++++++---
>  fs/nfs/nfs42proc.c        | 24 +++++++++++++++++++++---
>  fs/nfs/nfs4client.c       | 15 +++++++++++++++
>  include/linux/nfs_fs_sb.h |  1 +
>  4 files changed, 51 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index d3e7b61..d546c9f 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -676,11 +676,12 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>  	struct cb_offloadargs *args = data;
>  	struct nfs_server *server;
>  	struct nfs4_copy_state *copy;
> +	bool found = false;
>  
> +	spin_lock(&cps->clp->cl_lock);
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
>  				client_link) {
> -		spin_lock(&server->nfs_client->cl_lock);
>  		list_for_each_entry(copy, &server->ss_copies, copies) {
>  			if (memcmp(args->coa_stateid.other,
>  					copy->stateid.other,
> @@ -688,13 +689,23 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>  				continue;
>  			nfs4_copy_cb_args(copy, args);
>  			complete(&copy->completion);
> -			spin_unlock(&server->nfs_client->cl_lock);
> +			found = true;
>  			goto out;
>  		}
> -		spin_unlock(&server->nfs_client->cl_lock);
>  	}
>  out:
>  	rcu_read_unlock();
> +	if (!found) {
> +		copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
> +		if (!copy) {
> +			spin_unlock(&cps->clp->cl_lock);
> +			return -ENOMEM;
> +		}
> +		memcpy(&copy->stateid, &args->coa_stateid, NFS4_STATEID_SIZE);
> +		nfs4_copy_cb_args(copy, args);
> +		list_add_tail(&copy->copies, &cps->clp->pending_cb_stateids);
> +	}
> +	spin_unlock(&cps->clp->cl_lock);
>  
>  	return 0;
>  }
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index c8ca353..d0f454e 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -137,15 +137,32 @@ static int handle_async_copy(struct nfs42_copy_res *res,
>  			     uint64_t *ret_count)
>  {
>  	struct nfs4_copy_state *copy;
> -	int status;
> +	int status = NFS4_OK;

Might as well just move this bit to the previous patch :)

Anna

> +	bool found_pending = false;
> +
> +	spin_lock(&server->nfs_client->cl_lock);
> +	list_for_each_entry(copy, &server->nfs_client->pending_cb_stateids,
> +				copies) {
> +		if (memcmp(&res->write_res.stateid, &copy->stateid,
> +				NFS4_STATEID_SIZE))
> +			continue;
> +		found_pending = true;
> +		list_del(&copy->copies);
> +		break;
> +	}
> +	if (found_pending) {
> +		spin_unlock(&server->nfs_client->cl_lock);
> +		goto out;
> +	}
>  
>  	copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
> -	if (!copy)
> +	if (!copy) {
> +		spin_unlock(&server->nfs_client->cl_lock);
>  		return -ENOMEM;
> +	}
>  	memcpy(&copy->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
>  	init_completion(&copy->completion);
>  
> -	spin_lock(&server->nfs_client->cl_lock);
>  	list_add_tail(&copy->copies, &server->ss_copies);
>  	spin_unlock(&server->nfs_client->cl_lock);
>  
> @@ -153,6 +170,7 @@ static int handle_async_copy(struct nfs42_copy_res *res,
>  	spin_lock(&server->nfs_client->cl_lock);
>  	list_del_init(&copy->copies);
>  	spin_unlock(&server->nfs_client->cl_lock);
> +out:
>  	*ret_count = copy->count;
>  	memcpy(&res->write_res.verifier, &copy->verf, sizeof(copy->verf));
>  	if (copy->count <= 0)
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index e9bea90..46d4649 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -156,9 +156,23 @@ struct rpc_clnt *
>  	}
>  }
>  
> +static void
> +nfs4_cleanup_callback(struct nfs_client *clp)
> +{
> +	struct nfs4_copy_state *cp_state;
> +
> +	while (!list_empty(&clp->pending_cb_stateids)) {
> +		cp_state = list_entry(clp->pending_cb_stateids.next,
> +					struct nfs4_copy_state, copies);
> +		list_del(&cp_state->copies);
> +		kfree(cp_state);
> +	}
> +}
> +
>  void nfs41_shutdown_client(struct nfs_client *clp)
>  {
>  	if (nfs4_has_session(clp)) {
> +		nfs4_cleanup_callback(clp);
>  		nfs4_shutdown_ds_clients(clp);
>  		nfs4_destroy_session(clp->cl_session);
>  		nfs4_destroy_clientid(clp);
> @@ -202,6 +216,7 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
>  #if IS_ENABLED(CONFIG_NFS_V4_1)
>  	init_waitqueue_head(&clp->cl_lock_waitq);
>  #endif
> +	INIT_LIST_HEAD(&clp->pending_cb_stateids);
>  	return clp;
>  
>  error:
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 511eefb..72f159e 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -119,6 +119,7 @@ struct nfs_client {
>  #endif
>  
>  	struct net		*cl_net;
> +	struct list_head	pending_cb_stateids;
>  };
>  
>  /*
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 08/12] NFS handle COPY reply CB_OFFLOAD call race
  2017-09-28 19:50   ` Anna Schumaker
@ 2017-09-28 19:57     ` Olga Kornievskaia
  2017-09-28 19:59       ` Anna Schumaker
  0 siblings, 1 reply; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 19:57 UTC (permalink / raw)
  To: Anna Schumaker
  Cc: Olga Kornievskaia, Trond Myklebust, Anna Schumaker, linux-nfs

On Thu, Sep 28, 2017 at 3:50 PM, Anna Schumaker
<schumaker.anna@gmail.com> wrote:
> Hi Olga,
>
> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>> It's possible that server replies back with CB_OFFLOAD call and
>> COPY reply at the same time such that client will process
>> CB_OFFLOAD before reply to COPY. For that keep a list of pending
>> callback stateids received and them before waiting on completion
>                     ^^^^^^^^^^^^^^^^^^^^^^^^
> What are you doing with the stateids?

Is fixing "them" to "then" address the question?

>> check the pending list.
>>
>> Cleanup any pending copies on the client shutdown.
>>
>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>> ---
>>  fs/nfs/callback_proc.c    | 17 ++++++++++++++---
>>  fs/nfs/nfs42proc.c        | 24 +++++++++++++++++++++---
>>  fs/nfs/nfs4client.c       | 15 +++++++++++++++
>>  include/linux/nfs_fs_sb.h |  1 +
>>  4 files changed, 51 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>> index d3e7b61..d546c9f 100644
>> --- a/fs/nfs/callback_proc.c
>> +++ b/fs/nfs/callback_proc.c
>> @@ -676,11 +676,12 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>>       struct cb_offloadargs *args = data;
>>       struct nfs_server *server;
>>       struct nfs4_copy_state *copy;
>> +     bool found = false;
>>
>> +     spin_lock(&cps->clp->cl_lock);
>>       rcu_read_lock();
>>       list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
>>                               client_link) {
>> -             spin_lock(&server->nfs_client->cl_lock);
>>               list_for_each_entry(copy, &server->ss_copies, copies) {
>>                       if (memcmp(args->coa_stateid.other,
>>                                       copy->stateid.other,
>> @@ -688,13 +689,23 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>>                               continue;
>>                       nfs4_copy_cb_args(copy, args);
>>                       complete(&copy->completion);
>> -                     spin_unlock(&server->nfs_client->cl_lock);
>> +                     found = true;
>>                       goto out;
>>               }
>> -             spin_unlock(&server->nfs_client->cl_lock);
>>       }
>>  out:
>>       rcu_read_unlock();
>> +     if (!found) {
>> +             copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
>> +             if (!copy) {
>> +                     spin_unlock(&cps->clp->cl_lock);
>> +                     return -ENOMEM;
>> +             }
>> +             memcpy(&copy->stateid, &args->coa_stateid, NFS4_STATEID_SIZE);
>> +             nfs4_copy_cb_args(copy, args);
>> +             list_add_tail(&copy->copies, &cps->clp->pending_cb_stateids);
>> +     }
>> +     spin_unlock(&cps->clp->cl_lock);
>>
>>       return 0;
>>  }
>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>> index c8ca353..d0f454e 100644
>> --- a/fs/nfs/nfs42proc.c
>> +++ b/fs/nfs/nfs42proc.c
>> @@ -137,15 +137,32 @@ static int handle_async_copy(struct nfs42_copy_res *res,
>>                            uint64_t *ret_count)
>>  {
>>       struct nfs4_copy_state *copy;
>> -     int status;
>> +     int status = NFS4_OK;
>
> Might as well just move this bit to the previous patch :)

Got it.

> Anna
>
>> +     bool found_pending = false;
>> +
>> +     spin_lock(&server->nfs_client->cl_lock);
>> +     list_for_each_entry(copy, &server->nfs_client->pending_cb_stateids,
>> +                             copies) {
>> +             if (memcmp(&res->write_res.stateid, &copy->stateid,
>> +                             NFS4_STATEID_SIZE))
>> +                     continue;
>> +             found_pending = true;
>> +             list_del(&copy->copies);
>> +             break;
>> +     }
>> +     if (found_pending) {
>> +             spin_unlock(&server->nfs_client->cl_lock);
>> +             goto out;
>> +     }
>>
>>       copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
>> -     if (!copy)
>> +     if (!copy) {
>> +             spin_unlock(&server->nfs_client->cl_lock);
>>               return -ENOMEM;
>> +     }
>>       memcpy(&copy->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
>>       init_completion(&copy->completion);
>>
>> -     spin_lock(&server->nfs_client->cl_lock);
>>       list_add_tail(&copy->copies, &server->ss_copies);
>>       spin_unlock(&server->nfs_client->cl_lock);
>>
>> @@ -153,6 +170,7 @@ static int handle_async_copy(struct nfs42_copy_res *res,
>>       spin_lock(&server->nfs_client->cl_lock);
>>       list_del_init(&copy->copies);
>>       spin_unlock(&server->nfs_client->cl_lock);
>> +out:
>>       *ret_count = copy->count;
>>       memcpy(&res->write_res.verifier, &copy->verf, sizeof(copy->verf));
>>       if (copy->count <= 0)
>> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
>> index e9bea90..46d4649 100644
>> --- a/fs/nfs/nfs4client.c
>> +++ b/fs/nfs/nfs4client.c
>> @@ -156,9 +156,23 @@ struct rpc_clnt *
>>       }
>>  }
>>
>> +static void
>> +nfs4_cleanup_callback(struct nfs_client *clp)
>> +{
>> +     struct nfs4_copy_state *cp_state;
>> +
>> +     while (!list_empty(&clp->pending_cb_stateids)) {
>> +             cp_state = list_entry(clp->pending_cb_stateids.next,
>> +                                     struct nfs4_copy_state, copies);
>> +             list_del(&cp_state->copies);
>> +             kfree(cp_state);
>> +     }
>> +}
>> +
>>  void nfs41_shutdown_client(struct nfs_client *clp)
>>  {
>>       if (nfs4_has_session(clp)) {
>> +             nfs4_cleanup_callback(clp);
>>               nfs4_shutdown_ds_clients(clp);
>>               nfs4_destroy_session(clp->cl_session);
>>               nfs4_destroy_clientid(clp);
>> @@ -202,6 +216,7 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
>>  #if IS_ENABLED(CONFIG_NFS_V4_1)
>>       init_waitqueue_head(&clp->cl_lock_waitq);
>>  #endif
>> +     INIT_LIST_HEAD(&clp->pending_cb_stateids);
>>       return clp;
>>
>>  error:
>> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>> index 511eefb..72f159e 100644
>> --- a/include/linux/nfs_fs_sb.h
>> +++ b/include/linux/nfs_fs_sb.h
>> @@ -119,6 +119,7 @@ struct nfs_client {
>>  #endif
>>
>>       struct net              *cl_net;
>> +     struct list_head        pending_cb_stateids;
>>  };
>>
>>  /*
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 08/12] NFS handle COPY reply CB_OFFLOAD call race
  2017-09-28 19:57     ` Olga Kornievskaia
@ 2017-09-28 19:59       ` Anna Schumaker
  0 siblings, 0 replies; 24+ messages in thread
From: Anna Schumaker @ 2017-09-28 19:59 UTC (permalink / raw)
  To: Olga Kornievskaia
  Cc: Olga Kornievskaia, Trond Myklebust, Anna Schumaker, linux-nfs



On 09/28/2017 03:57 PM, Olga Kornievskaia wrote:
> On Thu, Sep 28, 2017 at 3:50 PM, Anna Schumaker
> <schumaker.anna@gmail.com> wrote:
>> Hi Olga,
>>
>> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>>> It's possible that server replies back with CB_OFFLOAD call and
>>> COPY reply at the same time such that client will process
>>> CB_OFFLOAD before reply to COPY. For that keep a list of pending
>>> callback stateids received and them before waiting on completion
>>                     ^^^^^^^^^^^^^^^^^^^^^^^^
>> What are you doing with the stateids?
> 
> Is fixing "them" to "then" address the question?

Yes, it does! :)

> 
>>> check the pending list.
>>>
>>> Cleanup any pending copies on the client shutdown.
>>>
>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>> ---
>>>  fs/nfs/callback_proc.c    | 17 ++++++++++++++---
>>>  fs/nfs/nfs42proc.c        | 24 +++++++++++++++++++++---
>>>  fs/nfs/nfs4client.c       | 15 +++++++++++++++
>>>  include/linux/nfs_fs_sb.h |  1 +
>>>  4 files changed, 51 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>>> index d3e7b61..d546c9f 100644
>>> --- a/fs/nfs/callback_proc.c
>>> +++ b/fs/nfs/callback_proc.c
>>> @@ -676,11 +676,12 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>>>       struct cb_offloadargs *args = data;
>>>       struct nfs_server *server;
>>>       struct nfs4_copy_state *copy;
>>> +     bool found = false;
>>>
>>> +     spin_lock(&cps->clp->cl_lock);
>>>       rcu_read_lock();
>>>       list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
>>>                               client_link) {
>>> -             spin_lock(&server->nfs_client->cl_lock);
>>>               list_for_each_entry(copy, &server->ss_copies, copies) {
>>>                       if (memcmp(args->coa_stateid.other,
>>>                                       copy->stateid.other,
>>> @@ -688,13 +689,23 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>>>                               continue;
>>>                       nfs4_copy_cb_args(copy, args);
>>>                       complete(&copy->completion);
>>> -                     spin_unlock(&server->nfs_client->cl_lock);
>>> +                     found = true;
>>>                       goto out;
>>>               }
>>> -             spin_unlock(&server->nfs_client->cl_lock);
>>>       }
>>>  out:
>>>       rcu_read_unlock();
>>> +     if (!found) {
>>> +             copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
>>> +             if (!copy) {
>>> +                     spin_unlock(&cps->clp->cl_lock);
>>> +                     return -ENOMEM;
>>> +             }
>>> +             memcpy(&copy->stateid, &args->coa_stateid, NFS4_STATEID_SIZE);
>>> +             nfs4_copy_cb_args(copy, args);
>>> +             list_add_tail(&copy->copies, &cps->clp->pending_cb_stateids);
>>> +     }
>>> +     spin_unlock(&cps->clp->cl_lock);
>>>
>>>       return 0;
>>>  }
>>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>>> index c8ca353..d0f454e 100644
>>> --- a/fs/nfs/nfs42proc.c
>>> +++ b/fs/nfs/nfs42proc.c
>>> @@ -137,15 +137,32 @@ static int handle_async_copy(struct nfs42_copy_res *res,
>>>                            uint64_t *ret_count)
>>>  {
>>>       struct nfs4_copy_state *copy;
>>> -     int status;
>>> +     int status = NFS4_OK;
>>
>> Might as well just move this bit to the previous patch :)
> 
> Got it.
> 
>> Anna
>>
>>> +     bool found_pending = false;
>>> +
>>> +     spin_lock(&server->nfs_client->cl_lock);
>>> +     list_for_each_entry(copy, &server->nfs_client->pending_cb_stateids,
>>> +                             copies) {
>>> +             if (memcmp(&res->write_res.stateid, &copy->stateid,
>>> +                             NFS4_STATEID_SIZE))
>>> +                     continue;
>>> +             found_pending = true;
>>> +             list_del(&copy->copies);
>>> +             break;
>>> +     }
>>> +     if (found_pending) {
>>> +             spin_unlock(&server->nfs_client->cl_lock);
>>> +             goto out;
>>> +     }
>>>
>>>       copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
>>> -     if (!copy)
>>> +     if (!copy) {
>>> +             spin_unlock(&server->nfs_client->cl_lock);
>>>               return -ENOMEM;
>>> +     }
>>>       memcpy(&copy->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
>>>       init_completion(&copy->completion);
>>>
>>> -     spin_lock(&server->nfs_client->cl_lock);
>>>       list_add_tail(&copy->copies, &server->ss_copies);
>>>       spin_unlock(&server->nfs_client->cl_lock);
>>>
>>> @@ -153,6 +170,7 @@ static int handle_async_copy(struct nfs42_copy_res *res,
>>>       spin_lock(&server->nfs_client->cl_lock);
>>>       list_del_init(&copy->copies);
>>>       spin_unlock(&server->nfs_client->cl_lock);
>>> +out:
>>>       *ret_count = copy->count;
>>>       memcpy(&res->write_res.verifier, &copy->verf, sizeof(copy->verf));
>>>       if (copy->count <= 0)
>>> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
>>> index e9bea90..46d4649 100644
>>> --- a/fs/nfs/nfs4client.c
>>> +++ b/fs/nfs/nfs4client.c
>>> @@ -156,9 +156,23 @@ struct rpc_clnt *
>>>       }
>>>  }
>>>
>>> +static void
>>> +nfs4_cleanup_callback(struct nfs_client *clp)
>>> +{
>>> +     struct nfs4_copy_state *cp_state;
>>> +
>>> +     while (!list_empty(&clp->pending_cb_stateids)) {
>>> +             cp_state = list_entry(clp->pending_cb_stateids.next,
>>> +                                     struct nfs4_copy_state, copies);
>>> +             list_del(&cp_state->copies);
>>> +             kfree(cp_state);
>>> +     }
>>> +}
>>> +
>>>  void nfs41_shutdown_client(struct nfs_client *clp)
>>>  {
>>>       if (nfs4_has_session(clp)) {
>>> +             nfs4_cleanup_callback(clp);
>>>               nfs4_shutdown_ds_clients(clp);
>>>               nfs4_destroy_session(clp->cl_session);
>>>               nfs4_destroy_clientid(clp);
>>> @@ -202,6 +216,7 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
>>>  #if IS_ENABLED(CONFIG_NFS_V4_1)
>>>       init_waitqueue_head(&clp->cl_lock_waitq);
>>>  #endif
>>> +     INIT_LIST_HEAD(&clp->pending_cb_stateids);
>>>       return clp;
>>>
>>>  error:
>>> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>>> index 511eefb..72f159e 100644
>>> --- a/include/linux/nfs_fs_sb.h
>>> +++ b/include/linux/nfs_fs_sb.h
>>> @@ -119,6 +119,7 @@ struct nfs_client {
>>>  #endif
>>>
>>>       struct net              *cl_net;
>>> +     struct list_head        pending_cb_stateids;
>>>  };
>>>
>>>  /*
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY
  2017-09-28 17:28 ` [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY Olga Kornievskaia
@ 2017-09-28 20:13   ` Anna Schumaker
  2017-09-28 20:34     ` Olga Kornievskaia
  0 siblings, 1 reply; 24+ messages in thread
From: Anna Schumaker @ 2017-09-28 20:13 UTC (permalink / raw)
  To: Olga Kornievskaia, Trond.Myklebust, anna.schumaker; +Cc: linux-nfs



On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
> A COPY with unstable write data needs a simple commit that doesn't
> deal with inodes
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs42proc.c | 22 ++++++++++++++++++++++
>  fs/nfs/nfs4_fs.h   |  2 +-
>  fs/nfs/nfs4proc.c  | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index b9e47a2..2064d11 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -252,6 +252,28 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>  			return status;
>  	}
>  
> +	if ((!res->synchronous || !args->sync) &&
> +			res->write_res.verifier.committed != NFS_FILE_SYNC) {
> +		struct nfs_commitres cres;
> +
> +		cres.verf = kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
> +		if (!cres.verf)
> +			return -ENOMEM;
> +
> +		status = nfs4_proc_commit(dst, pos_dst, res->write_res.count,
> +					  &cres);
> +		if (status) {
> +			kfree(cres.verf);
> +			return status;
> +		}
> +		if (!nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
> +					    &cres.verf->verifier)) {
> +			/* what are we suppose to do here ? */
> +			dprintk("commit verf differs from copy verf\n");

I assume you should retry the commit, but we're retrying the whole operation in the synchronous case so I'm not sure.

> +		}
> +		kfree(cres.verf);
> +	}
> +

_nfs42_proc_copy() does a lot of stuff right now.  Can you do the whole commit process into a separate function to make it easier to follow?

>  	truncate_pagecache_range(dst_inode, pos_dst,
>  				 pos_dst + res->write_res.count);
>  
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index c7225bb..5edb161 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -475,7 +475,7 @@ extern int nfs4_sequence_done(struct rpc_task *task,
>  			      struct nfs4_sequence_res *res);
>  
>  extern void nfs4_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp);
> -
> +extern int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res);
>  extern const nfs4_stateid zero_stateid;
>  
>  /* nfs4super.c */
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index f1bf19e..30829ce 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4829,6 +4829,39 @@ static void nfs4_proc_commit_setup(struct nfs_commit_data *data, struct rpc_mess
>  	nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
>  }
>  
> +static int _nfs4_proc_commit(struct file *dst, struct nfs_commitargs *args,
> +				struct nfs_commitres *res)
> +{
> +	struct inode *dst_inode = file_inode(dst);
> +	struct nfs_server *server = NFS_SERVER(dst_inode);
> +	struct rpc_message msg = {
> +		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_COMMIT],
> +		.rpc_argp = args,
> +		.rpc_resp = res,
> +	};
> +	return nfs4_call_sync(server->client, server, &msg,
> +			&args->seq_args, &res->seq_res, 1);
> +}
> +
> +int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res)
> +{
> +	struct nfs_commitargs args = {
> +		.fh = NFS_FH(file_inode(dst)),

How worried do we need to be about filehandles changing if we need to enter recovery during this operation?

Thanks,
Anna

> +		.offset = offset,
> +		.count = count,
> +	};
> +	struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
> +	struct nfs4_exception exception = { };
> +	int status;
> +
> +	do {
> +		status = _nfs4_proc_commit(dst, &args, res);> +		status = nfs4_handle_exception(dst_server, status, &exception);
> +	} while (exception.retry);
> +
> +	return status;
> +}
> +
>  struct nfs4_renewdata {
>  	struct nfs_client	*client;
>  	unsigned long		timestamp;
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 04/12] NFS OFFLOAD_STATUS op
  2017-09-28 17:28 ` [PATCH v4 04/12] NFS OFFLOAD_STATUS op Olga Kornievskaia
@ 2017-09-28 20:32   ` Anna Schumaker
  2017-09-28 20:41     ` Olga Kornievskaia
  0 siblings, 1 reply; 24+ messages in thread
From: Anna Schumaker @ 2017-09-28 20:32 UTC (permalink / raw)
  To: Olga Kornievskaia, Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Hi Olga,

On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs42.h     |  5 ++++-
>  fs/nfs/nfs42proc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h
> index b6cd153..6b9decf 100644
> --- a/fs/nfs/nfs42.h
> +++ b/fs/nfs/nfs42.h
> @@ -11,6 +11,7 @@
>   */
>  #define PNFS_LAYOUTSTATS_MAXDEV (4)
>  
> +#if defined(CONFIG_NFS_V4_2)
>  /* nfs4.2proc.c */
>  int nfs42_proc_allocate(struct file *, loff_t, loff_t);
>  ssize_t nfs42_proc_copy(struct file *, loff_t, struct file *, loff_t, size_t);
> @@ -19,5 +20,7 @@
>  int nfs42_proc_layoutstats_generic(struct nfs_server *,
>  				   struct nfs42_layoutstat_data *);
>  int nfs42_proc_clone(struct file *, struct file *, loff_t, loff_t, loff_t);
> -
> +int nfs42_proc_offload_status(struct file *, nfs4_stateid *,
> +			      struct nfs42_offload_status_res *);
> +#endif /* CONFIG_NFS_V4_2) */
>  #endif /* __LINUX_FS_NFS_NFS4_2_H */
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 6c2db51..5c42e71 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -263,6 +263,48 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
>  	return err;
>  }
>  
> +int _nfs42_proc_offload_status(struct file *dst, nfs4_stateid *stateid,
> +				struct nfs42_offload_status_res *res)

Does res have to be passed as a pointer here?  The struct is fairly small, so it couldn't it be allocated on the stack?

> +{
> +	struct nfs42_offload_status_args args;
> +	struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
> +	struct rpc_message msg = {
> +		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OFFLOAD_STATUS],
> +		.rpc_resp = res,
> +		.rpc_argp = &args,
> +	};
> +	int status;
> +
> +	args.osa_src_fh = NFS_FH(file_inode(dst));

This could probably be set in the struct initializer, similar to how msg is initialized.

> +	memcpy(&args.osa_stateid, stateid, sizeof(args.osa_stateid));
> +	status = nfs4_call_sync(dst_server->client, dst_server, &msg,
> +				&args.osa_seq_args, &res->osr_seq_res, 0);
> +	if (status == -ENOTSUPP)
> +		dst_server->caps &= ~NFS_CAP_OFFLOAD_STATUS;
> +
> +	return status;
> +}
> +
> +int nfs42_proc_offload_status(struct file *dst, nfs4_stateid *stateid,
> +				struct nfs42_offload_status_res *res)

Same question about the nfs42_offload_status_res here.

How have you been testing offload status?  As far as I can tell, nfs42_proc_offload_status() has no callers.  I guess I'm uncomfortable about adding something that never gets used.

Anna

> +{
> +	struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
> +	struct nfs4_exception exception = { };
> +	int status;
> +
> +	if (!(dst_server->caps & NFS_CAP_OFFLOAD_STATUS))
> +		return -EOPNOTSUPP;
> +
> +	do {
> +		status = _nfs42_proc_offload_status(dst, stateid, res);
> +		if (status == -ENOTSUPP)
> +			return -EOPNOTSUPP;
> +		status = nfs4_handle_exception(dst_server, status, &exception);
> +	} while (exception.retry);
> +
> +	return status;
> +}
> +
>  static loff_t _nfs42_proc_llseek(struct file *filep,
>  		struct nfs_lock_context *lock, loff_t offset, int whence)
>  {
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY
  2017-09-28 20:13   ` Anna Schumaker
@ 2017-09-28 20:34     ` Olga Kornievskaia
  2017-09-28 20:40       ` Anna Schumaker
  0 siblings, 1 reply; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 20:34 UTC (permalink / raw)
  To: Anna Schumaker
  Cc: Olga Kornievskaia, Trond Myklebust, Anna Schumaker, linux-nfs

On Thu, Sep 28, 2017 at 4:13 PM, Anna Schumaker
<schumaker.anna@gmail.com> wrote:
>
>
> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>> A COPY with unstable write data needs a simple commit that doesn't
>> deal with inodes
>>
>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>> ---
>>  fs/nfs/nfs42proc.c | 22 ++++++++++++++++++++++
>>  fs/nfs/nfs4_fs.h   |  2 +-
>>  fs/nfs/nfs4proc.c  | 33 +++++++++++++++++++++++++++++++++
>>  3 files changed, 56 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>> index b9e47a2..2064d11 100644
>> --- a/fs/nfs/nfs42proc.c
>> +++ b/fs/nfs/nfs42proc.c
>> @@ -252,6 +252,28 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>>                       return status;
>>       }
>>
>> +     if ((!res->synchronous || !args->sync) &&
>> +                     res->write_res.verifier.committed != NFS_FILE_SYNC) {
>> +             struct nfs_commitres cres;
>> +
>> +             cres.verf = kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
>> +             if (!cres.verf)
>> +                     return -ENOMEM;
>> +
>> +             status = nfs4_proc_commit(dst, pos_dst, res->write_res.count,
>> +                                       &cres);
>> +             if (status) {
>> +                     kfree(cres.verf);
>> +                     return status;
>> +             }
>> +             if (!nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
>> +                                         &cres.verf->verifier)) {
>> +                     /* what are we suppose to do here ? */
>> +                     dprintk("commit verf differs from copy verf\n");
>
> I assume you should retry the commit, but we're retrying the whole operation in the synchronous case so I'm not sure.

Urgh. I forgot about this. I guess I should retry the whole copy here..

>
>> +             }
>> +             kfree(cres.verf);
>> +     }
>> +
>
> _nfs42_proc_copy() does a lot of stuff right now.  Can you do the whole commit process into a separate function to make it easier to follow?

got it.

>
>>       truncate_pagecache_range(dst_inode, pos_dst,
>>                                pos_dst + res->write_res.count);
>>
>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>> index c7225bb..5edb161 100644
>> --- a/fs/nfs/nfs4_fs.h
>> +++ b/fs/nfs/nfs4_fs.h
>> @@ -475,7 +475,7 @@ extern int nfs4_sequence_done(struct rpc_task *task,
>>                             struct nfs4_sequence_res *res);
>>
>>  extern void nfs4_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp);
>> -
>> +extern int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res);
>>  extern const nfs4_stateid zero_stateid;
>>
>>  /* nfs4super.c */
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index f1bf19e..30829ce 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -4829,6 +4829,39 @@ static void nfs4_proc_commit_setup(struct nfs_commit_data *data, struct rpc_mess
>>       nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
>>  }
>>
>> +static int _nfs4_proc_commit(struct file *dst, struct nfs_commitargs *args,
>> +                             struct nfs_commitres *res)
>> +{
>> +     struct inode *dst_inode = file_inode(dst);
>> +     struct nfs_server *server = NFS_SERVER(dst_inode);
>> +     struct rpc_message msg = {
>> +             .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_COMMIT],
>> +             .rpc_argp = args,
>> +             .rpc_resp = res,
>> +     };
>> +     return nfs4_call_sync(server->client, server, &msg,
>> +                     &args->seq_args, &res->seq_res, 1);
>> +}
>> +
>> +int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res)
>> +{
>> +     struct nfs_commitargs args = {
>> +             .fh = NFS_FH(file_inode(dst)),
>
> How worried do we need to be about filehandles changing if we need to enter recovery during this operation?

This is the same thing we do in the copy and other operations (llseek, clone)?

>
> Thanks,
> Anna
>
>> +             .offset = offset,
>> +             .count = count,
>> +     };
>> +     struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
>> +     struct nfs4_exception exception = { };
>> +     int status;
>> +
>> +     do {
>> +             status = _nfs4_proc_commit(dst, &args, res);> +         status = nfs4_handle_exception(dst_server, status, &exception);
>> +     } while (exception.retry);
>> +
>> +     return status;
>> +}
>> +
>>  struct nfs4_renewdata {
>>       struct nfs_client       *client;
>>       unsigned long           timestamp;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY
  2017-09-28 20:34     ` Olga Kornievskaia
@ 2017-09-28 20:40       ` Anna Schumaker
  2017-09-29 16:01         ` Olga Kornievskaia
  0 siblings, 1 reply; 24+ messages in thread
From: Anna Schumaker @ 2017-09-28 20:40 UTC (permalink / raw)
  To: Olga Kornievskaia
  Cc: Olga Kornievskaia, Trond Myklebust, Anna Schumaker, linux-nfs



On 09/28/2017 04:34 PM, Olga Kornievskaia wrote:
> On Thu, Sep 28, 2017 at 4:13 PM, Anna Schumaker
> <schumaker.anna@gmail.com> wrote:
>>
>>
>> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>>> A COPY with unstable write data needs a simple commit that doesn't
>>> deal with inodes
>>>
>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>> ---
>>>  fs/nfs/nfs42proc.c | 22 ++++++++++++++++++++++
>>>  fs/nfs/nfs4_fs.h   |  2 +-
>>>  fs/nfs/nfs4proc.c  | 33 +++++++++++++++++++++++++++++++++
>>>  3 files changed, 56 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>>> index b9e47a2..2064d11 100644
>>> --- a/fs/nfs/nfs42proc.c
>>> +++ b/fs/nfs/nfs42proc.c
>>> @@ -252,6 +252,28 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>>>                       return status;
>>>       }
>>>
>>> +     if ((!res->synchronous || !args->sync) &&
>>> +                     res->write_res.verifier.committed != NFS_FILE_SYNC) {
>>> +             struct nfs_commitres cres;
>>> +
>>> +             cres.verf = kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
>>> +             if (!cres.verf)
>>> +                     return -ENOMEM;
>>> +
>>> +             status = nfs4_proc_commit(dst, pos_dst, res->write_res.count,
>>> +                                       &cres);
>>> +             if (status) {
>>> +                     kfree(cres.verf);
>>> +                     return status;
>>> +             }
>>> +             if (!nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
>>> +                                         &cres.verf->verifier)) {
>>> +                     /* what are we suppose to do here ? */
>>> +                     dprintk("commit verf differs from copy verf\n");
>>
>> I assume you should retry the commit, but we're retrying the whole operation in the synchronous case so I'm not sure.
> 
> Urgh. I forgot about this. I guess I should retry the whole copy here..
> 
>>
>>> +             }
>>> +             kfree(cres.verf);
>>> +     }
>>> +
>>
>> _nfs42_proc_copy() does a lot of stuff right now.  Can you do the whole commit process into a separate function to make it easier to follow?
> 
> got it.
> 
>>
>>>       truncate_pagecache_range(dst_inode, pos_dst,
>>>                                pos_dst + res->write_res.count);
>>>
>>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>>> index c7225bb..5edb161 100644
>>> --- a/fs/nfs/nfs4_fs.h
>>> +++ b/fs/nfs/nfs4_fs.h
>>> @@ -475,7 +475,7 @@ extern int nfs4_sequence_done(struct rpc_task *task,
>>>                             struct nfs4_sequence_res *res);
>>>
>>>  extern void nfs4_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp);
>>> -
>>> +extern int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res);
>>>  extern const nfs4_stateid zero_stateid;
>>>
>>>  /* nfs4super.c */
>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>> index f1bf19e..30829ce 100644
>>> --- a/fs/nfs/nfs4proc.c
>>> +++ b/fs/nfs/nfs4proc.c
>>> @@ -4829,6 +4829,39 @@ static void nfs4_proc_commit_setup(struct nfs_commit_data *data, struct rpc_mess
>>>       nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
>>>  }
>>>
>>> +static int _nfs4_proc_commit(struct file *dst, struct nfs_commitargs *args,
>>> +                             struct nfs_commitres *res)
>>> +{
>>> +     struct inode *dst_inode = file_inode(dst);
>>> +     struct nfs_server *server = NFS_SERVER(dst_inode);
>>> +     struct rpc_message msg = {
>>> +             .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_COMMIT],
>>> +             .rpc_argp = args,
>>> +             .rpc_resp = res,
>>> +     };
>>> +     return nfs4_call_sync(server->client, server, &msg,
>>> +                     &args->seq_args, &res->seq_res, 1);
>>> +}
>>> +
>>> +int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res)
>>> +{
>>> +     struct nfs_commitargs args = {
>>> +             .fh = NFS_FH(file_inode(dst)),
>>
>> How worried do we need to be about filehandles changing if we need to enter recovery during this operation?
> 
> This is the same thing we do in the copy and other operations (llseek, clone)?

Yeah, it's to match what we do in the other operations.  Otherwise  we could end up with a different filehandle if we need to do open recovery.

> 
>>
>> Thanks,
>> Anna
>>
>>> +             .offset = offset,
>>> +             .count = count,
>>> +     };
>>> +     struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
>>> +     struct nfs4_exception exception = { };
>>> +     int status;
>>> +
>>> +     do {
>>> +             status = _nfs4_proc_commit(dst, &args, res);> +         status = nfs4_handle_exception(dst_server, status, &exception);
>>> +     } while (exception.retry);
>>> +
>>> +     return status;
>>> +}
>>> +
>>>  struct nfs4_renewdata {
>>>       struct nfs_client       *client;
>>>       unsigned long           timestamp;
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 04/12] NFS OFFLOAD_STATUS op
  2017-09-28 20:32   ` Anna Schumaker
@ 2017-09-28 20:41     ` Olga Kornievskaia
  0 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-28 20:41 UTC (permalink / raw)
  To: Anna Schumaker; +Cc: Olga Kornievskaia, Trond Myklebust, linux-nfs

On Thu, Sep 28, 2017 at 4:32 PM, Anna Schumaker
<Anna.Schumaker@netapp.com> wrote:
> Hi Olga,
>
> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>> ---
>>  fs/nfs/nfs42.h     |  5 ++++-
>>  fs/nfs/nfs42proc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h
>> index b6cd153..6b9decf 100644
>> --- a/fs/nfs/nfs42.h
>> +++ b/fs/nfs/nfs42.h
>> @@ -11,6 +11,7 @@
>>   */
>>  #define PNFS_LAYOUTSTATS_MAXDEV (4)
>>
>> +#if defined(CONFIG_NFS_V4_2)
>>  /* nfs4.2proc.c */
>>  int nfs42_proc_allocate(struct file *, loff_t, loff_t);
>>  ssize_t nfs42_proc_copy(struct file *, loff_t, struct file *, loff_t, size_t);
>> @@ -19,5 +20,7 @@
>>  int nfs42_proc_layoutstats_generic(struct nfs_server *,
>>                                  struct nfs42_layoutstat_data *);
>>  int nfs42_proc_clone(struct file *, struct file *, loff_t, loff_t, loff_t);
>> -
>> +int nfs42_proc_offload_status(struct file *, nfs4_stateid *,
>> +                           struct nfs42_offload_status_res *);
>> +#endif /* CONFIG_NFS_V4_2) */
>>  #endif /* __LINUX_FS_NFS_NFS4_2_H */
>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>> index 6c2db51..5c42e71 100644
>> --- a/fs/nfs/nfs42proc.c
>> +++ b/fs/nfs/nfs42proc.c
>> @@ -263,6 +263,48 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
>>       return err;
>>  }
>>
>> +int _nfs42_proc_offload_status(struct file *dst, nfs4_stateid *stateid,
>> +                             struct nfs42_offload_status_res *res)
>
> Does res have to be passed as a pointer here?  The struct is fairly small, so it couldn't it be allocated on the stack?

Yes I'm assuming whatever is calling nfs42_proc_offload_status() would
be the one providing storage for the result and wanting to see the
results there.

>
>> +{
>> +     struct nfs42_offload_status_args args;
>> +     struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
>> +     struct rpc_message msg = {
>> +             .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OFFLOAD_STATUS],
>> +             .rpc_resp = res,
>> +             .rpc_argp = &args,
>> +     };
>> +     int status;
>> +
>> +     args.osa_src_fh = NFS_FH(file_inode(dst));
>
> This could probably be set in the struct initializer, similar to how msg is initialized.

Got it.

>> +     memcpy(&args.osa_stateid, stateid, sizeof(args.osa_stateid));
>> +     status = nfs4_call_sync(dst_server->client, dst_server, &msg,
>> +                             &args.osa_seq_args, &res->osr_seq_res, 0);
>> +     if (status == -ENOTSUPP)
>> +             dst_server->caps &= ~NFS_CAP_OFFLOAD_STATUS;
>> +
>> +     return status;
>> +}
>> +
>> +int nfs42_proc_offload_status(struct file *dst, nfs4_stateid *stateid,
>> +                             struct nfs42_offload_status_res *res)
>
> Same question about the nfs42_offload_status_res here.
>
> How have you been testing offload status?  As far as I can tell, nfs42_proc_offload_status() has no callers.  I guess I'm uncomfortable about adding something that never gets used.

I have tested offload status by instead of going to wait on the
completion of the cb_offload, it would send the offload status and
then go to sleep for a little bit. It would poll until it received all
the bytes or received the cb_offload.

I needed a way to test the OFFLOAD_STATUS for the server which is a
MUST for the asynchronous copy.

Functionality is contained in this patch so it could be just excluded
if desired.

> Anna
>
>> +{
>> +     struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
>> +     struct nfs4_exception exception = { };
>> +     int status;
>> +
>> +     if (!(dst_server->caps & NFS_CAP_OFFLOAD_STATUS))
>> +             return -EOPNOTSUPP;
>> +
>> +     do {
>> +             status = _nfs42_proc_offload_status(dst, stateid, res);
>> +             if (status == -ENOTSUPP)
>> +                     return -EOPNOTSUPP;
>> +             status = nfs4_handle_exception(dst_server, status, &exception);
>> +     } while (exception.retry);
>> +
>> +     return status;
>> +}
>> +
>>  static loff_t _nfs42_proc_llseek(struct file *filep,
>>               struct nfs_lock_context *lock, loff_t offset, int whence)
>>  {
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY
  2017-09-28 20:40       ` Anna Schumaker
@ 2017-09-29 16:01         ` Olga Kornievskaia
  0 siblings, 0 replies; 24+ messages in thread
From: Olga Kornievskaia @ 2017-09-29 16:01 UTC (permalink / raw)
  To: Anna Schumaker
  Cc: Olga Kornievskaia, Trond Myklebust, Anna Schumaker, linux-nfs

On Thu, Sep 28, 2017 at 4:40 PM, Anna Schumaker
<schumaker.anna@gmail.com> wrote:
>
>
> On 09/28/2017 04:34 PM, Olga Kornievskaia wrote:
>> On Thu, Sep 28, 2017 at 4:13 PM, Anna Schumaker
>> <schumaker.anna@gmail.com> wrote:
>>>
>>>
>>> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>>>> A COPY with unstable write data needs a simple commit that doesn't
>>>> deal with inodes
>>>>
>>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>>> ---
>>>>  fs/nfs/nfs42proc.c | 22 ++++++++++++++++++++++
>>>>  fs/nfs/nfs4_fs.h   |  2 +-
>>>>  fs/nfs/nfs4proc.c  | 33 +++++++++++++++++++++++++++++++++
>>>>  3 files changed, 56 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>>>> index b9e47a2..2064d11 100644
>>>> --- a/fs/nfs/nfs42proc.c
>>>> +++ b/fs/nfs/nfs42proc.c
>>>> @@ -252,6 +252,28 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>>>>                       return status;
>>>>       }
>>>>
>>>> +     if ((!res->synchronous || !args->sync) &&
>>>> +                     res->write_res.verifier.committed != NFS_FILE_SYNC) {
>>>> +             struct nfs_commitres cres;
>>>> +
>>>> +             cres.verf = kzalloc(sizeof(struct nfs_writeverf), GFP_NOFS);
>>>> +             if (!cres.verf)
>>>> +                     return -ENOMEM;
>>>> +
>>>> +             status = nfs4_proc_commit(dst, pos_dst, res->write_res.count,
>>>> +                                       &cres);
>>>> +             if (status) {
>>>> +                     kfree(cres.verf);
>>>> +                     return status;
>>>> +             }
>>>> +             if (!nfs_write_verifier_cmp(&res->write_res.verifier.verifier,
>>>> +                                         &cres.verf->verifier)) {
>>>> +                     /* what are we suppose to do here ? */
>>>> +                     dprintk("commit verf differs from copy verf\n");
>>>
>>> I assume you should retry the commit, but we're retrying the whole operation in the synchronous case so I'm not sure.
>>
>> Urgh. I forgot about this. I guess I should retry the whole copy here..
>>
>>>
>>>> +             }
>>>> +             kfree(cres.verf);
>>>> +     }
>>>> +
>>>
>>> _nfs42_proc_copy() does a lot of stuff right now.  Can you do the whole commit process into a separate function to make it easier to follow?
>>
>> got it.
>>
>>>
>>>>       truncate_pagecache_range(dst_inode, pos_dst,
>>>>                                pos_dst + res->write_res.count);
>>>>
>>>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>>>> index c7225bb..5edb161 100644
>>>> --- a/fs/nfs/nfs4_fs.h
>>>> +++ b/fs/nfs/nfs4_fs.h
>>>> @@ -475,7 +475,7 @@ extern int nfs4_sequence_done(struct rpc_task *task,
>>>>                             struct nfs4_sequence_res *res);
>>>>
>>>>  extern void nfs4_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp);
>>>> -
>>>> +extern int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res);
>>>>  extern const nfs4_stateid zero_stateid;
>>>>
>>>>  /* nfs4super.c */
>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>> index f1bf19e..30829ce 100644
>>>> --- a/fs/nfs/nfs4proc.c
>>>> +++ b/fs/nfs/nfs4proc.c
>>>> @@ -4829,6 +4829,39 @@ static void nfs4_proc_commit_setup(struct nfs_commit_data *data, struct rpc_mess
>>>>       nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
>>>>  }
>>>>
>>>> +static int _nfs4_proc_commit(struct file *dst, struct nfs_commitargs *args,
>>>> +                             struct nfs_commitres *res)
>>>> +{
>>>> +     struct inode *dst_inode = file_inode(dst);
>>>> +     struct nfs_server *server = NFS_SERVER(dst_inode);
>>>> +     struct rpc_message msg = {
>>>> +             .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_COMMIT],
>>>> +             .rpc_argp = args,
>>>> +             .rpc_resp = res,
>>>> +     };
>>>> +     return nfs4_call_sync(server->client, server, &msg,
>>>> +                     &args->seq_args, &res->seq_res, 1);
>>>> +}
>>>> +
>>>> +int nfs4_proc_commit(struct file *dst, __u64 offset, __u32 count, struct nfs_commitres *res)
>>>> +{
>>>> +     struct nfs_commitargs args = {
>>>> +             .fh = NFS_FH(file_inode(dst)),
>>>
>>> How worried do we need to be about filehandles changing if we need to enter recovery during this operation?
>>
>> This is the same thing we do in the copy and other operations (llseek, clone)?
>
> Yeah, it's to match what we do in the other operations.  Otherwise  we could end up with a different filehandle if we need to do open recovery.

Ok so I'd like some clarification to what is suppose to be here.

I think the correct behavior is to get the value of the "fh" from the
struct file inside the inner loop of the operation (I can change
commit to be that way).

Prior to commit 9d8cacbf563 "NFSv4: fix reboot recovery in copy
offload" copy used to assign "fh" inside the inner loop but then it
was moved into the main nfs42_proc_copy(). Was that not a correct
change then?

>
>>
>>>
>>> Thanks,
>>> Anna
>>>
>>>> +             .offset = offset,
>>>> +             .count = count,
>>>> +     };
>>>> +     struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
>>>> +     struct nfs4_exception exception = { };
>>>> +     int status;
>>>> +
>>>> +     do {
>>>> +             status = _nfs4_proc_commit(dst, &args, res);> +         status = nfs4_handle_exception(dst_server, status, &exception);
>>>> +     } while (exception.retry);
>>>> +
>>>> +     return status;
>>>> +}
>>>> +
>>>>  struct nfs4_renewdata {
>>>>       struct nfs_client       *client;
>>>>       unsigned long           timestamp;
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2017-09-29 16:01 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-28 17:28 [PATCH v4 00/12] NFS support for async intra COPY Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 01/12] fs: Don't copy beyond the end of the file Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 02/12] NFS CB_OFFLOAD xdr Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 03/12] NFS OFFLOAD_STATUS xdr Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 04/12] NFS OFFLOAD_STATUS op Olga Kornievskaia
2017-09-28 20:32   ` Anna Schumaker
2017-09-28 20:41     ` Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 05/12] NFS OFFLOAD_CANCEL xdr Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 06/12] NFS COPY xdr handle async reply Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 07/12] NFS add support for asynchronous COPY Olga Kornievskaia
2017-09-28 19:33   ` Anna Schumaker
2017-09-28 19:36     ` Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 08/12] NFS handle COPY reply CB_OFFLOAD call race Olga Kornievskaia
2017-09-28 19:50   ` Anna Schumaker
2017-09-28 19:57     ` Olga Kornievskaia
2017-09-28 19:59       ` Anna Schumaker
2017-09-28 17:28 ` [PATCH v4 09/12] NFS export nfs4_async_handle_error Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 10/12] NFS send OFFLOAD_CANCEL when COPY killed Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 11/12] NFS handle COPY ERR_OFFLOAD_NO_REQS Olga Kornievskaia
2017-09-28 17:28 ` [PATCH v4 12/12] NFS add a simple sync nfs4_proc_commit after async COPY Olga Kornievskaia
2017-09-28 20:13   ` Anna Schumaker
2017-09-28 20:34     ` Olga Kornievskaia
2017-09-28 20:40       ` Anna Schumaker
2017-09-29 16:01         ` Olga Kornievskaia

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.