[PATCH v8 0/9] NFSD support for async COPY

[PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

To do asynchronous copies, NFSD creates a new kthread to handle the request.
Upon receiving the COPY, it generates a unique copy stateid (stored in a
global list for keeping track of state for OFFLOAD_STATUS to be queried by),
starts the thread, and replies back to the client. nfsd4_copy arguments that
are allocated on the stack are copies for the kthread.

For the async copy handler, copy is done in the loop for the requested
number of bytes. If an error is encountered and partial copy has happened,
a successful partial copy is returned in the CB_OFFLOAD. vfs_copy_file_range
is called with 4MB chunks for both async and sync, allowing for 4MB
granularity of OFFLOAD_STATUS queiry. Once copy is done, the results are
queued for the callback workqueue and sent via CB_OFFLOAD.

When the server received an OFFLOAD_CANCEL, it will find the kthread running
the copy and will send a SIGPENDING and kthread_stop() and it will interrupt
the ongoing do_splice() and once vfs returns we are choosing not to send
the CB_OFFLOAD back to the client.

When the server receives an OFFLOAD_STATUS, it will find the kthread running
the copy and will locate within the copy state the current number of bytes
copied so far.

v8 addressing comments from the reviews
-- add comment in the code that we ignore error on copy
-- changed "fh_dst/src" to "file_dst/src"
-- dropped keeping "net", "stid" from the nfsd4_copy
-- removed "remove" arg from cleanup_async_copy()
-- reworked shutdown copy patch to 1) check for on going copies in
client_has_state() 2) reworked a bit the signalling code to incorporate
code that made sure only 1 thread can stop the thread. otherwise, if
somehow 2 things are trying to shutdown the copy, the 2nd one will hang.
-- removed storing parent state in the nfsd4_copy structure
-- removed nfs4_cp_state as we no longer need the list of them and moved
the stateid_t into nfsd4_copy itself
-- no longer taking a reference on the nfs4_client as on destruction
of that structure we make sure to store any copies.

Olga Kornievskaia (9):
  NFSD CB_OFFLOAD xdr
  NFSD OFFLOAD_STATUS xdr
  NFSD OFFLOAD_CANCEL xdr
  NFSD xdr callback stateid in async COPY reply
  NFSD introduce async copy feature
  NFSD create new stateid for async copy
  NFSD handle OFFLOAD_CANCEL op
  NFSD support OFFLOAD_STATUS
  NFSD stop ongoing async copies on client shutdown

 fs/nfsd/netns.h        |   8 ++
 fs/nfsd/nfs4callback.c |  98 +++++++++++++++++
 fs/nfsd/nfs4proc.c     | 290 ++++++++++++++++++++++++++++++++++++++++++++++---
 fs/nfsd/nfs4state.c    |  38 ++++++-
 fs/nfsd/nfs4xdr.c      |  50 +++++++--
 fs/nfsd/nfsctl.c       |   1 +
 fs/nfsd/state.h        |  10 ++
 fs/nfsd/xdr4.h         |  28 +++++
 fs/nfsd/xdr4cb.h       |  10 ++
 9 files changed, 509 insertions(+), 24 deletions(-)

-- 
1.8.3.1

[PATCH v8 1/9] NFSD CB_OFFLOAD xdr

[Olga Kornievskaia]

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4callback.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/state.h        |  1 +
 fs/nfsd/xdr4.h         |  6 ++++
 fs/nfsd/xdr4cb.h       | 10 ++++++
 4 files changed, 115 insertions(+)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 1f04d2a..2a979aa9 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -39,6 +39,7 @@
 #include "state.h"
 #include "netns.h"
 #include "xdr4cb.h"
+#include "xdr4.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
@@ -105,6 +106,7 @@ enum nfs_cb_opnum4 {
 	OP_CB_WANTS_CANCELLED		= 12,
 	OP_CB_NOTIFY_LOCK		= 13,
 	OP_CB_NOTIFY_DEVICEID		= 14,
+	OP_CB_OFFLOAD			= 15,
 	OP_CB_ILLEGAL			= 10044
 };
 
@@ -683,6 +685,101 @@ static int nfs4_xdr_dec_cb_notify_lock(struct rpc_rqst *rqstp,
 }
 
 /*
+ * struct write_response4 {
+ *	stateid4	wr_callback_id<1>;
+ *	length4		wr_count;
+ *	stable_how4	wr_committed;
+ *	verifier4	wr_writeverf;
+ * };
+ * union offload_info4 switch (nfsstat4 coa_status) {
+ *	case NFS4_OK:
+ *		write_response4	coa_resok4;
+ *	default:
+ *	length4		coa_bytes_copied;
+ * };
+ * struct CB_OFFLOAD4args {
+ *	nfs_fh4		coa_fh;
+ *	stateid4	coa_stateid;
+ *	offload_info4	coa_offload_info;
+ * };
+ */
+static void encode_offload_info4(struct xdr_stream *xdr,
+				 __be32 nfserr,
+				 const struct nfsd4_copy *cp)
+{
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 4);
+	*p++ = nfserr;
+	if (!nfserr) {
+		p = xdr_reserve_space(xdr, 4 + 8 + 4 + NFS4_VERIFIER_SIZE);
+		p = xdr_encode_empty_array(p);
+		p = xdr_encode_hyper(p, cp->cp_res.wr_bytes_written);
+		*p++ = cpu_to_be32(cp->cp_res.wr_stable_how);
+		p = xdr_encode_opaque_fixed(p, cp->cp_res.wr_verifier.data,
+					    NFS4_VERIFIER_SIZE);
+	} else {
+		p = xdr_reserve_space(xdr, 8);
+		/* We always return success if bytes were written */
+		p = xdr_encode_hyper(p, 0);
+	}
+}
+
+static void encode_cb_offload4args(struct xdr_stream *xdr,
+				   __be32 nfserr,
+				   const struct knfsd_fh *fh,
+				   const struct nfsd4_copy *cp,
+				   struct nfs4_cb_compound_hdr *hdr)
+{
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 4);
+	*p++ = cpu_to_be32(OP_CB_OFFLOAD);
+	encode_nfs_fh4(xdr, fh);
+	encode_stateid4(xdr, &cp->cp_res.cb_stateid);
+	encode_offload_info4(xdr, nfserr, cp);
+
+	hdr->nops++;
+}
+
+static void nfs4_xdr_enc_cb_offload(struct rpc_rqst *req,
+				    struct xdr_stream *xdr,
+				    const void *data)
+{
+	const struct nfsd4_callback *cb = data;
+	const struct nfsd4_copy *cp =
+		container_of(cb, struct nfsd4_copy, cp_cb);
+	struct nfs4_cb_compound_hdr hdr = {
+		.ident = 0,
+		.minorversion = cb->cb_clp->cl_minorversion,
+	};
+
+	encode_cb_compound4args(xdr, &hdr);
+	encode_cb_sequence4args(xdr, cb, &hdr);
+	encode_cb_offload4args(xdr, cp->nfserr, &cp->fh, cp, &hdr);
+	encode_cb_nops(&hdr);
+}
+
+static int nfs4_xdr_dec_cb_offload(struct rpc_rqst *rqstp,
+				   struct xdr_stream *xdr,
+				   void *data)
+{
+	struct nfsd4_callback *cb = data;
+	struct nfs4_cb_compound_hdr hdr;
+	int status;
+
+	status = decode_cb_compound4res(xdr, &hdr);
+	if (unlikely(status))
+		return status;
+
+	if (cb) {
+		status = decode_cb_sequence4res(xdr, cb);
+		if (unlikely(status || cb->cb_seq_status))
+			return status;
+	}
+	return decode_cb_op_status(xdr, OP_CB_OFFLOAD, &cb->cb_status);
+}
+/*
  * RPC procedure tables
  */
 #define PROC(proc, call, argtype, restype)				\
@@ -703,6 +800,7 @@ static int nfs4_xdr_dec_cb_notify_lock(struct rpc_rqst *rqstp,
 	PROC(CB_LAYOUT,	COMPOUND,	cb_layout,	cb_layout),
 #endif
 	PROC(CB_NOTIFY_LOCK,	COMPOUND,	cb_notify_lock,	cb_notify_lock),
+	PROC(CB_OFFLOAD,	COMPOUND,	cb_offload,	cb_offload),
 };
 
 static unsigned int nfs4_cb_counts[ARRAY_SIZE(nfs4_cb_procedures)];
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index f3772ea..5c16d35 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -573,6 +573,7 @@ enum nfsd4_cb_op {
 	NFSPROC4_CLNT_CB_NULL = 0,
 	NFSPROC4_CLNT_CB_RECALL,
 	NFSPROC4_CLNT_CB_LAYOUT,
+	NFSPROC4_CLNT_CB_OFFLOAD,
 	NFSPROC4_CLNT_CB_SEQUENCE,
 	NFSPROC4_CLNT_CB_NOTIFY_LOCK,
 };
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 17c453a..b7c34f4 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -511,6 +511,7 @@ struct nfsd42_write_res {
 	u64			wr_bytes_written;
 	u32			wr_stable_how;
 	nfs4_verifier		wr_verifier;
+	stateid_t		cb_stateid;
 };
 
 struct nfsd4_copy {
@@ -526,6 +527,11 @@ struct nfsd4_copy {
 
 	/* response */
 	struct nfsd42_write_res	cp_res;
+
+	/* for cb_offload */
+	struct nfsd4_callback	cp_cb;
+	__be32			nfserr;
+	struct knfsd_fh		fh;
 };
 
 struct nfsd4_seek {
diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
index 517239a..547cf07 100644
--- a/fs/nfsd/xdr4cb.h
+++ b/fs/nfsd/xdr4cb.h
@@ -38,3 +38,13 @@
 #define NFS4_dec_cb_notify_lock_sz	(cb_compound_dec_hdr_sz  +      \
 					cb_sequence_dec_sz +            \
 					op_dec_sz)
+#define enc_cb_offload_info_sz		(1 + 1 + 2 + 1 +		\
+					XDR_QUADLEN(NFS4_VERIFIER_SIZE))
+#define NFS4_enc_cb_offload_sz		(cb_compound_enc_hdr_sz +       \
+					cb_sequence_enc_sz +            \
+					enc_nfs4_fh_sz +		\
+					enc_stateid_sz +		\
+					enc_cb_offload_info_sz)
+#define NFS4_dec_cb_offload_sz		(cb_compound_dec_hdr_sz  +      \
+					cb_sequence_dec_sz +            \
+					op_dec_sz)
-- 
1.8.3.1

[PATCH v8 2/9] NFSD OFFLOAD_STATUS xdr

[Olga Kornievskaia]

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4proc.c | 20 ++++++++++++++++++++
 fs/nfsd/nfs4xdr.c  | 27 +++++++++++++++++++++++++--
 fs/nfsd/xdr4.h     | 10 ++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 5d99e88..15089b2 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1155,6 +1155,13 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write)
 	fput(file);
 	return status;
 }
+static __be32
+nfsd4_offload_status(struct svc_rqst *rqstp,
+		     struct nfsd4_compound_state *cstate,
+		     union nfsd4_op_u *u)
+{
+	return nfserr_notsupp;
+}
 
 static __be32
 nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
@@ -2052,6 +2059,14 @@ static inline u32 nfsd4_copy_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
 		1 /* cr_synchronous */) * sizeof(__be32);
 }
 
+static inline u32 nfsd4_offload_status_rsize(struct svc_rqst *rqstp,
+					     struct nfsd4_op *op)
+{
+	return (op_encode_hdr_size +
+		2 /* osr_count */ +
+		1 /* osr_complete<1> optional 0 for now */) * sizeof(__be32);
+}
+
 #ifdef CONFIG_NFSD_PNFS
 static inline u32 nfsd4_getdeviceinfo_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
 {
@@ -2465,6 +2480,11 @@ static inline u32 nfsd4_seek_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
 		.op_name = "OP_SEEK",
 		.op_rsize_bop = nfsd4_seek_rsize,
 	},
+	[OP_OFFLOAD_STATUS] = {
+		.op_func = nfsd4_offload_status,
+		.op_name = "OP_OFFLOAD_STATUS",
+		.op_rsize_bop = nfsd4_offload_status_rsize,
+	},
 };
 
 /**
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 1d048dd..738f49d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1765,6 +1765,13 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
 }
 
 static __be32
+nfsd4_decode_offload_status(struct nfsd4_compoundargs *argp,
+			    struct nfsd4_offload_status *os)
+{
+	return nfsd4_decode_stateid(argp, &os->stateid);
+}
+
+static __be32
 nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
 {
 	DECODE_HEAD;
@@ -1871,7 +1878,7 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
 	[OP_LAYOUTERROR]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_LAYOUTSTATS]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_OFFLOAD_CANCEL]	= (nfsd4_dec)nfsd4_decode_notsupp,
-	[OP_OFFLOAD_STATUS]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_OFFLOAD_STATUS]	= (nfsd4_dec)nfsd4_decode_offload_status,
 	[OP_READ_PLUS]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_SEEK]		= (nfsd4_dec)nfsd4_decode_seek,
 	[OP_WRITE_SAME]		= (nfsd4_dec)nfsd4_decode_notsupp,
@@ -4220,6 +4227,22 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 }
 
 static __be32
+nfsd4_encode_offload_status(struct nfsd4_compoundres *resp, __be32 nfserr,
+			    struct nfsd4_offload_status *os)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 8 + 4);
+	if (!p)
+		return nfserr_resource;
+	p = xdr_encode_hyper(p, os->count);
+	*p++ = cpu_to_be32(0);
+
+	return nfserr;
+}
+
+static __be32
 nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr,
 		  struct nfsd4_seek *seek)
 {
@@ -4322,7 +4345,7 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 	[OP_LAYOUTERROR]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_LAYOUTSTATS]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_OFFLOAD_CANCEL]	= (nfsd4_enc)nfsd4_encode_noop,
-	[OP_OFFLOAD_STATUS]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_OFFLOAD_STATUS]	= (nfsd4_enc)nfsd4_encode_offload_status,
 	[OP_READ_PLUS]		= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_SEEK]		= (nfsd4_enc)nfsd4_encode_seek,
 	[OP_WRITE_SAME]		= (nfsd4_enc)nfsd4_encode_noop,
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index b7c34f4..06cf218 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -545,6 +545,15 @@ struct nfsd4_seek {
 	loff_t		seek_pos;
 };
 
+struct nfsd4_offload_status {
+	/* request */
+	stateid_t	stateid;
+
+	/* response */
+	u64		count;
+	u32		status;
+};
+
 struct nfsd4_op {
 	int					opnum;
 	const struct nfsd4_operation *		opdesc;
@@ -603,6 +612,7 @@ struct nfsd4_op {
 		struct nfsd4_fallocate		deallocate;
 		struct nfsd4_clone		clone;
 		struct nfsd4_copy		copy;
+		struct nfsd4_offload_status	offload_status;
 		struct nfsd4_seek		seek;
 	} u;
 	struct nfs4_replay *			replay;
-- 
1.8.3.1

[PATCH v8 3/9] NFSD OFFLOAD_CANCEL xdr

[Olga Kornievskaia]

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4proc.c | 14 ++++++++++++++
 fs/nfsd/nfs4xdr.c  |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 15089b2..07ff6d0 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1134,6 +1134,14 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write)
 }
 
 static __be32
+nfsd4_offload_cancel(struct svc_rqst *rqstp,
+		     struct nfsd4_compound_state *cstate,
+		     union nfsd4_op_u *u)
+{
+	return 0;
+}
+
+static __be32
 nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		struct nfsd4_fallocate *fallocate, int flags)
 {
@@ -2485,6 +2493,12 @@ static inline u32 nfsd4_seek_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
 		.op_name = "OP_OFFLOAD_STATUS",
 		.op_rsize_bop = nfsd4_offload_status_rsize,
 	},
+	[OP_OFFLOAD_CANCEL] = {
+		.op_func = nfsd4_offload_cancel,
+		.op_flags = OP_MODIFIES_SOMETHING,
+		.op_name = "OP_OFFLOAD_CANCEL",
+		.op_rsize_bop = nfsd4_only_status_rsize,
+	},
 };
 
 /**
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 738f49d..9ea093f 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1877,7 +1877,7 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
 	[OP_IO_ADVISE]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_LAYOUTERROR]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_LAYOUTSTATS]	= (nfsd4_dec)nfsd4_decode_notsupp,
-	[OP_OFFLOAD_CANCEL]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_OFFLOAD_CANCEL]	= (nfsd4_dec)nfsd4_decode_offload_status,
 	[OP_OFFLOAD_STATUS]	= (nfsd4_dec)nfsd4_decode_offload_status,
 	[OP_READ_PLUS]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_SEEK]		= (nfsd4_dec)nfsd4_decode_seek,
-- 
1.8.3.1

[PATCH v8 4/9] NFSD xdr callback stateid in async COPY reply

[Olga Kornievskaia]

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4xdr.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9ea093f..6d11697 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4194,15 +4194,27 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 #endif /* CONFIG_NFSD_PNFS */
 
 static __be32
-nfsd42_encode_write_res(struct nfsd4_compoundres *resp, struct nfsd42_write_res *write)
+nfsd42_encode_write_res(struct nfsd4_compoundres *resp,
+		struct nfsd42_write_res *write, bool sync)
 {
 	__be32 *p;
+	p = xdr_reserve_space(&resp->xdr, 4);
+	if (!p)
+		return nfserr_resource;
 
-	p = xdr_reserve_space(&resp->xdr, 4 + 8 + 4 + NFS4_VERIFIER_SIZE);
+	if (sync)
+		*p++ = cpu_to_be32(0);
+	else {
+		__be32 nfserr;
+		*p++ = cpu_to_be32(1);
+		nfserr = nfsd4_encode_stateid(&resp->xdr, &write->cb_stateid);
+		if (nfserr)
+			return nfserr;
+	}
+	p = xdr_reserve_space(&resp->xdr, 8 + 4 + NFS4_VERIFIER_SIZE);
 	if (!p)
 		return nfserr_resource;
 
-	*p++ = cpu_to_be32(0);
 	p = xdr_encode_hyper(p, write->wr_bytes_written);
 	*p++ = cpu_to_be32(write->wr_stable_how);
 	p = xdr_encode_opaque_fixed(p, write->wr_verifier.data,
@@ -4216,7 +4228,8 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 {
 	__be32 *p;
 
-	nfserr = nfsd42_encode_write_res(resp, &copy->cp_res);
+	nfserr = nfsd42_encode_write_res(resp, &copy->cp_res,
+			copy->cp_synchronous);
 	if (nfserr)
 		return nfserr;
 
-- 
1.8.3.1

[PATCH v8 5/9] NFSD introduce async copy feature

[Olga Kornievskaia]

Upon receiving a request for async copy, create a new kthread.
If we get asynchronous request, make sure to copy the needed
arguments/state from the stack before starting the copy. Then
start the thread and reply back to the client indicating copy
is asynchronous.

nfsd_copy_file_range() will copy in a loop over the total
number of bytes is needed to copy. In case a failure happens
in the middle, we ignore the error and return how much we
copied so far. Once done creating a workitem for the callback
workqueue and send CB_OFFLOAD with the results.

In the following patches the unique stateid will be generated
for the async COPY to return, at that point will enable the
async feature.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4proc.c  | 160 ++++++++++++++++++++++++++++++++++++++++++++++------
 fs/nfsd/nfs4state.c |   2 +
 fs/nfsd/state.h     |   2 +
 fs/nfsd/xdr4.h      |   8 +++
 4 files changed, 155 insertions(+), 17 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 07ff6d0..0650271 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -36,6 +36,7 @@
 #include <linux/file.h>
 #include <linux/falloc.h>
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "idmap.h"
 #include "cache.h"
@@ -1099,38 +1100,163 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write)
 out:
 	return status;
 }
+static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
+{
+	struct nfsd4_copy *copy = container_of(cb, struct nfsd4_copy, cp_cb);
+
+	kfree(copy);
+}
+
+static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
+				 struct rpc_task *task)
+{
+	return 1;
+}
+
+static const struct nfsd4_callback_ops nfsd4_cb_offload_ops = {
+	.release = nfsd4_cb_offload_release,
+	.done = nfsd4_cb_offload_done
+};
+
+static __be32 nfsd4_init_copy_res(struct nfsd4_copy *copy, bool sync)
+{
+	memcpy(&copy->cp_res.cb_stateid, &copy->cp_dst_stateid,
+		sizeof(copy->cp_dst_stateid));
+	copy->cp_res.wr_stable_how = NFS_UNSTABLE;
+	copy->cp_synchronous = sync;
+	gen_boot_verifier(&copy->cp_res.wr_verifier, copy->cp_clp->net);
+
+	return nfs_ok;
+}
+
+static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy)
+{
+	ssize_t bytes_copied = 0;
+	size_t bytes_total = copy->cp_count;
+	u64 src_pos = copy->cp_src_pos;
+	u64 dst_pos = copy->cp_dst_pos;
+
+	do {
+		bytes_copied = nfsd_copy_file_range(copy->file_src, src_pos,
+				copy->file_dst, dst_pos, bytes_total);
+		if (bytes_copied <= 0)
+			break;
+		bytes_total -= bytes_copied;
+		copy->cp_res.wr_bytes_written += bytes_copied;
+		src_pos += bytes_copied;
+		dst_pos += bytes_copied;
+	} while (bytes_total > 0 && !copy->cp_synchronous);
+	return bytes_copied;
+}
+
+static __be32 nfsd4_do_copy(struct nfsd4_copy *copy, bool sync)
+{
+	__be32 status;
+	ssize_t bytes;
+
+	bytes = _nfsd_copy_file_range(copy);
+	/* for async copy, we ignore the error, client can always retry
+	 * to get the error
+	 */
+	if (bytes < 0 && !copy->cp_res.wr_bytes_written)
+		status = nfserrno(bytes);
+	else
+		status = nfsd4_init_copy_res(copy, sync);
+
+	fput(copy->file_src);
+	fput(copy->file_dst);
+	return status;
+}
+
+static void dup_copy_fields(struct nfsd4_copy *src, struct nfsd4_copy *dst)
+{
+	dst->cp_src_pos = src->cp_src_pos;
+	dst->cp_dst_pos = src->cp_dst_pos;
+	dst->cp_count = src->cp_count;
+	dst->cp_synchronous = src->cp_synchronous;
+	memcpy(&dst->cp_res, &src->cp_res, sizeof(src->cp_res));
+	memcpy(&dst->fh, &src->fh, sizeof(src->fh));
+	dst->cp_clp = src->cp_clp;
+	dst->file_dst = get_file(src->file_dst);
+	dst->file_src = get_file(src->file_src);
+}
+
+static void cleanup_async_copy(struct nfsd4_copy *copy)
+{
+	fput(copy->file_dst);
+	fput(copy->file_src);
+	spin_lock(&copy->cp_clp->async_lock);
+	list_del(&copy->copies);
+	spin_unlock(&copy->cp_clp->async_lock);
+	kfree(copy);
+}
+
+static int nfsd4_do_async_copy(void *data)
+{
+	struct nfsd4_copy *copy = (struct nfsd4_copy *)data;
+	struct nfsd4_copy *cb_copy;
+
+	copy->nfserr = nfsd4_do_copy(copy, 0);
+	cb_copy = kzalloc(sizeof(struct nfsd4_copy), GFP_KERNEL);
+	if (!cb_copy)
+		goto out;
+	memcpy(&cb_copy->cp_res, &copy->cp_res, sizeof(copy->cp_res));
+	cb_copy->cp_clp = copy->cp_clp;
+	cb_copy->nfserr = copy->nfserr;
+	memcpy(&cb_copy->fh, &copy->fh, sizeof(copy->fh));
+	nfsd4_init_cb(&cb_copy->cp_cb, cb_copy->cp_clp,
+			&nfsd4_cb_offload_ops, NFSPROC4_CLNT_CB_OFFLOAD);
+	nfsd4_run_cb(&cb_copy->cp_cb);
+out:
+	cleanup_async_copy(copy);
+	return 0;
+}
 
 static __be32
 nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		union nfsd4_op_u *u)
 {
 	struct nfsd4_copy *copy = &u->copy;
-	struct file *src, *dst;
 	__be32 status;
-	ssize_t bytes;
+	struct nfsd4_copy *async_copy = NULL;
 
-	status = nfsd4_verify_copy(rqstp, cstate, &copy->cp_src_stateid, &src,
-				   &copy->cp_dst_stateid, &dst);
+	status = nfsd4_verify_copy(rqstp, cstate, &copy->cp_src_stateid,
+				   &copy->file_src, &copy->cp_dst_stateid,
+				   &copy->file_dst);
 	if (status)
 		goto out;
 
-	bytes = nfsd_copy_file_range(src, copy->cp_src_pos,
-			dst, copy->cp_dst_pos, copy->cp_count);
-
-	if (bytes < 0)
-		status = nfserrno(bytes);
-	else {
-		copy->cp_res.wr_bytes_written = bytes;
-		copy->cp_res.wr_stable_how = NFS_UNSTABLE;
-		copy->cp_synchronous = 1;
-		gen_boot_verifier(&copy->cp_res.wr_verifier, SVC_NET(rqstp));
+	copy->cp_clp = cstate->clp;
+	memcpy(&copy->fh, &cstate->current_fh.fh_handle,
+		sizeof(struct knfsd_fh));
+	/* for now disable asynchronous copy feature */
+	copy->cp_synchronous = 1;
+	if (!copy->cp_synchronous) {
+		status = nfserrno(-ENOMEM);
+		async_copy = kzalloc(sizeof(struct nfsd4_copy), GFP_KERNEL);
+		if (!async_copy)
+			goto out;
+		dup_copy_fields(copy, async_copy);
+		memcpy(&copy->cp_res.cb_stateid, &copy->cp_dst_stateid,
+			sizeof(copy->cp_dst_stateid));
+		async_copy->copy_task = kthread_create(nfsd4_do_async_copy,
+				async_copy, "%s", "copy thread");
+		if (IS_ERR(async_copy->copy_task))
+			goto out_err;
+		spin_lock(&async_copy->cp_clp->async_lock);
+		list_add(&async_copy->copies,
+				&async_copy->cp_clp->async_copies);
+		spin_unlock(&async_copy->cp_clp->async_lock);
+		wake_up_process(async_copy->copy_task);
 		status = nfs_ok;
+	} else {
+		status = nfsd4_do_copy(copy, 1);
 	}
-
-	fput(src);
-	fput(dst);
 out:
 	return status;
+out_err:
+	cleanup_async_copy(async_copy);
+	goto out;
 }
 
 static __be32
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fc74d6f..1094ade 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1826,6 +1826,8 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
 #ifdef CONFIG_NFSD_PNFS
 	INIT_LIST_HEAD(&clp->cl_lo_states);
 #endif
+	INIT_LIST_HEAD(&clp->async_copies);
+	spin_lock_init(&clp->async_lock);
 	spin_lock_init(&clp->cl_lock);
 	rpc_init_wait_queue(&clp->cl_cb_waitq, "Backchannel slot table");
 	return clp;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 5c16d35..491030e 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -355,6 +355,8 @@ struct nfs4_client {
 	struct rpc_wait_queue	cl_cb_waitq;	/* backchannel callers may */
 						/* wait here for slots */
 	struct net		*net;
+	struct list_head	async_copies;	/* list of async copies */
+	spinlock_t		async_lock;	/* lock for async copies */
 };
 
 /* struct nfs4_client_reset
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 06cf218..ed49646 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -532,6 +532,14 @@ struct nfsd4_copy {
 	struct nfsd4_callback	cp_cb;
 	__be32			nfserr;
 	struct knfsd_fh		fh;
+
+	struct nfs4_client      *cp_clp;
+
+	struct file             *file_src;
+	struct file             *file_dst;
+
+	struct list_head	copies;
+	struct task_struct	*copy_task;
 };
 
 struct nfsd4_seek {
-- 
1.8.3.1

[PATCH v8 6/9] NFSD create new stateid for async copy

[Olga Kornievskaia]

Generate a new stateid to be used for reply to the asynchronous
COPY. Right now deciding to bind the lifetime to when the vfs copy
is done. This way don't need to keep the nfsd_net structure for
the callback. The drawback is that time copy state information
is available for query by OFFLOAD_STATUS is slightly less.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/netns.h     |  8 ++++++++
 fs/nfsd/nfs4proc.c  | 14 ++++++++------
 fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++++++++++
 fs/nfsd/nfsctl.c    |  1 +
 fs/nfsd/state.h     |  3 +++
 fs/nfsd/xdr4.h      |  2 ++
 6 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 36358d4..e83cf6e 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -122,6 +122,14 @@ struct nfsd_net {
 
 	wait_queue_head_t ntf_wq;
 	atomic_t ntf_refcnt;
+
+	/*
+	 * clientid and stateid data for construction of net unique COPY
+	 * stateids.
+	 */
+	u32		s2s_cp_cl_id;
+	struct idr	s2s_cp_stateids;
+	spinlock_t	s2s_cp_lock;
 };
 
 /* Simple check to find out if a given net was properly initialized */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 0650271..9d5f0d0 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1120,8 +1120,6 @@ static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
 
 static __be32 nfsd4_init_copy_res(struct nfsd4_copy *copy, bool sync)
 {
-	memcpy(&copy->cp_res.cb_stateid, &copy->cp_dst_stateid,
-		sizeof(copy->cp_dst_stateid));
 	copy->cp_res.wr_stable_how = NFS_UNSTABLE;
 	copy->cp_synchronous = sync;
 	gen_boot_verifier(&copy->cp_res.wr_verifier, copy->cp_clp->net);
@@ -1179,10 +1177,12 @@ static void dup_copy_fields(struct nfsd4_copy *src, struct nfsd4_copy *dst)
 	dst->cp_clp = src->cp_clp;
 	dst->file_dst = get_file(src->file_dst);
 	dst->file_src = get_file(src->file_src);
+	memcpy(&dst->cp_stateid, &src->cp_stateid, sizeof(src->cp_stateid));
 }
 
 static void cleanup_async_copy(struct nfsd4_copy *copy)
 {
+	nfs4_free_cp_state(copy);
 	fput(copy->file_dst);
 	fput(copy->file_src);
 	spin_lock(&copy->cp_clp->async_lock);
@@ -1229,16 +1229,18 @@ static int nfsd4_do_async_copy(void *data)
 	copy->cp_clp = cstate->clp;
 	memcpy(&copy->fh, &cstate->current_fh.fh_handle,
 		sizeof(struct knfsd_fh));
-	/* for now disable asynchronous copy feature */
-	copy->cp_synchronous = 1;
 	if (!copy->cp_synchronous) {
+		struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+
 		status = nfserrno(-ENOMEM);
 		async_copy = kzalloc(sizeof(struct nfsd4_copy), GFP_KERNEL);
 		if (!async_copy)
 			goto out;
+		if (!nfs4_init_cp_state(nn, copy))
+			goto out;
+		memcpy(&copy->cp_res.cb_stateid, &copy->cp_stateid,
+			sizeof(copy->cp_stateid));
 		dup_copy_fields(copy, async_copy);
-		memcpy(&copy->cp_res.cb_stateid, &copy->cp_dst_stateid,
-			sizeof(copy->cp_dst_stateid));
 		async_copy->copy_task = kthread_create(nfsd4_do_async_copy,
 				async_copy, "%s", "copy thread");
 		if (IS_ERR(async_copy->copy_task))
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1094ade..3416d29 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -713,6 +713,36 @@ struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *sla
 	return NULL;
 }
 
+/*
+ * Create a unique stateid_t to represent each COPY.
+ */
+int nfs4_init_cp_state(struct nfsd_net *nn, struct nfsd4_copy *copy)
+{
+	int new_id;
+
+	idr_preload(GFP_KERNEL);
+	spin_lock(&nn->s2s_cp_lock);
+	new_id = idr_alloc_cyclic(&nn->s2s_cp_stateids, copy, 0, 0, GFP_NOWAIT);
+	spin_unlock(&nn->s2s_cp_lock);
+	idr_preload_end();
+	if (new_id < 0)
+		return 0;
+	copy->cp_stateid.si_opaque.so_id = new_id;
+	copy->cp_stateid.si_opaque.so_clid.cl_boot = nn->boot_time;
+	copy->cp_stateid.si_opaque.so_clid.cl_id = nn->s2s_cp_cl_id;
+	return 1;
+}
+
+void nfs4_free_cp_state(struct nfsd4_copy *copy)
+{
+	struct nfsd_net *nn;
+
+	nn = net_generic(copy->cp_clp->net, nfsd_net_id);
+	spin_lock(&nn->s2s_cp_lock);
+	idr_remove(&nn->s2s_cp_stateids, copy->cp_stateid.si_opaque.so_id);
+	spin_unlock(&nn->s2s_cp_lock);
+}
+
 static struct nfs4_ol_stateid * nfs4_alloc_open_stateid(struct nfs4_client *clp)
 {
 	struct nfs4_stid *stid;
@@ -7124,6 +7154,8 @@ static int nfs4_state_create_net(struct net *net)
 	INIT_LIST_HEAD(&nn->close_lru);
 	INIT_LIST_HEAD(&nn->del_recall_lru);
 	spin_lock_init(&nn->client_lock);
+	spin_lock_init(&nn->s2s_cp_lock);
+	idr_init(&nn->s2s_cp_stateids);
 
 	spin_lock_init(&nn->blocked_locks_lock);
 	INIT_LIST_HEAD(&nn->blocked_locks_lru);
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index d107b44..63edf68 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1241,6 +1241,7 @@ static __net_init int nfsd_init_net(struct net *net)
 	nn->nfsd4_grace = 90;
 	nn->clverifier_counter = prandom_u32();
 	nn->clientid_counter = prandom_u32();
+	nn->s2s_cp_cl_id = nn->clientid_counter++;
 
 	atomic_set(&nn->ntf_refcnt, 0);
 	init_waitqueue_head(&nn->ntf_wq);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 491030e..d8893427 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -602,6 +602,7 @@ struct nfsd4_blocked_lock {
 
 struct nfsd4_compound_state;
 struct nfsd_net;
+struct nfsd4_copy;
 
 extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, struct svc_fh *fhp,
@@ -611,6 +612,8 @@ __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 		     struct nfs4_stid **s, struct nfsd_net *nn);
 struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *slab,
 				  void (*sc_free)(struct nfs4_stid *));
+int nfs4_init_cp_state(struct nfsd_net *nn, struct nfsd4_copy *copy);
+void nfs4_free_cp_state(struct nfsd4_copy *copy);
 void nfs4_unhash_stid(struct nfs4_stid *s);
 void nfs4_put_stid(struct nfs4_stid *s);
 void nfs4_inc_and_copy_stateid(stateid_t *dst, struct nfs4_stid *stid);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index ed49646..5af9eae 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -538,6 +538,8 @@ struct nfsd4_copy {
 	struct file             *file_src;
 	struct file             *file_dst;
 
+	stateid_t		cp_stateid;
+
 	struct list_head	copies;
 	struct task_struct	*copy_task;
 };
-- 
1.8.3.1

[PATCH v8 7/9] NFSD handle OFFLOAD_CANCEL op

[Olga Kornievskaia]

Upon receiving OFFLOAD_CANCEL search the list of copy stateids,
if found then set the SIGPENDING signal so that do_splice stops
copying and also send kthread_stop to the copy thread to stop
and wait for it. Take a reference on the copy from the
offload_cancel thread so that it won't go away while we are
trying to process it.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4proc.c | 40 +++++++++++++++++++++++++++++++++++++---
 fs/nfsd/state.h    |  1 +
 fs/nfsd/xdr4.h     |  1 +
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9d5f0d0..cebc087 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1100,11 +1100,19 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write)
 out:
 	return status;
 }
+
+void nfs4_put_copy(struct nfsd4_copy *copy)
+{
+	if (!refcount_dec_and_test(&copy->refcount))
+		return;
+	kfree(copy);
+}
+
 static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
 {
 	struct nfsd4_copy *copy = container_of(cb, struct nfsd4_copy, cp_cb);
 
-	kfree(copy);
+	nfs4_put_copy(copy);
 }
 
 static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
@@ -1135,6 +1143,8 @@ static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy)
 	u64 dst_pos = copy->cp_dst_pos;
 
 	do {
+		if (signalled() || kthread_should_stop())
+			break;
 		bytes_copied = nfsd_copy_file_range(copy->file_src, src_pos,
 				copy->file_dst, dst_pos, bytes_total);
 		if (bytes_copied <= 0)
@@ -1188,7 +1198,7 @@ static void cleanup_async_copy(struct nfsd4_copy *copy)
 	spin_lock(&copy->cp_clp->async_lock);
 	list_del(&copy->copies);
 	spin_unlock(&copy->cp_clp->async_lock);
-	kfree(copy);
+	nfs4_put_copy(copy);
 }
 
 static int nfsd4_do_async_copy(void *data)
@@ -1238,6 +1248,7 @@ static int nfsd4_do_async_copy(void *data)
 			goto out;
 		if (!nfs4_init_cp_state(nn, copy))
 			goto out;
+		refcount_set(&async_copy->refcount, 1);
 		memcpy(&copy->cp_res.cb_stateid, &copy->cp_stateid,
 			sizeof(copy->cp_stateid));
 		dup_copy_fields(copy, async_copy);
@@ -1266,7 +1277,30 @@ static int nfsd4_do_async_copy(void *data)
 		     struct nfsd4_compound_state *cstate,
 		     union nfsd4_op_u *u)
 {
-	return 0;
+	struct nfsd4_offload_status *os = &u->offload_status;
+	__be32 status = 0;
+	struct nfsd4_copy *copy;
+	bool found = false;
+	struct nfs4_client *clp = cstate->clp;
+
+	spin_lock(&clp->async_lock);
+	list_for_each_entry(copy, &clp->async_copies, copies) {
+		if (memcmp(&copy->cps->cp_stateid, &os->stateid,
+				NFS4_STATEID_SIZE))
+			continue;
+		found = true;
+		refcount_inc(&copy->refcount);
+		break;
+	}
+	spin_unlock(&clp->async_lock);
+	if (found) {
+		set_tsk_thread_flag(copy->copy_task, TIF_SIGPENDING);
+		kthread_stop(copy->copy_task);
+		nfs4_put_copy(copy);
+	} else
+		status = nfserr_bad_stateid;
+
+	return status;
 }
 
 static __be32
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index d8893427..0ee2ed3 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -641,6 +641,7 @@ extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(const char *name,
 
 struct nfs4_file *find_file(struct knfsd_fh *fh);
 void put_nfs4_file(struct nfs4_file *fi);
+extern void nfs4_put_copy(struct nfsd4_copy *copy);
 static inline void get_nfs4_file(struct nfs4_file *fi)
 {
 	refcount_inc(&fi->fi_ref);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 5af9eae..249d883 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -542,6 +542,7 @@ struct nfsd4_copy {
 
 	struct list_head	copies;
 	struct task_struct	*copy_task;
+	refcount_t		refcount;
 };
 
 struct nfsd4_seek {
-- 
1.8.3.1

[PATCH v8 8/9] NFSD support OFFLOAD_STATUS

[Olga Kornievskaia]

Search the list for the asynchronous copy based on the stateid,
then lookup the number of bytes copied so far.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4proc.c | 45 ++++++++++++++++++++++++++++++++-------------
 fs/nfsd/state.h    |  2 ++
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index cebc087..7e28921 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1272,6 +1272,23 @@ static int nfsd4_do_async_copy(void *data)
 	goto out;
 }
 
+struct nfsd4_copy *
+find_async_copy(struct nfs4_client *clp, stateid_t *stateid)
+{
+	struct nfsd4_copy *copy;
+
+	spin_lock(&clp->async_lock);
+	list_for_each_entry(copy, &clp->async_copies, copies) {
+		if (memcmp(&copy->cp_stateid, stateid, NFS4_STATEID_SIZE))
+			continue;
+		refcount_inc(&copy->refcount);
+		spin_unlock(&clp->async_lock);
+		return copy;
+	}
+	spin_unlock(&clp->async_lock);
+	return NULL;
+}
+
 static __be32
 nfsd4_offload_cancel(struct svc_rqst *rqstp,
 		     struct nfsd4_compound_state *cstate,
@@ -1280,20 +1297,10 @@ static int nfsd4_do_async_copy(void *data)
 	struct nfsd4_offload_status *os = &u->offload_status;
 	__be32 status = 0;
 	struct nfsd4_copy *copy;
-	bool found = false;
 	struct nfs4_client *clp = cstate->clp;
 
-	spin_lock(&clp->async_lock);
-	list_for_each_entry(copy, &clp->async_copies, copies) {
-		if (memcmp(&copy->cps->cp_stateid, &os->stateid,
-				NFS4_STATEID_SIZE))
-			continue;
-		found = true;
-		refcount_inc(&copy->refcount);
-		break;
-	}
-	spin_unlock(&clp->async_lock);
-	if (found) {
+	copy = find_async_copy(clp, &os->stateid);
+	if (copy) {
 		set_tsk_thread_flag(copy->copy_task, TIF_SIGPENDING);
 		kthread_stop(copy->copy_task);
 		nfs4_put_copy(copy);
@@ -1330,7 +1337,19 @@ static int nfsd4_do_async_copy(void *data)
 		     struct nfsd4_compound_state *cstate,
 		     union nfsd4_op_u *u)
 {
-	return nfserr_notsupp;
+	struct nfsd4_offload_status *os = &u->offload_status;
+	__be32 status = 0;
+	struct nfsd4_copy *copy;
+	struct nfs4_client *clp = cstate->clp;
+
+	copy = find_async_copy(clp, &os->stateid);
+	if (copy) {
+		os->count = copy->cp_res.wr_bytes_written;
+		nfs4_put_copy(copy);
+	} else
+		status = nfserr_bad_stateid;
+
+	return status;
 }
 
 static __be32
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 0ee2ed3..0bd4329 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -642,6 +642,8 @@ extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(const char *name,
 struct nfs4_file *find_file(struct knfsd_fh *fh);
 void put_nfs4_file(struct nfs4_file *fi);
 extern void nfs4_put_copy(struct nfsd4_copy *copy);
+extern struct nfsd4_copy *
+find_async_copy(struct nfs4_client *clp, stateid_t *staetid);
 static inline void get_nfs4_file(struct nfs4_file *fi)
 {
 	refcount_inc(&fi->fi_ref);
-- 
1.8.3.1

[PATCH v8 9/9] NFSD stop ongoing async copies on client shutdown

[Olga Kornievskaia]

If client received DESTROY_CLIENTID and there are (for some reason)
on-going async copies, send CLIENT_BUSY error back (eg., client
didn't send OFFLOAD_CANCEL).

If client's lease expired and client structure is being shutdown
and there are on-going async copies, then shutdown the copies.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4proc.c  | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 fs/nfsd/nfs4state.c |  4 +++-
 fs/nfsd/state.h     |  1 +
 fs/nfsd/xdr4.h      |  1 +
 4 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 7e28921..9536813 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1108,6 +1108,51 @@ void nfs4_put_copy(struct nfsd4_copy *copy)
 	kfree(copy);
 }
 
+static bool
+check_and_set_stop_copy(struct nfsd4_copy *copy)
+{
+	bool value;
+
+	spin_lock(&copy->cp_clp->async_lock);
+	value = copy->stopped;
+	if (!copy->stopped)
+		copy->stopped = true;
+	spin_unlock(&copy->cp_clp->async_lock);
+	return value;
+}
+
+static void nfsd4_stop_copy(struct nfsd4_copy *copy)
+{
+	/* only 1 thread should stop the copy */
+	if (!check_and_set_stop_copy(copy)) {
+		set_tsk_thread_flag(copy->copy_task, TIF_SIGPENDING);
+		kthread_stop(copy->copy_task);
+	}
+	nfs4_put_copy(copy);
+}
+
+static struct nfsd4_copy *nfsd4_get_copy(struct nfs4_client *clp)
+{
+	struct nfsd4_copy *copy = NULL;
+
+	spin_lock(&clp->async_lock);
+	if (!list_empty(&clp->async_copies)) {
+		copy = list_first_entry(&clp->async_copies, struct nfsd4_copy,
+					copies);
+		refcount_inc(&copy->refcount);
+	}
+	spin_unlock(&clp->async_lock);
+	return copy;
+}
+
+void nfsd4_shutdown_copy(struct nfs4_client *clp)
+{
+	struct nfsd4_copy *copy;
+
+	while ((copy = nfsd4_get_copy(clp)) != NULL)
+		nfsd4_stop_copy(copy);
+}
+
 static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
 {
 	struct nfsd4_copy *copy = container_of(cb, struct nfsd4_copy, cp_cb);
@@ -1300,11 +1345,9 @@ struct nfsd4_copy *
 	struct nfs4_client *clp = cstate->clp;
 
 	copy = find_async_copy(clp, &os->stateid);
-	if (copy) {
-		set_tsk_thread_flag(copy->copy_task, TIF_SIGPENDING);
-		kthread_stop(copy->copy_task);
-		nfs4_put_copy(copy);
-	} else
+	if (copy)
+		nfsd4_stop_copy(copy);
+	else
 		status = nfserr_bad_stateid;
 
 	return status;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3416d29..2d13d99 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1973,6 +1973,7 @@ static __be32 mark_client_expired_locked(struct nfs4_client *clp)
 		}
 	}
 	nfsd4_return_all_client_layouts(clp);
+	nfsd4_shutdown_copy(clp);
 	nfsd4_shutdown_callback(clp);
 	if (clp->cl_cb_conn.cb_xprt)
 		svc_xprt_put(clp->cl_cb_conn.cb_xprt);
@@ -2503,7 +2504,8 @@ static bool client_has_state(struct nfs4_client *clp)
 		|| !list_empty(&clp->cl_lo_states)
 #endif
 		|| !list_empty(&clp->cl_delegations)
-		|| !list_empty(&clp->cl_sessions);
+		|| !list_empty(&clp->cl_sessions)
+		|| !list_empty(&clp->async_copies);
 }
 
 __be32
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 0bd4329..8db45f9 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -634,6 +634,7 @@ extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
 extern int nfsd4_create_callback_queue(void);
 extern void nfsd4_destroy_callback_queue(void);
 extern void nfsd4_shutdown_callback(struct nfs4_client *);
+extern void nfsd4_shutdown_copy(struct nfs4_client *clp);
 extern void nfsd4_prepare_cb_recall(struct nfs4_delegation *dp);
 extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(const char *name,
 							struct nfsd_net *nn);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 249d883..feeb6d4 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -543,6 +543,7 @@ struct nfsd4_copy {
 	struct list_head	copies;
 	struct task_struct	*copy_task;
 	refcount_t		refcount;
+	bool			stopped;
 };
 
 struct nfsd4_seek {
-- 
1.8.3.1

Re: [PATCH v8 0/9] NFSD support for async COPY

[Christoph Hellwig]

What is the use case for adding all these crazy complications?

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Sat, Apr 14, 2018 at 3:22 AM, Christoph Hellwig <hch@infradead.org> wrote:
> What is the use case for adding all these crazy complications?

We have already provided evidence for improved performance of
asynchronous copy vs synchronous copy. This is also foundation for the
"inter" server-to-server copy which is coming later as it was decided
to add the support in parts.

> --
> 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

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Sat, Apr 14, 2018 at 12:22:02AM -0700, Christoph Hellwig wrote:
> What is the use case for adding all these crazy complications?

Is there anything specific that you think is too complicated?

I know you don't think server-to-server copy offload is worth the
trouble, but I haven't seen you actually explain why (beyond just that
it's more complicated).

Is there some reason you think it won't actually be useful?

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Christoph Hellwig]

On Mon, Apr 16, 2018 at 05:45:22PM -0400, J. Bruce Fields wrote:
> On Sat, Apr 14, 2018 at 12:22:02AM -0700, Christoph Hellwig wrote:
> > What is the use case for adding all these crazy complications?
> 
> Is there anything specific that you think is too complicated?

It is a lot of complexity for little gain.

> I know you don't think server-to-server copy offload is worth the
> trouble, but I haven't seen you actually explain why (beyond just that
> it's more complicated).

I'd like to see numbers for actual, real use cases.  Note that this
series doesn't seem to include inter-server support, so this is locally
only, and I'd like to see why we want to support this over the simpler
and better performing CLONE op.

Also even if we have a good reason to add it I absolutely want a config
option for the feature - it is a lot code adding potential attack
vectors, so we should not just enabled it by default.

> Is there some reason you think it won't actually be useful?

Lets start with explaining why it would actually be useful and benefit
Linux users.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Tue, Apr 17, 2018 at 2:52 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Apr 16, 2018 at 05:45:22PM -0400, J. Bruce Fields wrote:
>> On Sat, Apr 14, 2018 at 12:22:02AM -0700, Christoph Hellwig wrote:
>> > What is the use case for adding all these crazy complications?
>>
>> Is there anything specific that you think is too complicated?
>
> It is a lot of complexity for little gain.
>
>> I know you don't think server-to-server copy offload is worth the
>> trouble, but I haven't seen you actually explain why (beyond just that
>> it's more complicated).
>
> I'd like to see numbers for actual, real use cases.  Note that this
> series doesn't seem to include inter-server support, so this is locally
> only, and I'd like to see why we want to support this over the simpler
> and better performing CLONE op.
>
> Also even if we have a good reason to add it I absolutely want a config
> option for the feature - it is a lot code adding potential attack
> vectors, so we should not just enabled it by default.
>
>> Is there some reason you think it won't actually be useful?
>
> Lets start with explaining why it would actually be useful and benefit
> Linux users.

This is performance improvement feature. Why do you keep asking about
benefits? Besides my employer I had other companies chime in on this
mailing list that they are interested in the copy offload feature
therefore this work is being done.

Yes this series only introduced asynchronous copy offload because the
concern was that doing "inter"+asynchronous was too much to review and
therefore it is being done in parts.

I have no objections to having a configuration option for this
feature. It would be up to the Linux distributions to "make is a
default". Bruce/Anna/Trond, do you want this code to be ifdef-ed under
a config option(s) (one for the client and one for the server)?

> --
> 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

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Tue, Apr 17, 2018 at 09:22:01AM -0400, Olga Kornievskaia wrote:
> On Tue, Apr 17, 2018 at 2:52 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Mon, Apr 16, 2018 at 05:45:22PM -0400, J. Bruce Fields wrote:
> >> On Sat, Apr 14, 2018 at 12:22:02AM -0700, Christoph Hellwig wrote:
> >> > What is the use case for adding all these crazy complications?
> >>
> >> Is there anything specific that you think is too complicated?
> >
> > It is a lot of complexity for little gain.
> >
> >> I know you don't think server-to-server copy offload is worth the
> >> trouble, but I haven't seen you actually explain why (beyond just that
> >> it's more complicated).
> >
> > I'd like to see numbers for actual, real use cases.  Note that this
> > series doesn't seem to include inter-server support, so this is locally
> > only, and I'd like to see why we want to support this over the simpler
> > and better performing CLONE op.
> >
> > Also even if we have a good reason to add it I absolutely want a config
> > option for the feature - it is a lot code adding potential attack
> > vectors, so we should not just enabled it by default.
> >
> >> Is there some reason you think it won't actually be useful?
> >
> > Lets start with explaining why it would actually be useful and benefit
> > Linux users.
> 
> This is performance improvement feature. Why do you keep asking about
> benefits? Besides my employer I had other companies chime in on this
> mailing list that they are interested in the copy offload feature
> therefore this work is being done.

Christoph said: "I'd like to see numbers for actual, real use cases."
Yes, I'd love to see that too.

Is it possible to get testing on real hardware that we'd expect people
to use, with sufficient details about the setup that someone else could
reproduce the results?

> Yes this series only introduced asynchronous copy offload because the
> concern was that doing "inter"+asynchronous was too much to review and
> therefore it is being done in parts.
> 
> I have no objections to having a configuration option for this
> feature. It would be up to the Linux distributions to "make is a
> default". Bruce/Anna/Trond, do you want this code to be ifdef-ed under
> a config option(s) (one for the client and one for the server)?

I could live with that.

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Mon, Apr 16, 2018 at 11:52:03PM -0700, Christoph Hellwig wrote:
> On Mon, Apr 16, 2018 at 05:45:22PM -0400, J. Bruce Fields wrote:
> > On Sat, Apr 14, 2018 at 12:22:02AM -0700, Christoph Hellwig wrote:
> > > What is the use case for adding all these crazy complications?
> > 
> > Is there anything specific that you think is too complicated?
> 
> It is a lot of complexity for little gain.
> 
> > I know you don't think server-to-server copy offload is worth the
> > trouble, but I haven't seen you actually explain why (beyond just that
> > it's more complicated).
> 
> I'd like to see numbers for actual, real use cases.  Note that this
> series doesn't seem to include inter-server support, so this is locally
> only, and I'd like to see why we want to support this over the simpler
> and better performing CLONE op.

Note that we already support COPY (using a read-write loop if
necessary).

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Tue, Apr 17, 2018 at 9:41 AM, J. Bruce Fields <bfields@redhat.com> wrote:
> On Tue, Apr 17, 2018 at 09:22:01AM -0400, Olga Kornievskaia wrote:
>> On Tue, Apr 17, 2018 at 2:52 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> > On Mon, Apr 16, 2018 at 05:45:22PM -0400, J. Bruce Fields wrote:
>> >> On Sat, Apr 14, 2018 at 12:22:02AM -0700, Christoph Hellwig wrote:
>> >> > What is the use case for adding all these crazy complications?
>> >>
>> >> Is there anything specific that you think is too complicated?
>> >
>> > It is a lot of complexity for little gain.
>> >
>> >> I know you don't think server-to-server copy offload is worth the
>> >> trouble, but I haven't seen you actually explain why (beyond just that
>> >> it's more complicated).
>> >
>> > I'd like to see numbers for actual, real use cases.  Note that this
>> > series doesn't seem to include inter-server support, so this is locally
>> > only, and I'd like to see why we want to support this over the simpler
>> > and better performing CLONE op.
>> >
>> > Also even if we have a good reason to add it I absolutely want a config
>> > option for the feature - it is a lot code adding potential attack
>> > vectors, so we should not just enabled it by default.
>> >
>> >> Is there some reason you think it won't actually be useful?
>> >
>> > Lets start with explaining why it would actually be useful and benefit
>> > Linux users.
>>
>> This is performance improvement feature. Why do you keep asking about
>> benefits? Besides my employer I had other companies chime in on this
>> mailing list that they are interested in the copy offload feature
>> therefore this work is being done.
>
> Christoph said: "I'd like to see numbers for actual, real use cases."
> Yes, I'd love to see that too.
>
> Is it possible to get testing on real hardware that we'd expect people
> to use, with sufficient details about the setup that someone else could
> reproduce the results?

The numbers we presented was on real hardware. If you have any more
questions regarding the setup please ask.

>> Yes this series only introduced asynchronous copy offload because the
>> concern was that doing "inter"+asynchronous was too much to review and
>> therefore it is being done in parts.
>>
>> I have no objections to having a configuration option for this
>> feature. It would be up to the Linux distributions to "make is a
>> default". Bruce/Anna/Trond, do you want this code to be ifdef-ed under
>> a config option(s) (one for the client and one for the server)?
>
> I could live with that.
>
> --b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Tue, Apr 17, 2018 at 09:43:45AM -0400, Olga Kornievskaia wrote:
> > On Apr 17, 2018, at 9:41 AM, J. Bruce Fields <bfields@redhat.com> wrote:
> > Christoph said: "I'd like to see numbers for actual, real use cases."
> > Yes, I'd love to see that too.
> > 
> > Is it possible to get testing on real hardware that we'd expect people
> > to use, with sufficient details about the setup that someone else could
> > reproduce the results?
> 
> The numbers we presented was on real hardware. If you have any more questions regarding the setup please ask.

Argh, sorry, you had to remind me once already:

	https://www.ietf.org/mail-archive/web/nfsv4/current/msg15379.html

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Tue, Apr 17, 2018 at 09:57:08AM -0400, J. Bruce Fields wrote:
> On Tue, Apr 17, 2018 at 09:43:45AM -0400, Olga Kornievskaia wrote:
> > > On Apr 17, 2018, at 9:41 AM, J. Bruce Fields <bfields@redhat.com> wrote:
> > > Christoph said: "I'd like to see numbers for actual, real use cases."
> > > Yes, I'd love to see that too.
> > > 
> > > Is it possible to get testing on real hardware that we'd expect people
> > > to use, with sufficient details about the setup that someone else could
> > > reproduce the results?
> > 
> > The numbers we presented was on real hardware. If you have any more questions regarding the setup please ask.
> 
> Argh, sorry, you had to remind me once already:
> 
> 	https://www.ietf.org/mail-archive/web/nfsv4/current/msg15379.html

And there was a digression on the commit behavior, but important point
to me was that it looked like the server-server copy was something close
to what the hardware could do.  (Well, 800-900Mb/s on gigabit, I assume
that's not bad.)

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Tue, Apr 17, 2018 at 9:57 AM, J. Bruce Fields <bfields@redhat.com> wrote:
> On Tue, Apr 17, 2018 at 09:43:45AM -0400, Olga Kornievskaia wrote:
>> > On Apr 17, 2018, at 9:41 AM, J. Bruce Fields <bfields@redhat.com> wrote:
>> > Christoph said: "I'd like to see numbers for actual, real use cases."
>> > Yes, I'd love to see that too.
>> >
>> > Is it possible to get testing on real hardware that we'd expect people
>> > to use, with sufficient details about the setup that someone else could
>> > reproduce the results?
>>
>> The numbers we presented was on real hardware. If you have any more questions regarding the setup please ask.
>
> Argh, sorry, you had to remind me once already:
>
>         https://www.ietf.org/mail-archive/web/nfsv4/current/msg15379.html

Yes that was for the "inter" copy.

Besides those numbers, Jorge also did the numbers for the asynchronous
vs synchronous intra copy. Are you interested in them as well?
Hopefully Jorge still has them.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Tue, Apr 17, 2018 at 10:08 AM, Olga Kornievskaia <aglo@umich.edu> wrote:
> On Tue, Apr 17, 2018 at 9:57 AM, J. Bruce Fields <bfields@redhat.com> wrote:
>> On Tue, Apr 17, 2018 at 09:43:45AM -0400, Olga Kornievskaia wrote:
>>> > On Apr 17, 2018, at 9:41 AM, J. Bruce Fields <bfields@redhat.com> wrote:
>>> > Christoph said: "I'd like to see numbers for actual, real use cases."
>>> > Yes, I'd love to see that too.
>>> >
>>> > Is it possible to get testing on real hardware that we'd expect people
>>> > to use, with sufficient details about the setup that someone else could
>>> > reproduce the results?
>>>
>>> The numbers we presented was on real hardware. If you have any more questions regarding the setup please ask.
>>
>> Argh, sorry, you had to remind me once already:
>>
>>         https://www.ietf.org/mail-archive/web/nfsv4/current/msg15379.html
>
> Yes that was for the "inter" copy.
>
> Besides those numbers, Jorge also did the numbers for the asynchronous
> vs synchronous intra copy. Are you interested in them as well?
> Hopefully Jorge still has them.

I found this in my emails. There is degradation for file size <- 8MB
and increase in performance for >16MB

Here are the numbers (each test is run 10 times and the average is displayed):

/home/mora/logs/nfstest_ssc_20171015174051.log:    synchronous SSC
/home/mora/logs/nfstest_ssc_20171015171323.log:    asynchronous SSC

Performance degradation for file size <= 8MB:

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
1KB file: 0.162018680573 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
1KB file: 0.636984395981 seconds
                                                         74.56%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
2KB file: 0.177096033096 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
2KB file: 0.583548688889 seconds
                                                         69.65%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
4KB file: 0.207866406441 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
4KB file: 0.782712388039 seconds
                                                         73.44%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
8KB file: 0.201173448563 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
8KB file: 0.692702102661 seconds
                                                         70.96%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
16KB file: 0.216166472435 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
16KB file: 0.56289691925 seconds
                                                         61.60%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
32KB file: 0.185425901413 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
32KB file: 0.65691485405 seconds
                                                         71.77%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
64KB file: 0.238445782661 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
64KB file: 0.525265741348 seconds
                                                         54.60%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
128KB file: 0.166219258308 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
128KB file: 0.69602959156 seconds
                                                         76.12%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
256KB file: 0.192826724052 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
256KB file: 0.556627941132 seconds
                                                         65.36%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
512KB file: 0.230020523071 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
512KB file: 0.496951031685 seconds
                                                         53.71%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
1MB file: 0.225317955017 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
1MB file: 0.50447602272 seconds
                                                         55.34%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
2MB file: 0.233719205856 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
2MB file: 0.570275163651 seconds
                                                         59.02%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
4MB file: 0.285868096352 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
4MB file: 0.656079149246 seconds
                                                         56.43%
performance degradation by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
8MB file: 0.509396600723 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
8MB file: 0.696055078506 seconds
                                                         26.82%
performance degradation by async


Performance improvement for file size >= 16MB:

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
16MB file: 0.960887002945 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
16MB file: 0.817601919174 seconds
                                                         14.91%
performance improvement by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
32MB file: 1.58274662495 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
32MB file: 1.24578406811 seconds
                                                         21.29%
performance improvement by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
64MB file: 2.54690010548 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
64MB file: 1.58639280796 seconds
                                                         37.71%
performance improvement by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
128MB file: 4.2253510952 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
128MB file: 2.58433949947 seconds
                                                         38.84%
performance improvement by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
256MB file: 7.71305201054 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
256MB file: 4.43859291077 seconds
                                                         42.45%
performance improvement by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
512MB file: 14.6445164919 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
512MB file: 8.18448216915 seconds
                                                         44.11%
performance improvement by async

/home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
1GB file: 28.7619983673 seconds
/home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
1GB file: 16.1399238825 seconds
                                                         43.88%
performance improvement by async

Re: [PATCH v8 0/9] NFSD support for async COPY

[Steve Dickson]

On 04/17/2018 09:22 AM, Olga Kornievskaia wrote:
> On Tue, Apr 17, 2018 at 2:52 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> On Mon, Apr 16, 2018 at 05:45:22PM -0400, J. Bruce Fields wrote:
>>> On Sat, Apr 14, 2018 at 12:22:02AM -0700, Christoph Hellwig wrote:
>>>> What is the use case for adding all these crazy complications?
>>>
>>> Is there anything specific that you think is too complicated?
>>
>> It is a lot of complexity for little gain.
>>
>>> I know you don't think server-to-server copy offload is worth the
>>> trouble, but I haven't seen you actually explain why (beyond just that
>>> it's more complicated).
>>
>> I'd like to see numbers for actual, real use cases.  Note that this
>> series doesn't seem to include inter-server support, so this is locally
>> only, and I'd like to see why we want to support this over the simpler
>> and better performing CLONE op.
>>
>> Also even if we have a good reason to add it I absolutely want a config
>> option for the feature - it is a lot code adding potential attack
>> vectors, so we should not just enabled it by default.
>>
>>> Is there some reason you think it won't actually be useful?
>>
>> Lets start with explaining why it would actually be useful and benefit
>> Linux users.
> 
> This is performance improvement feature. Why do you keep asking about
> benefits? Besides my employer I had other companies chime in on this
> mailing list that they are interested in the copy offload feature
> therefore this work is being done.
+1 I guess I don't see why people would object to a performance
improvement... Do you?

> 
> Yes this series only introduced asynchronous copy offload because the
> concern was that doing "inter"+asynchronous was too much to review and
> therefore it is being done in parts.
Which is a solid plan... IMHO... But the bottom line is this... 

A decision needs to be made... Either accept these patches/concept or don't!
This discussion has been going on quite a while now... If this
performance improvement is not something the community wants
so be bit.. Make the call... so we can move on... 
 
> 
> I have no objections to having a configuration option for this
> feature. It would be up to the Linux distributions to "make is a
> default". Bruce/Anna/Trond, do you want this code to be ifdef-ed under
> a config option(s) (one for the client and one for the server)?
Way is this needed? Why wouldn't users want a performance gain on
by default??

steved.
 

Re: [PATCH v8 0/9] NFSD support for async COPY

[Anna Schumaker]

On 04/17/2018 10:13 AM, Olga Kornievskaia wrote:
> On Tue, Apr 17, 2018 at 10:08 AM, Olga Kornievskaia <aglo@umich.edu> wrote:
>> On Tue, Apr 17, 2018 at 9:57 AM, J. Bruce Fields <bfields@redhat.com> wrote:
>>> On Tue, Apr 17, 2018 at 09:43:45AM -0400, Olga Kornievskaia wrote:
>>>>> On Apr 17, 2018, at 9:41 AM, J. Bruce Fields <bfields@redhat.com> wrote:
>>>>> Christoph said: "I'd like to see numbers for actual, real use cases."
>>>>> Yes, I'd love to see that too.
>>>>>
>>>>> Is it possible to get testing on real hardware that we'd expect people
>>>>> to use, with sufficient details about the setup that someone else could
>>>>> reproduce the results?
>>>>
>>>> The numbers we presented was on real hardware. If you have any more questions regarding the setup please ask.
>>>
>>> Argh, sorry, you had to remind me once already:
>>>
>>>         https://www.ietf.org/mail-archive/web/nfsv4/current/msg15379.html
>>
>> Yes that was for the "inter" copy.
>>
>> Besides those numbers, Jorge also did the numbers for the asynchronous
>> vs synchronous intra copy. Are you interested in them as well?
>> Hopefully Jorge still has them.
> 
> I found this in my emails. There is degradation for file size <- 8MB
> and increase in performance for >16MB

Right now we have an arbitrary sychronous copy limit of 4MB.  Could we bump that to 8MB, and then have the server choose to do copies less than this limit as synchronous?

As for theconfig optionquestion, my preference is to at least have the client side enabled by default.

Anna

> 
> Here are the numbers (each test is run 10 times and the average is displayed):
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    synchronous SSC
> /home/mora/logs/nfstest_ssc_20171015171323.log:    asynchronous SSC
> 
> Performance degradation for file size <= 8MB:
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 1KB file: 0.162018680573 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 1KB file: 0.636984395981 seconds
>                                                          74.56%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 2KB file: 0.177096033096 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 2KB file: 0.583548688889 seconds
>                                                          69.65%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 4KB file: 0.207866406441 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 4KB file: 0.782712388039 seconds
>                                                          73.44%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 8KB file: 0.201173448563 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 8KB file: 0.692702102661 seconds
>                                                          70.96%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 16KB file: 0.216166472435 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 16KB file: 0.56289691925 seconds
>                                                          61.60%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 32KB file: 0.185425901413 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 32KB file: 0.65691485405 seconds
>                                                          71.77%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 64KB file: 0.238445782661 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 64KB file: 0.525265741348 seconds
>                                                          54.60%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 128KB file: 0.166219258308 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 128KB file: 0.69602959156 seconds
>                                                          76.12%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 256KB file: 0.192826724052 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 256KB file: 0.556627941132 seconds
>                                                          65.36%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 512KB file: 0.230020523071 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 512KB file: 0.496951031685 seconds
>                                                          53.71%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 1MB file: 0.225317955017 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 1MB file: 0.50447602272 seconds
>                                                          55.34%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 2MB file: 0.233719205856 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 2MB file: 0.570275163651 seconds
>                                                          59.02%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 4MB file: 0.285868096352 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 4MB file: 0.656079149246 seconds
>                                                          56.43%
> performance degradation by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 8MB file: 0.509396600723 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 8MB file: 0.696055078506 seconds
>                                                          26.82%
> performance degradation by async
> 
> 
> Performance improvement for file size >= 16MB:
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 16MB file: 0.960887002945 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 16MB file: 0.817601919174 seconds
>                                                          14.91%
> performance improvement by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 32MB file: 1.58274662495 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 32MB file: 1.24578406811 seconds
>                                                          21.29%
> performance improvement by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 64MB file: 2.54690010548 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 64MB file: 1.58639280796 seconds
>                                                          37.71%
> performance improvement by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 128MB file: 4.2253510952 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 128MB file: 2.58433949947 seconds
>                                                          38.84%
> performance improvement by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 256MB file: 7.71305201054 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 256MB file: 4.43859291077 seconds
>                                                          42.45%
> performance improvement by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 512MB file: 14.6445164919 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 512MB file: 8.18448216915 seconds
>                                                          44.11%
> performance improvement by async
> 
> /home/mora/logs/nfstest_ssc_20171015174051.log:    PASS: SSC copy of
> 1GB file: 28.7619983673 seconds
> /home/mora/logs/nfstest_ssc_20171015171323.log:    PASS: SSC copy of
> 1GB file: 16.1399238825 seconds
>                                                          43.88%
> performance improvement by async
> --
> 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
> 

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Mon, Apr 16, 2018 at 11:52:03PM -0700, Christoph Hellwig wrote:
> Also even if we have a good reason to add it I absolutely want a config
> option for the feature - it is a lot code adding potential attack
> vectors, so we should not just enabled it by default.

By the way, am I forgetting some mitigation or can a client provide any
address it wants as the source server to copy from?

That opens up the server to a lot of the same risks you'd see from
unprivileged NFS mounts--a malicious client could copy from a server
under it's control that's modified to exploit bugs in the server's NFS
client code.

I wonder if there's also the possibility of weird results even without
malicious intent: e.g. a user copying files between two different
servers may unintentionally tie up resources on the target server.

There's interest in enabling unprivileged NFS mounts to allow
unprivileged containers to do NFS mounts.  But even if we get to the
point where we're comfortable enabling that, distributions may still
want it off by default, and may advise admins to do firewalling that
restricts the set of NFS servers that containers can mount.

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

> On Apr 17, 2018, at 11:00 AM, J. Bruce Fields <bfields@redhat.com> =
wrote:
>=20
> On Mon, Apr 16, 2018 at 11:52:03PM -0700, Christoph Hellwig wrote:
>> Also even if we have a good reason to add it I absolutely want a =
config
>> option for the feature - it is a lot code adding potential attack
>> vectors, so we should not just enabled it by default.
>=20
> By the way, am I forgetting some mitigation or can a client provide =
any
> address it wants as the source server to copy from?
>=20
> That opens up the server to a lot of the same risks you'd see from
> unprivileged NFS mounts--a malicious client could copy from a server
> under it's control that's modified to exploit bugs in the server's NFS
> client code.

There is nothing in the specification that can protects from a malicious =
user to initiate a copy. There is GSSv3 that were added to prevent =
unprivileged server from accessing information from the source server =
without the user=E2=80=99s knowledge. I don=E2=80=99t see how copy =
offload is the same as unprivileged NFS mount. What copy offload feature =
offers is ability to READ a very specific file.=20

We do have in the pipe line GSSv3 implementation as well as the COPY =
piece that uses it. It was implemented by Andy and inherited by Anna. =
However, before we could submit that we need the copy offload to go in.

> I wonder if there's also the possibility of weird results even without
> malicious intent: e.g. a user copying files between two different
> servers may unintentionally tie up resources on the target server.

Is the target server in your case a target of the copy? The copy is =
initiated and will use resource of the copy source server and the copy =
destination server until either copy is done or client cancels the copy =
or client=E2=80=99s lease expires on the target copy server.=20

> There's interest in enabling unprivileged NFS mounts to allow
> unprivileged containers to do NFS mounts.  But even if we get to the
> point where we're comfortable enabling that, distributions may still
> want it off by default, and may advise admins to do firewalling that
> restricts the set of NFS servers that containers can mount.
>=20
> --b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Tue, Apr 17, 2018 at 11:17:03AM -0400, Olga Kornievskaia wrote:
> 
> 
> > On Apr 17, 2018, at 11:00 AM, J. Bruce Fields <bfields@redhat.com>
> > wrote:
> > 
> > On Mon, Apr 16, 2018 at 11:52:03PM -0700, Christoph Hellwig wrote:
> >> Also even if we have a good reason to add it I absolutely want a
> >> config option for the feature - it is a lot code adding potential
> >> attack vectors, so we should not just enabled it by default.
> > 
> > By the way, am I forgetting some mitigation or can a client provide
> > any address it wants as the source server to copy from?
> > 
> > That opens up the server to a lot of the same risks you'd see from
> > unprivileged NFS mounts--a malicious client could copy from a server
> > under it's control that's modified to exploit bugs in the server's
> > NFS client code.
> 
> There is nothing in the specification that can protects from a
> malicious user to initiate a copy. There is GSSv3 that were added to
> prevent unprivileged server from accessing information from the source
> server without the user’s knowledge. I don’t see how copy offload is
> the same as unprivileged NFS mount. What copy offload feature offers
> is ability to READ a very specific file. 

I can't remember how much more protocol you need besides READ....  If
you support 4.1+ as the copy protocol then you probably also need to do
EXCHANGE_ID and CREATE_SESSION?  Fair enough, I agree that you're not
exposing as much of the client code as an unprivileged mount would, but
it's still a lot of opportunities for something to go wrong.

> We do have in the pipe line GSSv3 implementation as well as the COPY
> piece that uses it. It was implemented by Andy and inherited by Anna.
> However, before we could submit that we need the copy offload to go
> in.

A malicious client can give the server credentials that a server under
its control will accept.  GSSv3 lets the source server authenticate the
read requests, but I don't think it helps here.

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

So, could we just have a global run-time configuration option?  Maybe a
module parameter, defaulting to off.

Then an admin can decide to turn it on, or to turn it on only after
installing some firewall rules to restrict the source of copies.

Another odd option might be to restrict to copies from files on
filesystems that server already has mounted.  Maybe that would simplify
the copy code too.  I think that's too inflexible, though.

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Tue, Apr 17, 2018 at 11:41 AM, J. Bruce Fields <bfields@redhat.com> wrot=
e:
> On Tue, Apr 17, 2018 at 11:17:03AM -0400, Olga Kornievskaia wrote:
>>
>>
>> > On Apr 17, 2018, at 11:00 AM, J. Bruce Fields <bfields@redhat.com>
>> > wrote:
>> >
>> > On Mon, Apr 16, 2018 at 11:52:03PM -0700, Christoph Hellwig wrote:
>> >> Also even if we have a good reason to add it I absolutely want a
>> >> config option for the feature - it is a lot code adding potential
>> >> attack vectors, so we should not just enabled it by default.
>> >
>> > By the way, am I forgetting some mitigation or can a client provide
>> > any address it wants as the source server to copy from?
>> >
>> > That opens up the server to a lot of the same risks you'd see from
>> > unprivileged NFS mounts--a malicious client could copy from a server
>> > under it's control that's modified to exploit bugs in the server's
>> > NFS client code.
>>
>> There is nothing in the specification that can protects from a
>> malicious user to initiate a copy. There is GSSv3 that were added to
>> prevent unprivileged server from accessing information from the source
>> server without the user=E2=80=99s knowledge. I don=E2=80=99t see how cop=
y offload is
>> the same as unprivileged NFS mount. What copy offload feature offers
>> is ability to READ a very specific file.
>
> I can't remember how much more protocol you need besides READ....  If
> you support 4.1+ as the copy protocol then you probably also need to do
> EXCHANGE_ID and CREATE_SESSION?  Fair enough, I agree that you're not
> exposing as much of the client code as an unprivileged mount would, but
> it's still a lot of opportunities for something to go wrong.

The spec has an option that I think in general is not very useful. We
did not implement in the "inter" series of patches. When the
COPY_NOTIFY is sent the client specifies destination server's IP
address(es). The source server could potentially then modify its
export policy to also allow access from those addresses.

The reason it's restrictive is even illustrated by the RFC itself. It
provides a possible scenario where source and destination servers have
a high-speed network that client isn't not aware of and it is not the
network the client connects to the destination server. Therefore
client can't provide the "real" IP address that the destination server
would utilize to do access the source server and read the file.

So I see your concern that in order to allow for the destination
server to read the file from the source server, the source server must
allow client_id/session creation and that actually really leads to
being able to send any other compound to the source server.

>> We do have in the pipe line GSSv3 implementation as well as the COPY
>> piece that uses it. It was implemented by Andy and inherited by Anna.
>> However, before we could submit that we need the copy offload to go
>> in.
>
> A malicious client can give the server credentials that a server under
> its control will accept.  GSSv3 lets the source server authenticate the
> read requests, but I don't think it helps here.

Btw, what your security thread here? If the client has control over
the server, then what are you trying to protect? If the client
controls the source server, then it can read whatever is stored on it
and if it decides to provide same ability to anybody else why would
that matter? How's any different from giving away your password to
whomever and them accessing files as that user?

>
> --b.
> --
> 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

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Tue, Apr 17, 2018 at 12:15:13PM -0400, Olga Kornievskaia wrote:
> So I see your concern that in order to allow for the destination
> server to read the file from the source server, the source server must
> allow client_id/session creation and that actually really leads to
> being able to send any other compound to the source server.

That may be, but I wasn't actually worrying about the source server, I
was worrying about the target:

> Btw, what your security thread here? If the client has control over
> the server, then what are you trying to protect? If the client
> controls the source server, then it can read whatever is stored on it
> and if it decides to provide same ability to anybody else why would
> that matter? How's any different from giving away your password to
> whomever and them accessing files as that user?

I assume the attacker knows a vunlerability in the Linux NFS client code
that processes READ (or EXCHANGE_ID or CREATE_SESSION) replies.

It sends a COPY request to an NFS server that tells it copy a file from
a "server" that the attacker controls.  The victim NFS server then tries
to read from the attacker's server, which sends replies that exploit the
vulnerability.

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Tue, Apr 17, 2018 at 11:58:29AM -0400, J. Bruce Fields wrote:
> So, could we just have a global run-time configuration option?  Maybe a
> module parameter, defaulting to off.

Also if we do that I don't think we'd really need a build-time
configuration option any more.

--b.

Re: [PATCH v8 7/9] NFSD handle OFFLOAD_CANCEL op

[Anna Schumaker]

Hi Olga,

On 04/13/2018 01:01 PM, Olga Kornievskaia wrote:
> Upon receiving OFFLOAD_CANCEL search the list of copy stateids,
> if found then set the SIGPENDING signal so that do_splice stops
> copying and also send kthread_stop to the copy thread to stop
> and wait for it. Take a reference on the copy from the
> offload_cancel thread so that it won't go away while we are
> trying to process it.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfsd/nfs4proc.c | 40 +++++++++++++++++++++++++++++++++++++---
>  fs/nfsd/state.h    |  1 +
>  fs/nfsd/xdr4.h     |  1 +
>  3 files changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 9d5f0d0..cebc087 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1100,11 +1100,19 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write)
>  out:
>  	return status;
>  }
> +
> +void nfs4_put_copy(struct nfsd4_copy *copy)
> +{
> +	if (!refcount_dec_and_test(&copy->refcount))
> +		return;
> +	kfree(copy);
> +}
> +
>  static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
>  {
>  	struct nfsd4_copy *copy = container_of(cb, struct nfsd4_copy, cp_cb);
>  
> -	kfree(copy);
> +	nfs4_put_copy(copy);
>  }
>  
>  static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
> @@ -1135,6 +1143,8 @@ static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy)
>  	u64 dst_pos = copy->cp_dst_pos;
>  
>  	do {
> +		if (signalled() || kthread_should_stop())
> +			break;
>  		bytes_copied = nfsd_copy_file_range(copy->file_src, src_pos,
>  				copy->file_dst, dst_pos, bytes_total);
>  		if (bytes_copied <= 0)
> @@ -1188,7 +1198,7 @@ static void cleanup_async_copy(struct nfsd4_copy *copy)
>  	spin_lock(&copy->cp_clp->async_lock);
>  	list_del(&copy->copies);
>  	spin_unlock(&copy->cp_clp->async_lock);
> -	kfree(copy);
> +	nfs4_put_copy(copy);
>  }
>  
>  static int nfsd4_do_async_copy(void *data)
> @@ -1238,6 +1248,7 @@ static int nfsd4_do_async_copy(void *data)
>  			goto out;
>  		if (!nfs4_init_cp_state(nn, copy))
>  			goto out;
> +		refcount_set(&async_copy->refcount, 1);
>  		memcpy(&copy->cp_res.cb_stateid, &copy->cp_stateid,
>  			sizeof(copy->cp_stateid));
>  		dup_copy_fields(copy, async_copy);
> @@ -1266,7 +1277,30 @@ static int nfsd4_do_async_copy(void *data)
>  		     struct nfsd4_compound_state *cstate,
>  		     union nfsd4_op_u *u)
>  {
> -	return 0;
> +	struct nfsd4_offload_status *os = &u->offload_status;
> +	__be32 status = 0;
> +	struct nfsd4_copy *copy;
> +	bool found = false;
> +	struct nfs4_client *clp = cstate->clp;
> +
> +	spin_lock(&clp->async_lock);
> +	list_for_each_entry(copy, &clp->async_copies, copies) {
> +		if (memcmp(&copy->cps->cp_stateid, &os->stateid,
> +				NFS4_STATEID_SIZE))

I'm having trouble compiling this patch:  

fs/nfsd/nfs4proc.c: In function 'nfsd4_offload_cancel':
fs/nfsd/nfs4proc.c:1288:21: error: 'struct nfsd4_copy' has no member named 'cps'; did you mean 'cp_res'?
   if (memcmp(&copy->cps->cp_stateid, &os->stateid,
                     ^~~
                     cp_res

Is something out of order here?

Anna

> +			continue;
> +		found = true;
> +		refcount_inc(&copy->refcount);
> +		break;
> +	}
> +	spin_unlock(&clp->async_lock);
> +	if (found) {
> +		set_tsk_thread_flag(copy->copy_task, TIF_SIGPENDING);
> +		kthread_stop(copy->copy_task);
> +		nfs4_put_copy(copy);
> +	} else
> +		status = nfserr_bad_stateid;
> +
> +	return status;
>  }
>  
>  static __be32
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index d8893427..0ee2ed3 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -641,6 +641,7 @@ extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(const char *name,
>  
>  struct nfs4_file *find_file(struct knfsd_fh *fh);
>  void put_nfs4_file(struct nfs4_file *fi);
> +extern void nfs4_put_copy(struct nfsd4_copy *copy);
>  static inline void get_nfs4_file(struct nfs4_file *fi)
>  {
>  	refcount_inc(&fi->fi_ref);
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 5af9eae..249d883 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -542,6 +542,7 @@ struct nfsd4_copy {
>  
>  	struct list_head	copies;
>  	struct task_struct	*copy_task;
> +	refcount_t		refcount;
>  };
>  
>  struct nfsd4_seek {
> 

Re: [PATCH v8 7/9] NFSD handle OFFLOAD_CANCEL op

[Olga Kornievskaia]

> On Apr 17, 2018, at 2:06 PM, Anna Schumaker <schumaker.anna@gmail.com> =
wrote:
>=20
> Hi Olga,
>=20
> On 04/13/2018 01:01 PM, Olga Kornievskaia wrote:
>> Upon receiving OFFLOAD_CANCEL search the list of copy stateids,
>> if found then set the SIGPENDING signal so that do_splice stops
>> copying and also send kthread_stop to the copy thread to stop
>> and wait for it. Take a reference on the copy from the
>> offload_cancel thread so that it won't go away while we are
>> trying to process it.
>>=20
>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>> ---
>> fs/nfsd/nfs4proc.c | 40 +++++++++++++++++++++++++++++++++++++---
>> fs/nfsd/state.h    |  1 +
>> fs/nfsd/xdr4.h     |  1 +
>> 3 files changed, 39 insertions(+), 3 deletions(-)
>>=20
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 9d5f0d0..cebc087 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1100,11 +1100,19 @@ static int fill_in_write_vector(struct kvec =
*vec, struct nfsd4_write *write)
>> out:
>> 	return status;
>> }
>> +
>> +void nfs4_put_copy(struct nfsd4_copy *copy)
>> +{
>> +	if (!refcount_dec_and_test(&copy->refcount))
>> +		return;
>> +	kfree(copy);
>> +}
>> +
>> static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
>> {
>> 	struct nfsd4_copy *copy =3D container_of(cb, struct nfsd4_copy, =
cp_cb);
>>=20
>> -	kfree(copy);
>> +	nfs4_put_copy(copy);
>> }
>>=20
>> static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
>> @@ -1135,6 +1143,8 @@ static ssize_t _nfsd_copy_file_range(struct =
nfsd4_copy *copy)
>> 	u64 dst_pos =3D copy->cp_dst_pos;
>>=20
>> 	do {
>> +		if (signalled() || kthread_should_stop())
>> +			break;
>> 		bytes_copied =3D nfsd_copy_file_range(copy->file_src, =
src_pos,
>> 				copy->file_dst, dst_pos, bytes_total);
>> 		if (bytes_copied <=3D 0)
>> @@ -1188,7 +1198,7 @@ static void cleanup_async_copy(struct =
nfsd4_copy *copy)
>> 	spin_lock(&copy->cp_clp->async_lock);
>> 	list_del(&copy->copies);
>> 	spin_unlock(&copy->cp_clp->async_lock);
>> -	kfree(copy);
>> +	nfs4_put_copy(copy);
>> }
>>=20
>> static int nfsd4_do_async_copy(void *data)
>> @@ -1238,6 +1248,7 @@ static int nfsd4_do_async_copy(void *data)
>> 			goto out;
>> 		if (!nfs4_init_cp_state(nn, copy))
>> 			goto out;
>> +		refcount_set(&async_copy->refcount, 1);
>> 		memcpy(&copy->cp_res.cb_stateid, &copy->cp_stateid,
>> 			sizeof(copy->cp_stateid));
>> 		dup_copy_fields(copy, async_copy);
>> @@ -1266,7 +1277,30 @@ static int nfsd4_do_async_copy(void *data)
>> 		     struct nfsd4_compound_state *cstate,
>> 		     union nfsd4_op_u *u)
>> {
>> -	return 0;
>> +	struct nfsd4_offload_status *os =3D &u->offload_status;
>> +	__be32 status =3D 0;
>> +	struct nfsd4_copy *copy;
>> +	bool found =3D false;
>> +	struct nfs4_client *clp =3D cstate->clp;
>> +
>> +	spin_lock(&clp->async_lock);
>> +	list_for_each_entry(copy, &clp->async_copies, copies) {
>> +		if (memcmp(&copy->cps->cp_stateid, &os->stateid,
>> +				NFS4_STATEID_SIZE))
>=20
> I'm having trouble compiling this patch: =20
>=20
> fs/nfsd/nfs4proc.c: In function 'nfsd4_offload_cancel':
> fs/nfsd/nfs4proc.c:1288:21: error: 'struct nfsd4_copy' has no member =
named 'cps'; did you mean 'cp_res'?
>   if (memcmp(&copy->cps->cp_stateid, &os->stateid,
>                     ^~~
>                     cp_res
>=20
> Is something out of order here?
>=20

Yep that=E2=80=99s the one I told you about that kbuilt have sent me and =
I have a fix for. I was waiting for more comments before sending =
corrections. Patches 7 and consequently 8 are effected.


> Anna
>=20
>> +			continue;
>> +		found =3D true;
>> +		refcount_inc(&copy->refcount);
>> +		break;
>> +	}
>> +	spin_unlock(&clp->async_lock);
>> +	if (found) {
>> +		set_tsk_thread_flag(copy->copy_task, TIF_SIGPENDING);
>> +		kthread_stop(copy->copy_task);
>> +		nfs4_put_copy(copy);
>> +	} else
>> +		status =3D nfserr_bad_stateid;
>> +
>> +	return status;
>> }
>>=20
>> static __be32
>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>> index d8893427..0ee2ed3 100644
>> --- a/fs/nfsd/state.h
>> +++ b/fs/nfsd/state.h
>> @@ -641,6 +641,7 @@ extern struct nfs4_client_reclaim =
*nfs4_client_to_reclaim(const char *name,
>>=20
>> struct nfs4_file *find_file(struct knfsd_fh *fh);
>> void put_nfs4_file(struct nfs4_file *fi);
>> +extern void nfs4_put_copy(struct nfsd4_copy *copy);
>> static inline void get_nfs4_file(struct nfs4_file *fi)
>> {
>> 	refcount_inc(&fi->fi_ref);
>> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
>> index 5af9eae..249d883 100644
>> --- a/fs/nfsd/xdr4.h
>> +++ b/fs/nfsd/xdr4.h
>> @@ -542,6 +542,7 @@ struct nfsd4_copy {
>>=20
>> 	struct list_head	copies;
>> 	struct task_struct	*copy_task;
>> +	refcount_t		refcount;
>> };
>>=20
>> struct nfsd4_seek {
>>=20

Re: [PATCH v8 7/9] NFSD handle OFFLOAD_CANCEL op

[Olga Kornievskaia]

On Tue, Apr 17, 2018 at 2:06 PM, Anna Schumaker
<schumaker.anna@gmail.com> wrote:
> Hi Olga,
>
> On 04/13/2018 01:01 PM, Olga Kornievskaia wrote:
>> Upon receiving OFFLOAD_CANCEL search the list of copy stateids,
>> if found then set the SIGPENDING signal so that do_splice stops
>> copying and also send kthread_stop to the copy thread to stop
>> and wait for it. Take a reference on the copy from the
>> offload_cancel thread so that it won't go away while we are
>> trying to process it.
>>
>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>> ---
>>  fs/nfsd/nfs4proc.c | 40 +++++++++++++++++++++++++++++++++++++---
>>  fs/nfsd/state.h    |  1 +
>>  fs/nfsd/xdr4.h     |  1 +
>>  3 files changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 9d5f0d0..cebc087 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1100,11 +1100,19 @@ static int fill_in_write_vector(struct kvec *vec=
, struct nfsd4_write *write)
>>  out:
>>       return status;
>>  }
>> +
>> +void nfs4_put_copy(struct nfsd4_copy *copy)
>> +{
>> +     if (!refcount_dec_and_test(&copy->refcount))
>> +             return;
>> +     kfree(copy);
>> +}
>> +
>>  static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
>>  {
>>       struct nfsd4_copy *copy =3D container_of(cb, struct nfsd4_copy, cp=
_cb);
>>
>> -     kfree(copy);
>> +     nfs4_put_copy(copy);
>>  }
>>
>>  static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
>> @@ -1135,6 +1143,8 @@ static ssize_t _nfsd_copy_file_range(struct nfsd4_=
copy *copy)
>>       u64 dst_pos =3D copy->cp_dst_pos;
>>
>>       do {
>> +             if (signalled() || kthread_should_stop())
>> +                     break;
>>               bytes_copied =3D nfsd_copy_file_range(copy->file_src, src_=
pos,
>>                               copy->file_dst, dst_pos, bytes_total);
>>               if (bytes_copied <=3D 0)
>> @@ -1188,7 +1198,7 @@ static void cleanup_async_copy(struct nfsd4_copy *=
copy)
>>       spin_lock(&copy->cp_clp->async_lock);
>>       list_del(&copy->copies);
>>       spin_unlock(&copy->cp_clp->async_lock);
>> -     kfree(copy);
>> +     nfs4_put_copy(copy);
>>  }
>>
>>  static int nfsd4_do_async_copy(void *data)
>> @@ -1238,6 +1248,7 @@ static int nfsd4_do_async_copy(void *data)
>>                       goto out;
>>               if (!nfs4_init_cp_state(nn, copy))
>>                       goto out;
>> +             refcount_set(&async_copy->refcount, 1);
>>               memcpy(&copy->cp_res.cb_stateid, &copy->cp_stateid,
>>                       sizeof(copy->cp_stateid));
>>               dup_copy_fields(copy, async_copy);
>> @@ -1266,7 +1277,30 @@ static int nfsd4_do_async_copy(void *data)
>>                    struct nfsd4_compound_state *cstate,
>>                    union nfsd4_op_u *u)
>>  {
>> -     return 0;
>> +     struct nfsd4_offload_status *os =3D &u->offload_status;
>> +     __be32 status =3D 0;
>> +     struct nfsd4_copy *copy;
>> +     bool found =3D false;
>> +     struct nfs4_client *clp =3D cstate->clp;
>> +
>> +     spin_lock(&clp->async_lock);
>> +     list_for_each_entry(copy, &clp->async_copies, copies) {
>> +             if (memcmp(&copy->cps->cp_stateid, &os->stateid,
>> +                             NFS4_STATEID_SIZE))
>
> I'm having trouble compiling this patch:
>
> fs/nfsd/nfs4proc.c: In function 'nfsd4_offload_cancel':
> fs/nfsd/nfs4proc.c:1288:21: error: 'struct nfsd4_copy' has no member name=
d 'cps'; did you mean 'cp_res'?
>    if (memcmp(&copy->cps->cp_stateid, &os->stateid,
>                      ^~~
>                      cp_res
>
> Is something out of order here?

Yep that=E2=80=99s the one I told you about that kbuilt have sent me and I
have a fix for. I was waiting for more comments before sending
corrections. Patches 7 and consequently 8 are effected.

>
> Anna
>
>> +                     continue;
>> +             found =3D true;
>> +             refcount_inc(&copy->refcount);
>> +             break;
>> +     }
>> +     spin_unlock(&clp->async_lock);
>> +     if (found) {
>> +             set_tsk_thread_flag(copy->copy_task, TIF_SIGPENDING);
>> +             kthread_stop(copy->copy_task);
>> +             nfs4_put_copy(copy);
>> +     } else
>> +             status =3D nfserr_bad_stateid;
>> +
>> +     return status;
>>  }
>>
>>  static __be32
>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>> index d8893427..0ee2ed3 100644
>> --- a/fs/nfsd/state.h
>> +++ b/fs/nfsd/state.h
>> @@ -641,6 +641,7 @@ extern struct nfs4_client_reclaim *nfs4_client_to_re=
claim(const char *name,
>>
>>  struct nfs4_file *find_file(struct knfsd_fh *fh);
>>  void put_nfs4_file(struct nfs4_file *fi);
>> +extern void nfs4_put_copy(struct nfsd4_copy *copy);
>>  static inline void get_nfs4_file(struct nfs4_file *fi)
>>  {
>>       refcount_inc(&fi->fi_ref);
>> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
>> index 5af9eae..249d883 100644
>> --- a/fs/nfsd/xdr4.h
>> +++ b/fs/nfsd/xdr4.h
>> @@ -542,6 +542,7 @@ struct nfsd4_copy {
>>
>>       struct list_head        copies;
>>       struct task_struct      *copy_task;
>> +     refcount_t              refcount;
>>  };
>>
>>  struct nfsd4_seek {
>>
> --
> 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

Re: [PATCH v8 0/9] NFSD support for async COPY

[Christoph Hellwig]

On Sat, Apr 14, 2018 at 08:32:48AM -0400, Olga Kornievskaia wrote:
> On Sat, Apr 14, 2018 at 3:22 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > What is the use case for adding all these crazy complications?
> 
> We have already provided evidence for improved performance of
> asynchronous copy vs synchronous copy. This is also foundation for the
> "inter" server-to-server copy which is coming later as it was decided
> to add the support in parts.

Well, show your evidence in the introduction of the series then.  And
synchronous copy is not the target - clone is.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Christoph Hellwig]

On Mon, Apr 16, 2018 at 05:45:22PM -0400, J. Bruce Fields wrote:
> On Sat, Apr 14, 2018 at 12:22:02AM -0700, Christoph Hellwig wrote:
> > What is the use case for adding all these crazy complications?
> 
> Is there anything specific that you think is too complicated?
> 
> I know you don't think server-to-server copy offload is worth the
> trouble, but I haven't seen you actually explain why (beyond just that
> it's more complicated).

For one all code is buggy and introduces issues, and code implementing
asynchronous, distributed state machines is even more so.  So a large
chunk of code is a big warning sign.  Especially if it isn't actually
useful to normal users.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Christoph Hellwig]

On Tue, Apr 17, 2018 at 11:19:25AM -0400, Olga Kornievskaia wrote:
> Yes I agree. Let’s please decide if this will go in (with whatever code improvements are required) or let’s drop it.

Well, my vote is very clearly to drop it.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Wed, Apr 18, 2018 at 3:08 AM, Christoph Hellwig <hch@infradead.org> wrot=
e:
> On Tue, Apr 17, 2018 at 11:19:25AM -0400, Olga Kornievskaia wrote:
>> Yes I agree. Let=E2=80=99s please decide if this will go in (with whatev=
er code improvements are required) or let=E2=80=99s drop it.
>
> Well, my vote is very clearly to drop it.

Bruce, when will you make a decision about this? Is there something
more that needs to happen before it can be decided if the "async"
patches are moving forward (and then "inter" patches).

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Tue, Apr 24, 2018 at 04:29:14PM -0400, Olga Kornievskaia wrote:
> On Wed, Apr 18, 2018 at 3:08 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Tue, Apr 17, 2018 at 11:19:25AM -0400, Olga Kornievskaia wrote:
> >> Yes I agree. Let’s please decide if this will go in (with whatever code improvements are required) or let’s drop it.
> >
> > Well, my vote is very clearly to drop it.
> 
> Bruce, when will you make a decision about this? Is there something
> more that needs to happen before it can be decided if the "async"
> patches are moving forward (and then "inter" patches).

I'm OK with the patches.

It could help to have some more information about actual customer use
cases: who specifically is asking for this, and what about their
situation makes them believe they'll benefit?

But to me it seems obvious that server-to-server copy will be faster in
some cases as long there's not some screwup preventing it from using the
server-to-server bandwidth (and your numbers don't show any).  So I'm
not terribly worried about this.

If we wanted to simplify I think we could ditch the asynchronous
protocol and still make server-to-server copy work as a series of
synchronous calls.  (Or maybe that would make getting good performance
the complicated part.)

The only security issue I'm worried about is the fact that you can make
it try to copy from any arbitrary IP address.  I'd be satisfied if we
document the issue and make server-to-server-copy support require a
runtime switch that defaults to off.  (And with that in place I don't
see a need to also provide a build option.)

--b.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Fri, Apr 27, 2018 at 12:03 PM, J. Bruce Fields <bfields@redhat.com> wrot=
e:
> On Tue, Apr 24, 2018 at 04:29:14PM -0400, Olga Kornievskaia wrote:
>> On Wed, Apr 18, 2018 at 3:08 AM, Christoph Hellwig <hch@infradead.org> w=
rote:
>> > On Tue, Apr 17, 2018 at 11:19:25AM -0400, Olga Kornievskaia wrote:
>> >> Yes I agree. Let=E2=80=99s please decide if this will go in (with wha=
tever code improvements are required) or let=E2=80=99s drop it.
>> >
>> > Well, my vote is very clearly to drop it.
>>
>> Bruce, when will you make a decision about this? Is there something
>> more that needs to happen before it can be decided if the "async"
>> patches are moving forward (and then "inter" patches).
>
> I'm OK with the patches.
>
> It could help to have some more information about actual customer use
> cases: who specifically is asking for this, and what about their
> situation makes them believe they'll benefit?

I'm really not involved with customer or know of how exactly they will
benefit. I have some knowledge of some company that is interested in
using copy offload functionality in game development. I have no
details. It has been talked about a case scenario of copying VM
images. I don't know if VMware uses copy offload or not.

> But to me it seems obvious that server-to-server copy will be faster in
> some cases as long there's not some screwup preventing it from using the
> server-to-server bandwidth (and your numbers don't show any).  So I'm
> not terribly worried about this.
>
> If we wanted to simplify I think we could ditch the asynchronous
> protocol and still make server-to-server copy work as a series of
> synchronous calls.  (Or maybe that would make getting good performance
> the complicated part.)

I'm not in favor of dropping asynchronous piece as I think it's an
important performance improvement. It's likely it won't be must of an
improvement due to an overhead of establishing clientid/session for
every "chuck" of the copy that will be sent synchronously.

> The only security issue I'm worried about is the fact that you can make
> it try to copy from any arbitrary IP address.  I'd be satisfied if we
> document the issue and make server-to-server-copy support require a
> runtime switch that defaults to off.  (And with that in place I don't
> see a need to also provide a build option.)

Ok, runtime option, I'll work on it.

Re: [PATCH v8 0/9] NFSD support for async COPY

[Olga Kornievskaia]

On Fri, Apr 27, 2018 at 7:11 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> On Fri, Apr 27, 2018 at 12:03 PM, J. Bruce Fields <bfields@redhat.com> wr=
ote:
>> On Tue, Apr 24, 2018 at 04:29:14PM -0400, Olga Kornievskaia wrote:
>>> On Wed, Apr 18, 2018 at 3:08 AM, Christoph Hellwig <hch@infradead.org> =
wrote:
>>> > On Tue, Apr 17, 2018 at 11:19:25AM -0400, Olga Kornievskaia wrote:
>>> >> Yes I agree. Let=E2=80=99s please decide if this will go in (with wh=
atever code improvements are required) or let=E2=80=99s drop it.
>>> >
>>> > Well, my vote is very clearly to drop it.
>>>
>>> Bruce, when will you make a decision about this? Is there something
>>> more that needs to happen before it can be decided if the "async"
>>> patches are moving forward (and then "inter" patches).
>>
>> I'm OK with the patches.
>>
>> It could help to have some more information about actual customer use
>> cases: who specifically is asking for this, and what about their
>> situation makes them believe they'll benefit?
>
> I'm really not involved with customer or know of how exactly they will
> benefit. I have some knowledge of some company that is interested in
> using copy offload functionality in game development. I have no
> details. It has been talked about a case scenario of copying VM
> images. I don't know if VMware uses copy offload or not.
>
>> But to me it seems obvious that server-to-server copy will be faster in
>> some cases as long there's not some screwup preventing it from using the
>> server-to-server bandwidth (and your numbers don't show any).  So I'm
>> not terribly worried about this.
>>
>> If we wanted to simplify I think we could ditch the asynchronous
>> protocol and still make server-to-server copy work as a series of
>> synchronous calls.  (Or maybe that would make getting good performance
>> the complicated part.)
>
> I'm not in favor of dropping asynchronous piece as I think it's an
> important performance improvement. It's likely it won't be must of an
> improvement due to an overhead of establishing clientid/session for
> every "chuck" of the copy that will be sent synchronously.
>
>> The only security issue I'm worried about is the fact that you can make
>> it try to copy from any arbitrary IP address.  I'd be satisfied if we
>> document the issue and make server-to-server-copy support require a
>> runtime switch that defaults to off.  (And with that in place I don't
>> see a need to also provide a build option.)
>
> Ok, runtime option, I'll work on it.

Hi Bruce,

I would like to come back to this code and hopefully make progress.

I heard about the LSF discussion that there was interest in the copy
offload and async code. If so where do we stand now and what are the
next steps now?

Re: [PATCH v8 0/9] NFSD support for async COPY

[J. Bruce Fields]

On Tue, May 22, 2018 at 05:05:10PM -0400, Olga Kornievskaia wrote:
> On Fri, Apr 27, 2018 at 7:11 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> > On Fri, Apr 27, 2018 at 12:03 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> >> On Tue, Apr 24, 2018 at 04:29:14PM -0400, Olga Kornievskaia wrote:
> >>> On Wed, Apr 18, 2018 at 3:08 AM, Christoph Hellwig <hch@infradead.org> wrote:
> >>> > On Tue, Apr 17, 2018 at 11:19:25AM -0400, Olga Kornievskaia wrote:
> >>> >> Yes I agree. Let’s please decide if this will go in (with whatever code improvements are required) or let’s drop it.
> >>> >
> >>> > Well, my vote is very clearly to drop it.
> >>>
> >>> Bruce, when will you make a decision about this? Is there something
> >>> more that needs to happen before it can be decided if the "async"
> >>> patches are moving forward (and then "inter" patches).
> >>
> >> I'm OK with the patches.
> >>
> >> It could help to have some more information about actual customer use
> >> cases: who specifically is asking for this, and what about their
> >> situation makes them believe they'll benefit?
> >
> > I'm really not involved with customer or know of how exactly they will
> > benefit. I have some knowledge of some company that is interested in
> > using copy offload functionality in game development. I have no
> > details. It has been talked about a case scenario of copying VM
> > images. I don't know if VMware uses copy offload or not.
> >
> >> But to me it seems obvious that server-to-server copy will be faster in
> >> some cases as long there's not some screwup preventing it from using the
> >> server-to-server bandwidth (and your numbers don't show any).  So I'm
> >> not terribly worried about this.
> >>
> >> If we wanted to simplify I think we could ditch the asynchronous
> >> protocol and still make server-to-server copy work as a series of
> >> synchronous calls.  (Or maybe that would make getting good performance
> >> the complicated part.)
> >
> > I'm not in favor of dropping asynchronous piece as I think it's an
> > important performance improvement. It's likely it won't be must of an
> > improvement due to an overhead of establishing clientid/session for
> > every "chuck" of the copy that will be sent synchronously.
> >
> >> The only security issue I'm worried about is the fact that you can make
> >> it try to copy from any arbitrary IP address.  I'd be satisfied if we
> >> document the issue and make server-to-server-copy support require a
> >> runtime switch that defaults to off.  (And with that in place I don't
> >> see a need to also provide a build option.)
> >
> > Ok, runtime option, I'll work on it.
> 
> Hi Bruce,
> 
> I would like to come back to this code and hopefully make progress.
> 
> I heard about the LSF discussion that there was interest in the copy
> offload and async code. If so where do we stand now and what are the
> next steps now?

I actually don't remember that discussion, apologies.

Next steps might be for you to add the runtime option and resend?  And
then I need to give it another read.

--b.