[v4,04/12] NFS OFFLOAD_STATUS op
diff mbox

Message ID 20170928172819.50703-5-kolga@netapp.com
State New
Headers show

Commit Message

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

Comments

Anna Schumaker Sept. 28, 2017, 8:32 p.m. UTC | #1
Hi Olga,

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

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

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

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

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

Same question about the nfs42_offload_status_res here.

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

Anna

> +{
> +	struct nfs_server *dst_server = NFS_SERVER(file_inode(dst));
> +	struct nfs4_exception exception = { };
> +	int status;
> +
> +	if (!(dst_server->caps & NFS_CAP_OFFLOAD_STATUS))
> +		return -EOPNOTSUPP;
> +
> +	do {
> +		status = _nfs42_proc_offload_status(dst, stateid, res);
> +		if (status == -ENOTSUPP)
> +			return -EOPNOTSUPP;
> +		status = nfs4_handle_exception(dst_server, status, &exception);
> +	} while (exception.retry);
> +
> +	return status;
> +}
> +
>  static loff_t _nfs42_proc_llseek(struct file *filep,
>  		struct nfs_lock_context *lock, loff_t offset, int whence)
>  {
> 
--
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
Olga Kornievskaia Sept. 28, 2017, 8:41 p.m. UTC | #2
On Thu, Sep 28, 2017 at 4:32 PM, Anna Schumaker
<Anna.Schumaker@netapp.com> wrote:
> Hi Olga,
>
> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>> ---
>>  fs/nfs/nfs42.h     |  5 ++++-
>>  fs/nfs/nfs42proc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h
>> index b6cd153..6b9decf 100644
>> --- a/fs/nfs/nfs42.h
>> +++ b/fs/nfs/nfs42.h
>> @@ -11,6 +11,7 @@
>>   */
>>  #define PNFS_LAYOUTSTATS_MAXDEV (4)
>>
>> +#if defined(CONFIG_NFS_V4_2)
>>  /* nfs4.2proc.c */
>>  int nfs42_proc_allocate(struct file *, loff_t, loff_t);
>>  ssize_t nfs42_proc_copy(struct file *, loff_t, struct file *, loff_t, size_t);
>> @@ -19,5 +20,7 @@
>>  int nfs42_proc_layoutstats_generic(struct nfs_server *,
>>                                  struct nfs42_layoutstat_data *);
>>  int nfs42_proc_clone(struct file *, struct file *, loff_t, loff_t, loff_t);
>> -
>> +int nfs42_proc_offload_status(struct file *, nfs4_stateid *,
>> +                           struct nfs42_offload_status_res *);
>> +#endif /* CONFIG_NFS_V4_2) */
>>  #endif /* __LINUX_FS_NFS_NFS4_2_H */
>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>> index 6c2db51..5c42e71 100644
>> --- a/fs/nfs/nfs42proc.c
>> +++ b/fs/nfs/nfs42proc.c
>> @@ -263,6 +263,48 @@ ssize_t nfs42_proc_copy(struct file *src, loff_t pos_src,
>>       return err;
>>  }
>>
>> +int _nfs42_proc_offload_status(struct file *dst, nfs4_stateid *stateid,
>> +                             struct nfs42_offload_status_res *res)
>
> Does res have to be passed as a pointer here?  The struct is fairly small, so it couldn't it be allocated on the stack?

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

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

Got it.

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

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

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

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

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

Patch
diff mbox

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