NFSD: De-duplicate the svc_fill_write_vector() call sites
authorChuck Lever <chuck.lever@oracle.com>
Fri, 9 May 2025 17:39:23 +0000 (13:39 -0400)
committerChuck Lever <chuck.lever@oracle.com>
Thu, 15 May 2025 20:16:23 +0000 (16:16 -0400)
All three call sites do the same thing.

I'm struggling with this a bit, however. struct xdr_buf is an XDR
layer object and unmarshaling a WRITE payload is clearly a task
intended to be done by the proc and xdr functions, not by VFS. This
feels vaguely like a layering violation.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
fs/nfsd/nfs3proc.c
fs/nfsd/nfs4proc.c
fs/nfsd/nfsproc.c
fs/nfsd/vfs.c
fs/nfsd/vfs.h
include/linux/sunrpc/svc.h
net/sunrpc/svc.c

index 8902fae8c62d29ab80c5bf8bd79285531c639658..12e1eef810e77ca7f75fbd02a6fe93efa04eea0e 100644 (file)
@@ -220,7 +220,6 @@ nfsd3_proc_write(struct svc_rqst *rqstp)
        struct nfsd3_writeargs *argp = rqstp->rq_argp;
        struct nfsd3_writeres *resp = rqstp->rq_resp;
        unsigned long cnt = argp->len;
-       unsigned int nvecs;
 
        dprintk("nfsd: WRITE(3)    %s %d bytes at %Lu%s\n",
                                SVCFH_fmt(&argp->fh),
@@ -235,10 +234,8 @@ nfsd3_proc_write(struct svc_rqst *rqstp)
 
        fh_copy(&resp->fh, &argp->fh);
        resp->committed = argp->stable;
-       nvecs = svc_fill_write_vector(rqstp, &argp->payload);
-
        resp->status = nfsd_write(rqstp, &resp->fh, argp->offset,
-                                 rqstp->rq_vec, nvecs, &cnt,
+                                 &argp->payload, &cnt,
                                  resp->committed, resp->verf);
        resp->count = cnt;
        resp->status = nfsd3_map_status(resp->status);
index 2b16ee1ae461672a502f8c716f144c8c8bbfdf0b..ffd8b1d499dfe24d4e6a74aa9446441c4870c715 100644 (file)
@@ -1216,7 +1216,6 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
        struct nfsd_file *nf = NULL;
        __be32 status = nfs_ok;
        unsigned long cnt;
-       int nvecs;
 
        if (write->wr_offset > (u64)OFFSET_MAX ||
            write->wr_offset + write->wr_buflen > (u64)OFFSET_MAX)
@@ -1231,12 +1230,9 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
                return status;
 
        write->wr_how_written = write->wr_stable_how;
-
-       nvecs = svc_fill_write_vector(rqstp, &write->wr_payload);
-
        status = nfsd_vfs_write(rqstp, &cstate->current_fh, nf,
-                               write->wr_offset, rqstp->rq_vec, nvecs, &cnt,
-                               write->wr_how_written,
+                               write->wr_offset, &write->wr_payload,
+                               &cnt, write->wr_how_written,
                                (__be32 *)write->wr_verifier.data);
        nfsd_file_put(nf);
 
index 7c573d792252a608bdbe7fa8352d395d86e64dd6..65cbe04184f31f25fcdf9b002daad8883d2775b9 100644 (file)
@@ -251,17 +251,14 @@ nfsd_proc_write(struct svc_rqst *rqstp)
        struct nfsd_writeargs *argp = rqstp->rq_argp;
        struct nfsd_attrstat *resp = rqstp->rq_resp;
        unsigned long cnt = argp->len;
-       unsigned int nvecs;
 
        dprintk("nfsd: WRITE    %s %u bytes at %d\n",
                SVCFH_fmt(&argp->fh),
                argp->len, argp->offset);
 
-       nvecs = svc_fill_write_vector(rqstp, &argp->payload);
-
-       resp->status = nfsd_write(rqstp, fh_copy(&resp->fh, &argp->fh),
-                                 argp->offset, rqstp->rq_vec, nvecs,
-                                 &cnt, NFS_DATA_SYNC, NULL);
+       fh_copy(&resp->fh, &argp->fh);
+       resp->status = nfsd_write(rqstp, &resp->fh, argp->offset,
+                                 &argp->payload, &cnt, NFS_DATA_SYNC, NULL);
        if (resp->status == nfs_ok)
                resp->status = fh_getattr(&resp->fh, &resp->stat);
        else if (resp->status == nfserr_jukebox)
index 7cfd26dec5a8ac6ff2a9a6fe30f4d052e946ad79..43ecc5ae0c3f12af0fa49b1c29a6235a5ad78587 100644 (file)
@@ -1143,11 +1143,27 @@ static int wait_for_concurrent_writes(struct file *file)
        return err;
 }
 
+/**
+ * nfsd_vfs_write - write data to an already-open file
+ * @rqstp: RPC execution context
+ * @fhp: File handle of file to write into
+ * @nf: An open file matching @fhp
+ * @offset: Byte offset of start
+ * @payload: xdr_buf containing the write payload
+ * @cnt: IN: number of bytes to write, OUT: number of bytes actually written
+ * @stable: An NFS stable_how value
+ * @verf: NFS WRITE verifier
+ *
+ * Upon return, caller must invoke fh_put on @fhp.
+ *
+ * Return values:
+ *   An nfsstat value in network byte order.
+ */
 __be32
-nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
-                               loff_t offset, struct kvec *vec, int vlen,
-                               unsigned long *cnt, int stable,
-                               __be32 *verf)
+nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
+              struct nfsd_file *nf, loff_t offset,
+              const struct xdr_buf *payload, unsigned long *cnt,
+              int stable, __be32 *verf)
 {
        struct nfsd_net         *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
        struct file             *file = nf->nf_file;
@@ -1162,6 +1178,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
        unsigned int            pflags = current->flags;
        rwf_t                   flags = 0;
        bool                    restore_flags = false;
+       unsigned int            nvecs;
 
        trace_nfsd_write_opened(rqstp, fhp, offset, *cnt);
 
@@ -1189,7 +1206,8 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
        if (stable && !fhp->fh_use_wgather)
                flags |= RWF_SYNC;
 
-       iov_iter_kvec(&iter, ITER_SOURCE, vec, vlen, *cnt);
+       nvecs = svc_fill_write_vector(rqstp, payload);
+       iov_iter_kvec(&iter, ITER_SOURCE, rqstp->rq_vec, nvecs, *cnt);
        since = READ_ONCE(file->f_wb_err);
        if (verf)
                nfsd_copy_write_verifier(verf, nn);
@@ -1289,14 +1307,24 @@ __be32 nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
        return err;
 }
 
-/*
- * Write data to a file.
- * The stable flag requests synchronous writes.
- * N.B. After this call fhp needs an fh_put
+/**
+ * nfsd_write - open a file and write data to it
+ * @rqstp: RPC execution context
+ * @fhp: File handle of file to write into; nfsd_write() may modify it
+ * @offset: Byte offset of start
+ * @payload: xdr_buf containing the write payload
+ * @cnt: IN: number of bytes to write, OUT: number of bytes actually written
+ * @stable: An NFS stable_how value
+ * @verf: NFS WRITE verifier
+ *
+ * Upon return, caller must invoke fh_put on @fhp.
+ *
+ * Return values:
+ *   An nfsstat value in network byte order.
  */
 __be32
 nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset,
-          struct kvec *vec, int vlen, unsigned long *cnt, int stable,
+          const struct xdr_buf *payload, unsigned long *cnt, int stable,
           __be32 *verf)
 {
        struct nfsd_file *nf;
@@ -1308,8 +1336,8 @@ nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset,
        if (err)
                goto out;
 
-       err = nfsd_vfs_write(rqstp, fhp, nf, offset, vec,
-                       vlen, cnt, stable, verf);
+       err = nfsd_vfs_write(rqstp, fhp, nf, offset, payload, cnt,
+                            stable, verf);
        nfsd_file_put(nf);
 out:
        trace_nfsd_write_done(rqstp, fhp, offset, *cnt);
index f9b09b842856614f21c4c10f41ce09c414f8c0b7..eff04959606fe55c141ab4a2eed97c7e0716a5f5 100644 (file)
@@ -128,13 +128,13 @@ bool              nfsd_read_splice_ok(struct svc_rqst *rqstp);
 __be32         nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
                                loff_t offset, unsigned long *count,
                                u32 *eof);
-__be32                 nfsd_write(struct svc_rqst *, struct svc_fh *, loff_t,
-                               struct kvec *, int, unsigned long *,
-                               int stable, __be32 *verf);
+__be32         nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
+                               loff_t offset, const struct xdr_buf *payload,
+                               unsigned long *cnt, int stable, __be32 *verf);
 __be32         nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
                                struct nfsd_file *nf, loff_t offset,
-                               struct kvec *vec, int vlen, unsigned long *cnt,
-                               int stable, __be32 *verf);
+                               const struct xdr_buf *payload,
+                               unsigned long *cnt, int stable, __be32 *verf);
 __be32         nfsd_readlink(struct svc_rqst *, struct svc_fh *,
                                char *, int *);
 __be32         nfsd_symlink(struct svc_rqst *, struct svc_fh *,
index 538bea716c6b1ea85759143fb1971fe82c4214e2..dec636345ee22c3990845b9d623b7ee28e91209b 100644 (file)
@@ -471,7 +471,7 @@ int            svc_encode_result_payload(struct svc_rqst *rqstp,
                                             unsigned int offset,
                                             unsigned int length);
 unsigned int      svc_fill_write_vector(struct svc_rqst *rqstp,
-                                        struct xdr_buf *payload);
+                                        const struct xdr_buf *payload);
 char             *svc_fill_symlink_pathname(struct svc_rqst *rqstp,
                                             struct kvec *first, void *p,
                                             size_t total);
index 2c1c4aa93f43fe689f774b944fda72d6b9d6c716..a8861a80bc04c5d4cbfe4cec038411439ff2674c 100644 (file)
@@ -1727,10 +1727,10 @@ EXPORT_SYMBOL_GPL(svc_encode_result_payload);
  * Fills in rqstp::rq_vec, and returns the number of elements.
  */
 unsigned int svc_fill_write_vector(struct svc_rqst *rqstp,
-                                  struct xdr_buf *payload)
+                                  const struct xdr_buf *payload)
 {
+       const struct kvec *first = payload->head;
        struct page **pages = payload->pages;
-       struct kvec *first = payload->head;
        struct kvec *vec = rqstp->rq_vec;
        size_t total = payload->len;
        unsigned int i;