* [PATCH 1/3] nfsd: call svc_fill_write_vector from nfsd_vfs_write
2025-05-07 11:55 remove rq_vec Christoph Hellwig
@ 2025-05-07 11:55 ` Christoph Hellwig
2025-05-08 23:08 ` kernel test robot
2025-05-07 11:55 ` [PATCH 2/3] nfsd: use rq_bvec instead of rq_vec in nfsd_vfs_write Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2025-05-07 11:55 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker
Cc: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs
Creating the kvec is always done so that nfsd_vfs_write can use them.
Move the call there Instead of doing it in the first and second degree
callers, which just makes the code more complicated.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/nfsd/nfs3proc.c | 5 +----
fs/nfsd/nfs4proc.c | 7 +------
fs/nfsd/nfsproc.c | 7 ++-----
fs/nfsd/vfs.c | 18 +++++++++++-------
fs/nfsd/vfs.h | 4 ++--
5 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 372bdcf5e07a..3eecaabd96ce 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -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);
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b397246dae7b..2b814fc4c8cc 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1211,7 +1211,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)
@@ -1226,12 +1225,8 @@ 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);
- WARN_ON_ONCE(nvecs > ARRAY_SIZE(rqstp->rq_vec));
-
status = nfsd_vfs_write(rqstp, &cstate->current_fh, nf,
- write->wr_offset, rqstp->rq_vec, nvecs, &cnt,
+ write->wr_offset, &write->wr_payload, &cnt,
write->wr_how_written,
(__be32 *)write->wr_verifier.data);
nfsd_file_put(nf);
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 6dda081eb24c..b4520b6fd9ee 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -250,17 +250,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);
+ 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)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 9abdc4b75813..da62082d06dc 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1141,9 +1141,8 @@ static int wait_for_concurrent_writes(struct file *file)
__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)
+ loff_t offset, 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;
@@ -1158,9 +1157,14 @@ 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);
+ nvecs = svc_fill_write_vector(rqstp, payload);
+ if (WARN_ON_ONCE(nvecs > ARRAY_SIZE(rqstp->rq_vec)))
+ return -EIO;
+
if (sb->s_export_op)
exp_op_flags = sb->s_export_op->flags;
@@ -1185,7 +1189,7 @@ 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);
+ 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);
@@ -1290,7 +1294,7 @@ __be32 nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
*/
__be32
nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset,
- struct kvec *vec, int vlen, unsigned long *cnt, int stable,
+ struct xdr_buf *payload, unsigned long *cnt, int stable,
__be32 *verf)
{
struct nfsd_file *nf;
@@ -1302,8 +1306,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);
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index f9b09b842856..97c509e0d170 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -129,11 +129,11 @@ __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 *,
+ 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,
+ struct xdr_buf *payload, unsigned long *cnt,
int stable, __be32 *verf);
__be32 nfsd_readlink(struct svc_rqst *, struct svc_fh *,
char *, int *);
--
2.47.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] nfsd: use rq_bvec instead of rq_vec in nfsd_vfs_write
2025-05-07 11:55 remove rq_vec Christoph Hellwig
2025-05-07 11:55 ` [PATCH 1/3] nfsd: call svc_fill_write_vector from nfsd_vfs_write Christoph Hellwig
@ 2025-05-07 11:55 ` Christoph Hellwig
2025-05-08 15:42 ` kernel test robot
2025-05-07 11:55 ` [PATCH 3/3] nfsd: use rq_bvec in nfsd_iter_read Christoph Hellwig
2025-05-07 13:58 ` remove rq_vec Chuck Lever
3 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2025-05-07 11:55 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker
Cc: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs
There is no benefit in using a kvec vs a bvec for in-kernel writes,
but phasing out the kvec here will allow us to eventually remove that
array.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/nfsd/vfs.c | 10 +++++++---
include/linux/sunrpc/svc.h | 2 --
net/sunrpc/svc.c | 40 --------------------------------------
3 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index da62082d06dc..0cc9639fba79 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1161,8 +1161,12 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
trace_nfsd_write_opened(rqstp, fhp, offset, *cnt);
- nvecs = svc_fill_write_vector(rqstp, payload);
- if (WARN_ON_ONCE(nvecs > ARRAY_SIZE(rqstp->rq_vec)))
+ if (WARN_ON_ONCE(payload->tail->iov_len))
+ return -EIO;
+
+ nvecs = xdr_buf_to_bvec(rqstp->rq_bvec, ARRAY_SIZE(rqstp->rq_bvec),
+ payload);
+ if (WARN_ON_ONCE(nvecs > ARRAY_SIZE(rqstp->rq_bvec)))
return -EIO;
if (sb->s_export_op)
@@ -1189,7 +1193,7 @@ 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, rqstp->rq_vec, nvecs, *cnt);
+ iov_iter_bvec(&iter, ITER_SOURCE, rqstp->rq_bvec, nvecs, *cnt);
since = READ_ONCE(file->f_wb_err);
if (verf)
nfsd_copy_write_verifier(verf, nn);
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 74658cca0f38..c25456738315 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -452,8 +452,6 @@ const char * svc_proc_name(const struct svc_rqst *rqstp);
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);
char *svc_fill_symlink_pathname(struct svc_rqst *rqstp,
struct kvec *first, void *p,
size_t total);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index e7f9c295d13c..9fd370d6676c 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1713,46 +1713,6 @@ int svc_encode_result_payload(struct svc_rqst *rqstp, unsigned int offset,
}
EXPORT_SYMBOL_GPL(svc_encode_result_payload);
-/**
- * svc_fill_write_vector - Construct data argument for VFS write call
- * @rqstp: svc_rqst to operate on
- * @payload: xdr_buf containing only the write data 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)
-{
- struct page **pages = payload->pages;
- struct kvec *first = payload->head;
- struct kvec *vec = rqstp->rq_vec;
- size_t total = payload->len;
- unsigned int i;
-
- /* Some types of transport can present the write payload
- * entirely in rq_arg.pages. In this case, @first is empty.
- */
- i = 0;
- if (first->iov_len) {
- vec[i].iov_base = first->iov_base;
- vec[i].iov_len = min_t(size_t, total, first->iov_len);
- total -= vec[i].iov_len;
- ++i;
- }
-
- while (total) {
- vec[i].iov_base = page_address(*pages);
- vec[i].iov_len = min_t(size_t, total, PAGE_SIZE);
- total -= vec[i].iov_len;
- ++i;
- ++pages;
- }
-
- WARN_ON_ONCE(i > ARRAY_SIZE(rqstp->rq_vec));
- return i;
-}
-EXPORT_SYMBOL_GPL(svc_fill_write_vector);
-
/**
* svc_fill_symlink_pathname - Construct pathname argument for VFS symlink call
* @rqstp: svc_rqst to operate on
--
2.47.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] nfsd: use rq_bvec in nfsd_iter_read
2025-05-07 11:55 remove rq_vec Christoph Hellwig
2025-05-07 11:55 ` [PATCH 1/3] nfsd: call svc_fill_write_vector from nfsd_vfs_write Christoph Hellwig
2025-05-07 11:55 ` [PATCH 2/3] nfsd: use rq_bvec instead of rq_vec in nfsd_vfs_write Christoph Hellwig
@ 2025-05-07 11:55 ` Christoph Hellwig
2025-05-07 13:58 ` remove rq_vec Chuck Lever
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2025-05-07 11:55 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker
Cc: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs
Using a bvec array vs a kvec one slightly simplifies the logic, and allow
to remove the rq_vecs array.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/nfsd/vfs.c | 27 ++++++++++++++++-----------
include/linux/sunrpc/svc.h | 1 -
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 0cc9639fba79..1804aed0739b 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1078,26 +1078,31 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
struct file *file, loff_t offset, unsigned long *count,
unsigned int base, u32 *eof)
{
- unsigned long v, total;
+ unsigned long total = *count;
struct iov_iter iter;
loff_t ppos = offset;
- struct page *page;
+ unsigned int v = 0;
ssize_t host_err;
- v = 0;
- total = *count;
+ /*
+ * Note: if this request is served through the socket transport, rq_bvec
+ * will be rebuilt again in the transport, including the XDR header and
+ * trailer. It would be nice to fully build it in one place, but that
+ * needs more work.
+ */
while (total) {
- page = *(rqstp->rq_next_page++);
- rqstp->rq_vec[v].iov_base = page_address(page) + base;
- rqstp->rq_vec[v].iov_len = min_t(size_t, total, PAGE_SIZE - base);
- total -= rqstp->rq_vec[v].iov_len;
- ++v;
+ unsigned int len = min(total, PAGE_SIZE - base);
+ struct page *page = *(rqstp->rq_next_page++);
+
+ bvec_set_page(&rqstp->rq_bvec[v++], page, len, base);
+ if (WARN_ON_ONCE(v > ARRAY_SIZE(rqstp->rq_bvec)))
+ return -EIO;
+ total -= len;
base = 0;
}
- WARN_ON_ONCE(v > ARRAY_SIZE(rqstp->rq_vec));
trace_nfsd_read_vector(rqstp, fhp, offset, *count);
- iov_iter_kvec(&iter, ITER_DEST, rqstp->rq_vec, v, *count);
+ iov_iter_bvec(&iter, ITER_DEST, rqstp->rq_bvec, v, *count);
host_err = vfs_iter_read(file, &iter, &ppos, 0);
return nfsd_finish_read(rqstp, fhp, file, offset, count, eof, host_err);
}
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index c25456738315..a040583a29da 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -194,7 +194,6 @@ struct svc_rqst {
struct page * *rq_page_end; /* one past the last page */
struct folio_batch rq_fbatch;
- struct kvec rq_vec[RPCSVC_MAXPAGES]; /* generally useful.. */
struct bio_vec rq_bvec[RPCSVC_MAXPAGES];
__be32 rq_xid; /* transmission id */
--
2.47.2
^ permalink raw reply related [flat|nested] 7+ messages in thread