diff mbox series

NFSv4.2: Rework scratch handling for READ_PLUS

Message ID 20230407204826.1274206-1-anna@kernel.org (mailing list archive)
State New, archived
Headers show
Series NFSv4.2: Rework scratch handling for READ_PLUS | expand

Commit Message

Anna Schumaker April 7, 2023, 8:48 p.m. UTC 
From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Instead of using a tiny scratch buffer, we should use a full scratch
page to match how other NFSv4 operations handle scratch data. This patch
also lets us set the scratch page before decoding any part of the
READ_PLUS operation instead of setting the buffer right before segment
decoding, which feels a little more robust to me.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs42xdr.c       |  4 ++--
 fs/nfs/nfs4proc.c       | 17 ++++++++++++-----
 include/linux/nfs_xdr.h |  1 +
 3 files changed, 15 insertions(+), 7 deletions(-)

Comments

Trond Myklebust April 17, 2023, 8:30 p.m. UTC | #1 
On Fri, 2023-04-07 at 16:48 -0400, Anna Schumaker wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> Instead of using a tiny scratch buffer, we should use a full scratch
> page to match how other NFSv4 operations handle scratch data. This
> patch
> also lets us set the scratch page before decoding any part of the
> READ_PLUS operation instead of setting the buffer right before
> segment
> decoding, which feels a little more robust to me.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> ---
>  fs/nfs/nfs42xdr.c       |  4 ++--
>  fs/nfs/nfs4proc.c       | 17 ++++++++++++-----
>  include/linux/nfs_xdr.h |  1 +
>  3 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> index d80ee88ca996..702567d5b1db 100644
> --- a/fs/nfs/nfs42xdr.c
> +++ b/fs/nfs/nfs42xdr.c
> @@ -1122,7 +1122,6 @@ static int decode_read_plus(struct xdr_stream
> *xdr, struct nfs_pgio_res *res)
>         uint32_t segments;
>         struct read_plus_segment *segs;
>         int status, i;
> -       char scratch_buf[16];
>         __be32 *p;
>  
>         status = decode_op_hdr(xdr, OP_READ_PLUS);
> @@ -1143,7 +1142,6 @@ static int decode_read_plus(struct xdr_stream
> *xdr, struct nfs_pgio_res *res)
>         if (!segs)
>                 return -ENOMEM;
>  
> -       xdr_set_scratch_buffer(xdr, &scratch_buf,
> sizeof(scratch_buf));
>         status = -EIO;
>         for (i = 0; i < segments; i++) {
>                 status = decode_read_plus_segment(xdr, &segs[i]);
> @@ -1348,6 +1346,8 @@ static int nfs4_xdr_dec_read_plus(struct
> rpc_rqst *rqstp,
>         struct compound_hdr hdr;
>         int status;
>  
> +       xdr_set_scratch_page(xdr, res->scratch);
> +
>         status = decode_compound_hdr(xdr, &hdr);
>         if (status)
>                 goto out;
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 5607b1e2b821..2b5e62713bdd 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5439,6 +5439,8 @@ static bool nfs4_read_plus_not_supported(struct
> rpc_task *task,
>  
>  static int nfs4_read_done(struct rpc_task *task, struct
> nfs_pgio_header *hdr)
>  {
> +       if (hdr->res.scratch)
> +               __free_page(hdr->res.scratch);
>         if (!nfs4_sequence_done(task, &hdr->res.seq_res))
>                 return -EAGAIN;
>         if (nfs4_read_stateid_changed(task, &hdr->args))
> @@ -5452,17 +5454,22 @@ static int nfs4_read_done(struct rpc_task
> *task, struct nfs_pgio_header *hdr)
>  }
>  
>  #if defined CONFIG_NFS_V4_2 && defined CONFIG_NFS_V4_2_READ_PLUS
> -static void nfs42_read_plus_support(struct nfs_pgio_header *hdr,
> +static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr,
>                                     struct rpc_message *msg)
>  {
>         /* Note: We don't use READ_PLUS with pNFS yet */
> -       if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) &&
> !hdr->ds_clp)
> +       if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) &&
> !hdr->ds_clp) {
>                 msg->rpc_proc =
> &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS];
> +               hdr->res.scratch = alloc_page(GFP_KERNEL);
> +               return hdr->res.scratch != NULL;

Hmm... Does it make sense to do this if we're only requesting a read of
a few bytes? Perhaps we should look at a minimum size when deciding
whether or not to do read plus?

> +       }
> +       return false;
>  }
>  #else
> -static void nfs42_read_plus_support(struct nfs_pgio_header *hdr,
> +static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr,
>                                     struct rpc_message *msg)
>  {
> +       return false;
>  }
>  #endif /* CONFIG_NFS_V4_2 */
>  
> @@ -5472,8 +5479,8 @@ static void nfs4_proc_read_setup(struct
> nfs_pgio_header *hdr,
>         hdr->timestamp   = jiffies;
>         if (!hdr->pgio_done_cb)
>                 hdr->pgio_done_cb = nfs4_read_done_cb;
> -       msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> -       nfs42_read_plus_support(hdr, msg);
> +       if (!nfs42_read_plus_support(hdr, msg))
> +               msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
>         nfs4_init_sequence(&hdr->args.seq_args, &hdr->res.seq_res, 0,
> 0);
>  }
>  
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index e86cf6642d21..6d821aaf0b1a 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -670,6 +670,7 @@ struct nfs_pgio_res {
>                 struct {
>                         unsigned int            replen;         /*
> used by read */
>                         int                     eof;            /*
> used by read */
> +                       struct page             *scratch;       /*
> used by read */
>                 };
>                 struct {
>                         struct nfs_writeverf *  verf;           /*
> used by write */
diff mbox series

Patch

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index d80ee88ca996..702567d5b1db 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -1122,7 +1122,6 @@  static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
 	uint32_t segments;
 	struct read_plus_segment *segs;
 	int status, i;
-	char scratch_buf[16];
 	__be32 *p;
 
 	status = decode_op_hdr(xdr, OP_READ_PLUS);
@@ -1143,7 +1142,6 @@  static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
 	if (!segs)
 		return -ENOMEM;
 
-	xdr_set_scratch_buffer(xdr, &scratch_buf, sizeof(scratch_buf));
 	status = -EIO;
 	for (i = 0; i < segments; i++) {
 		status = decode_read_plus_segment(xdr, &segs[i]);
@@ -1348,6 +1346,8 @@  static int nfs4_xdr_dec_read_plus(struct rpc_rqst *rqstp,
 	struct compound_hdr hdr;
 	int status;
 
+	xdr_set_scratch_page(xdr, res->scratch);
+
 	status = decode_compound_hdr(xdr, &hdr);
 	if (status)
 		goto out;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5607b1e2b821..2b5e62713bdd 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5439,6 +5439,8 @@  static bool nfs4_read_plus_not_supported(struct rpc_task *task,
 
 static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
 {
+	if (hdr->res.scratch)
+		__free_page(hdr->res.scratch);
 	if (!nfs4_sequence_done(task, &hdr->res.seq_res))
 		return -EAGAIN;
 	if (nfs4_read_stateid_changed(task, &hdr->args))
@@ -5452,17 +5454,22 @@  static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
 }
 
 #if defined CONFIG_NFS_V4_2 && defined CONFIG_NFS_V4_2_READ_PLUS
-static void nfs42_read_plus_support(struct nfs_pgio_header *hdr,
+static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr,
 				    struct rpc_message *msg)
 {
 	/* Note: We don't use READ_PLUS with pNFS yet */
-	if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) && !hdr->ds_clp)
+	if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) && !hdr->ds_clp) {
 		msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS];
+		hdr->res.scratch = alloc_page(GFP_KERNEL);
+		return hdr->res.scratch != NULL;
+	}
+	return false;
 }
 #else
-static void nfs42_read_plus_support(struct nfs_pgio_header *hdr,
+static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr,
 				    struct rpc_message *msg)
 {
+	return false;
 }
 #endif /* CONFIG_NFS_V4_2 */
 
@@ -5472,8 +5479,8 @@  static void nfs4_proc_read_setup(struct nfs_pgio_header *hdr,
 	hdr->timestamp   = jiffies;
 	if (!hdr->pgio_done_cb)
 		hdr->pgio_done_cb = nfs4_read_done_cb;
-	msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
-	nfs42_read_plus_support(hdr, msg);
+	if (!nfs42_read_plus_support(hdr, msg))
+		msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
 	nfs4_init_sequence(&hdr->args.seq_args, &hdr->res.seq_res, 0, 0);
 }
 
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index e86cf6642d21..6d821aaf0b1a 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -670,6 +670,7 @@  struct nfs_pgio_res {
 		struct {
 			unsigned int		replen;		/* used by read */
 			int			eof;		/* used by read */
+			struct page		*scratch;	/* used by read */
 		};
 		struct {
 			struct nfs_writeverf *	verf;		/* used by write */