* [PATCH net-next 1/6] Remove file->f_op->sendpage
2023-06-09 10:02 [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes David Howells
@ 2023-06-09 10:02 ` David Howells
2023-06-09 10:02 ` [PATCH net-next 2/6] algif: Remove hash_sendpage*() David Howells
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2023-06-09 10:02 UTC (permalink / raw)
To: netdev
Cc: David Howells, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox,
Jens Axboe, linux-mm, linux-kernel
Remove file->f_op->sendpage as splicing to a socket now calls sendmsg
rather than sendpage.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: netdev@vger.kernel.org
---
include/linux/fs.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index de2cb1132f07..67998c64556d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1790,7 +1790,6 @@ struct file_operations {
int (*fsync) (struct file *, loff_t, loff_t, int datasync);
int (*fasync) (int, struct file *, int);
int (*lock) (struct file *, int, struct file_lock *);
- ssize_t (*sendpage) (struct file *, struct page *, int, size_t, loff_t *, int);
unsigned long (*get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
int (*check_flags)(int);
int (*flock) (struct file *, int, struct file_lock *);
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net-next 2/6] algif: Remove hash_sendpage*()
2023-06-09 10:02 [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes David Howells
2023-06-09 10:02 ` [PATCH net-next 1/6] Remove file->f_op->sendpage David Howells
@ 2023-06-09 10:02 ` David Howells
2023-06-09 10:02 ` [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage David Howells
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2023-06-09 10:02 UTC (permalink / raw)
To: netdev
Cc: David Howells, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox,
Jens Axboe, linux-mm, linux-kernel, Herbert Xu, linux-crypto
Remove hash_sendpage*() as nothing should now call it since the rewrite of
splice_to_socket()[1].
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Herbert Xu <herbert@gondor.apana.org.au>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: linux-crypto@vger.kernel.org
cc: netdev@vger.kernel.org
Link: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=2dc334f1a63a8839b88483a3e73c0f27c9c1791c [1]
---
crypto/algif_hash.c | 66 ---------------------------------------------
1 file changed, 66 deletions(-)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 1a2d80c6c91a..dfb048cefb60 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -161,58 +161,6 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
goto unlock;
}
-static ssize_t hash_sendpage(struct socket *sock, struct page *page,
- int offset, size_t size, int flags)
-{
- struct sock *sk = sock->sk;
- struct alg_sock *ask = alg_sk(sk);
- struct hash_ctx *ctx = ask->private;
- int err;
-
- if (flags & MSG_SENDPAGE_NOTLAST)
- flags |= MSG_MORE;
-
- lock_sock(sk);
- sg_init_table(ctx->sgl.sgl, 1);
- sg_set_page(ctx->sgl.sgl, page, size, offset);
-
- if (!(flags & MSG_MORE)) {
- err = hash_alloc_result(sk, ctx);
- if (err)
- goto unlock;
- } else if (!ctx->more)
- hash_free_result(sk, ctx);
-
- ahash_request_set_crypt(&ctx->req, ctx->sgl.sgl, ctx->result, size);
-
- if (!(flags & MSG_MORE)) {
- if (ctx->more)
- err = crypto_ahash_finup(&ctx->req);
- else
- err = crypto_ahash_digest(&ctx->req);
- } else {
- if (!ctx->more) {
- err = crypto_ahash_init(&ctx->req);
- err = crypto_wait_req(err, &ctx->wait);
- if (err)
- goto unlock;
- }
-
- err = crypto_ahash_update(&ctx->req);
- }
-
- err = crypto_wait_req(err, &ctx->wait);
- if (err)
- goto unlock;
-
- ctx->more = flags & MSG_MORE;
-
-unlock:
- release_sock(sk);
-
- return err ?: size;
-}
-
static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
int flags)
{
@@ -328,7 +276,6 @@ static struct proto_ops algif_hash_ops = {
.release = af_alg_release,
.sendmsg = hash_sendmsg,
- .sendpage = hash_sendpage,
.recvmsg = hash_recvmsg,
.accept = hash_accept,
};
@@ -380,18 +327,6 @@ static int hash_sendmsg_nokey(struct socket *sock, struct msghdr *msg,
return hash_sendmsg(sock, msg, size);
}
-static ssize_t hash_sendpage_nokey(struct socket *sock, struct page *page,
- int offset, size_t size, int flags)
-{
- int err;
-
- err = hash_check_key(sock);
- if (err)
- return err;
-
- return hash_sendpage(sock, page, offset, size, flags);
-}
-
static int hash_recvmsg_nokey(struct socket *sock, struct msghdr *msg,
size_t ignored, int flags)
{
@@ -430,7 +365,6 @@ static struct proto_ops algif_hash_ops_nokey = {
.release = af_alg_release,
.sendmsg = hash_sendmsg_nokey,
- .sendpage = hash_sendpage_nokey,
.recvmsg = hash_recvmsg_nokey,
.accept = hash_accept_nokey,
};
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
2023-06-09 10:02 [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes David Howells
2023-06-09 10:02 ` [PATCH net-next 1/6] Remove file->f_op->sendpage David Howells
2023-06-09 10:02 ` [PATCH net-next 2/6] algif: Remove hash_sendpage*() David Howells
@ 2023-06-09 10:02 ` David Howells
2023-08-11 22:50 ` Jeff Layton
2023-06-09 10:02 ` [PATCH net-next 4/6] tcp_bpf: Make tcp_bpf_sendpage() go through tcp_bpf_sendmsg(MSG_SPLICE_PAGES) David Howells
` (3 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: David Howells @ 2023-06-09 10:02 UTC (permalink / raw)
To: netdev
Cc: David Howells, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox,
Jens Axboe, linux-mm, linux-kernel, Chuck Lever, Trond Myklebust,
Anna Schumaker, Jeff Layton, linux-nfs
When transmitting data, call down into TCP using sendmsg with
MSG_SPLICE_PAGES to indicate that content should be spliced rather than
performing sendpage calls to transmit header, data pages and trailer.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Chuck Lever <chuck.lever@oracle.com>
cc: Trond Myklebust <trond.myklebust@hammerspace.com>
cc: Anna Schumaker <anna@kernel.org>
cc: Jeff Layton <jlayton@kernel.org>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: linux-nfs@vger.kernel.org
cc: netdev@vger.kernel.org
---
include/linux/sunrpc/svc.h | 11 +++++------
net/sunrpc/svcsock.c | 38 ++++++++++++--------------------------
2 files changed, 17 insertions(+), 32 deletions(-)
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 762d7231e574..f66ec8fdb331 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -161,16 +161,15 @@ static inline bool svc_put_not_last(struct svc_serv *serv)
extern u32 svc_max_payload(const struct svc_rqst *rqstp);
/*
- * RPC Requsts and replies are stored in one or more pages.
+ * RPC Requests and replies are stored in one or more pages.
* We maintain an array of pages for each server thread.
* Requests are copied into these pages as they arrive. Remaining
* pages are available to write the reply into.
*
- * Pages are sent using ->sendpage so each server thread needs to
- * allocate more to replace those used in sending. To help keep track
- * of these pages we have a receive list where all pages initialy live,
- * and a send list where pages are moved to when there are to be part
- * of a reply.
+ * Pages are sent using ->sendmsg with MSG_SPLICE_PAGES so each server thread
+ * needs to allocate more to replace those used in sending. To help keep track
+ * of these pages we have a receive list where all pages initialy live, and a
+ * send list where pages are moved to when there are to be part of a reply.
*
* We use xdr_buf for holding responses as it fits well with NFS
* read responses (that have a header, and some data pages, and possibly
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index f77cebe2c071..9d9f522e3ae1 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1203,13 +1203,14 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)
static int svc_tcp_send_kvec(struct socket *sock, const struct kvec *vec,
int flags)
{
- return kernel_sendpage(sock, virt_to_page(vec->iov_base),
- offset_in_page(vec->iov_base),
- vec->iov_len, flags);
+ struct msghdr msg = { .msg_flags = MSG_SPLICE_PAGES | flags, };
+
+ iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, vec, 1, vec->iov_len);
+ return sock_sendmsg(sock, &msg);
}
/*
- * kernel_sendpage() is used exclusively to reduce the number of
+ * MSG_SPLICE_PAGES is used exclusively to reduce the number of
* copy operations in this path. Therefore the caller must ensure
* that the pages backing @xdr are unchanging.
*
@@ -1249,28 +1250,13 @@ static int svc_tcp_sendmsg(struct socket *sock, struct xdr_buf *xdr,
if (ret != head->iov_len)
goto out;
- if (xdr->page_len) {
- unsigned int offset, len, remaining;
- struct bio_vec *bvec;
-
- bvec = xdr->bvec + (xdr->page_base >> PAGE_SHIFT);
- offset = offset_in_page(xdr->page_base);
- remaining = xdr->page_len;
- while (remaining > 0) {
- len = min(remaining, bvec->bv_len - offset);
- ret = kernel_sendpage(sock, bvec->bv_page,
- bvec->bv_offset + offset,
- len, 0);
- if (ret < 0)
- return ret;
- *sentp += ret;
- if (ret != len)
- goto out;
- remaining -= len;
- offset = 0;
- bvec++;
- }
- }
+ msg.msg_flags = MSG_SPLICE_PAGES;
+ iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, xdr->bvec,
+ xdr_buf_pagecount(xdr), xdr->page_len);
+ ret = sock_sendmsg(sock, &msg);
+ if (ret < 0)
+ return ret;
+ *sentp += ret;
if (tail->iov_len) {
ret = svc_tcp_send_kvec(sock, tail, 0);
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
2023-06-09 10:02 ` [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage David Howells
@ 2023-08-11 22:50 ` Jeff Layton
2023-08-11 23:07 ` Jeff Layton
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2023-08-11 22:50 UTC (permalink / raw)
To: David Howells, netdev
Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
Willem de Bruijn, David Ahern, Matthew Wilcox, Jens Axboe,
linux-mm, linux-kernel, Chuck Lever, Trond Myklebust,
Anna Schumaker, linux-nfs
On Fri, 2023-06-09 at 11:02 +0100, David Howells wrote:
> When transmitting data, call down into TCP using sendmsg with
> MSG_SPLICE_PAGES to indicate that content should be spliced rather than
> performing sendpage calls to transmit header, data pages and trailer.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Acked-by: Chuck Lever <chuck.lever@oracle.com>
> cc: Trond Myklebust <trond.myklebust@hammerspace.com>
> cc: Anna Schumaker <anna@kernel.org>
> cc: Jeff Layton <jlayton@kernel.org>
> cc: "David S. Miller" <davem@davemloft.net>
> cc: Eric Dumazet <edumazet@google.com>
> cc: Jakub Kicinski <kuba@kernel.org>
> cc: Paolo Abeni <pabeni@redhat.com>
> cc: Jens Axboe <axboe@kernel.dk>
> cc: Matthew Wilcox <willy@infradead.org>
> cc: linux-nfs@vger.kernel.org
> cc: netdev@vger.kernel.org
> ---
> include/linux/sunrpc/svc.h | 11 +++++------
> net/sunrpc/svcsock.c | 38 ++++++++++++--------------------------
> 2 files changed, 17 insertions(+), 32 deletions(-)
>
I'm seeing a regression in pynfs runs with v6.5-rc5. 3 tests are failing
in a similar fashion. WRT1b is one of them
[vagrant@jlayton-kdo-nfsd nfs4.0]$ ./testserver.py --rundeps --maketree --uid=0 --gid=0 localhost:/export/pynfs/4.0/ WRT1b
**************************************************
WRT1b st_write.testSimpleWrite2 : FAILURE
READ returned
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
expected b'\x00\x00\x00\x00\x00write data'
INIT st_setclientid.testValid : PASS
MKFILE st_open.testOpen : PASS
**************************************************
Command line asked for 3 of 679 tests
Of those: 0 Skipped, 1 Failed, 0 Warned, 2 Passed
This test just writes "write data" starting at offset 30 and then reads
the data back. It looks like we're seeing zeroes in the read reply where
the data should be.
A bisect landed on this patch, which I'm assuming is the same as this
commit in mainline:
5df5dd03a8f7 sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
...any thoughts as to what might be wrong?
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 762d7231e574..f66ec8fdb331 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -161,16 +161,15 @@ static inline bool svc_put_not_last(struct svc_serv *serv)
> extern u32 svc_max_payload(const struct svc_rqst *rqstp);
>
> /*
> - * RPC Requsts and replies are stored in one or more pages.
> + * RPC Requests and replies are stored in one or more pages.
> * We maintain an array of pages for each server thread.
> * Requests are copied into these pages as they arrive. Remaining
> * pages are available to write the reply into.
> *
> - * Pages are sent using ->sendpage so each server thread needs to
> - * allocate more to replace those used in sending. To help keep track
> - * of these pages we have a receive list where all pages initialy live,
> - * and a send list where pages are moved to when there are to be part
> - * of a reply.
> + * Pages are sent using ->sendmsg with MSG_SPLICE_PAGES so each server thread
> + * needs to allocate more to replace those used in sending. To help keep track
> + * of these pages we have a receive list where all pages initialy live, and a
> + * send list where pages are moved to when there are to be part of a reply.
> *
> * We use xdr_buf for holding responses as it fits well with NFS
> * read responses (that have a header, and some data pages, and possibly
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index f77cebe2c071..9d9f522e3ae1 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1203,13 +1203,14 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)
> static int svc_tcp_send_kvec(struct socket *sock, const struct kvec *vec,
> int flags)
> {
> - return kernel_sendpage(sock, virt_to_page(vec->iov_base),
> - offset_in_page(vec->iov_base),
> - vec->iov_len, flags);
> + struct msghdr msg = { .msg_flags = MSG_SPLICE_PAGES | flags, };
> +
> + iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, vec, 1, vec->iov_len);
> + return sock_sendmsg(sock, &msg);
> }
>
> /*
> - * kernel_sendpage() is used exclusively to reduce the number of
> + * MSG_SPLICE_PAGES is used exclusively to reduce the number of
> * copy operations in this path. Therefore the caller must ensure
> * that the pages backing @xdr are unchanging.
> *
> @@ -1249,28 +1250,13 @@ static int svc_tcp_sendmsg(struct socket *sock, struct xdr_buf *xdr,
> if (ret != head->iov_len)
> goto out;
>
> - if (xdr->page_len) {
> - unsigned int offset, len, remaining;
> - struct bio_vec *bvec;
> -
> - bvec = xdr->bvec + (xdr->page_base >> PAGE_SHIFT);
> - offset = offset_in_page(xdr->page_base);
> - remaining = xdr->page_len;
> - while (remaining > 0) {
> - len = min(remaining, bvec->bv_len - offset);
> - ret = kernel_sendpage(sock, bvec->bv_page,
> - bvec->bv_offset + offset,
> - len, 0);
> - if (ret < 0)
> - return ret;
> - *sentp += ret;
> - if (ret != len)
> - goto out;
> - remaining -= len;
> - offset = 0;
> - bvec++;
> - }
> - }
> + msg.msg_flags = MSG_SPLICE_PAGES;
> + iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, xdr->bvec,
> + xdr_buf_pagecount(xdr), xdr->page_len);
> + ret = sock_sendmsg(sock, &msg);
> + if (ret < 0)
> + return ret;
> + *sentp += ret;
>
> if (tail->iov_len) {
> ret = svc_tcp_send_kvec(sock, tail, 0);
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
2023-08-11 22:50 ` Jeff Layton
@ 2023-08-11 23:07 ` Jeff Layton
2023-08-12 11:45 ` Jeff Layton
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2023-08-11 23:07 UTC (permalink / raw)
To: David Howells, netdev
Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
Willem de Bruijn, David Ahern, Matthew Wilcox, Jens Axboe,
linux-mm, linux-kernel, Chuck Lever, Trond Myklebust,
Anna Schumaker, linux-nfs
[-- Attachment #1: Type: text/plain, Size: 3364 bytes --]
On Fri, 2023-08-11 at 18:50 -0400, Jeff Layton wrote:
> On Fri, 2023-06-09 at 11:02 +0100, David Howells wrote:
> > When transmitting data, call down into TCP using sendmsg with
> > MSG_SPLICE_PAGES to indicate that content should be spliced rather than
> > performing sendpage calls to transmit header, data pages and trailer.
> >
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > Acked-by: Chuck Lever <chuck.lever@oracle.com>
> > cc: Trond Myklebust <trond.myklebust@hammerspace.com>
> > cc: Anna Schumaker <anna@kernel.org>
> > cc: Jeff Layton <jlayton@kernel.org>
> > cc: "David S. Miller" <davem@davemloft.net>
> > cc: Eric Dumazet <edumazet@google.com>
> > cc: Jakub Kicinski <kuba@kernel.org>
> > cc: Paolo Abeni <pabeni@redhat.com>
> > cc: Jens Axboe <axboe@kernel.dk>
> > cc: Matthew Wilcox <willy@infradead.org>
> > cc: linux-nfs@vger.kernel.org
> > cc: netdev@vger.kernel.org
> > ---
> > include/linux/sunrpc/svc.h | 11 +++++------
> > net/sunrpc/svcsock.c | 38 ++++++++++++--------------------------
> > 2 files changed, 17 insertions(+), 32 deletions(-)
> >
>
> I'm seeing a regression in pynfs runs with v6.5-rc5. 3 tests are failing
> in a similar fashion. WRT1b is one of them
>
> [vagrant@jlayton-kdo-nfsd nfs4.0]$ ./testserver.py --rundeps --maketree --uid=0 --gid=0 localhost:/export/pynfs/4.0/ WRT1b
> **************************************************
> WRT1b st_write.testSimpleWrite2 : FAILURE
> READ returned
> b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
> expected b'\x00\x00\x00\x00\x00write data'
> INIT st_setclientid.testValid : PASS
> MKFILE st_open.testOpen : PASS
> **************************************************
> Command line asked for 3 of 679 tests
> Of those: 0 Skipped, 1 Failed, 0 Warned, 2 Passed
FWIW, here's a capture that shows the problem. See frames 109-112 in
particular. If no one has thoughts on this one, I'll plan to have a look
early next week.
Thanks,
--
Jeff Layton <jlayton@kernel.org>
[-- Attachment #2: nfs.pcap.xz --]
[-- Type: application/x-xz, Size: 3772 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
2023-08-11 23:07 ` Jeff Layton
@ 2023-08-12 11:45 ` Jeff Layton
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Layton @ 2023-08-12 11:45 UTC (permalink / raw)
To: David Howells, netdev
Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
Willem de Bruijn, David Ahern, Matthew Wilcox, Jens Axboe,
linux-mm, linux-kernel, Chuck Lever, Trond Myklebust,
Anna Schumaker, linux-nfs
On Fri, 2023-08-11 at 19:07 -0400, Jeff Layton wrote:
> On Fri, 2023-08-11 at 18:50 -0400, Jeff Layton wrote:
> > On Fri, 2023-06-09 at 11:02 +0100, David Howells wrote:
> > > When transmitting data, call down into TCP using sendmsg with
> > > MSG_SPLICE_PAGES to indicate that content should be spliced rather than
> > > performing sendpage calls to transmit header, data pages and trailer.
> > >
> > > Signed-off-by: David Howells <dhowells@redhat.com>
> > > Acked-by: Chuck Lever <chuck.lever@oracle.com>
> > > cc: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > cc: Anna Schumaker <anna@kernel.org>
> > > cc: Jeff Layton <jlayton@kernel.org>
> > > cc: "David S. Miller" <davem@davemloft.net>
> > > cc: Eric Dumazet <edumazet@google.com>
> > > cc: Jakub Kicinski <kuba@kernel.org>
> > > cc: Paolo Abeni <pabeni@redhat.com>
> > > cc: Jens Axboe <axboe@kernel.dk>
> > > cc: Matthew Wilcox <willy@infradead.org>
> > > cc: linux-nfs@vger.kernel.org
> > > cc: netdev@vger.kernel.org
> > > ---
> > > include/linux/sunrpc/svc.h | 11 +++++------
> > > net/sunrpc/svcsock.c | 38 ++++++++++++--------------------------
> > > 2 files changed, 17 insertions(+), 32 deletions(-)
> > >
> >
> > I'm seeing a regression in pynfs runs with v6.5-rc5. 3 tests are failing
> > in a similar fashion. WRT1b is one of them
> >
> > [vagrant@jlayton-kdo-nfsd nfs4.0]$ ./testserver.py --rundeps --maketree --uid=0 --gid=0 localhost:/export/pynfs/4.0/ WRT1b
> > **************************************************
> > WRT1b st_write.testSimpleWrite2 : FAILURE
> > READ returned
> > b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
> > expected b'\x00\x00\x00\x00\x00write data'
> > INIT st_setclientid.testValid : PASS
> > MKFILE st_open.testOpen : PASS
> > **************************************************
> > Command line asked for 3 of 679 tests
> > Of those: 0 Skipped, 1 Failed, 0 Warned, 2 Passed
>
> FWIW, here's a capture that shows the problem. See frames 109-112 in
> particular. If no one has thoughts on this one, I'll plan to have a look
> early next week.
Since Chuck's nfsd-next branch (which is based on v6.5-rc5) wasn't
showing this issue, I ran a bisect to see what fixes it, and it landed
on this patch:
commit ed9cd98404c8ae5d0bdd6e7ce52e458a8e0841bb
Author: Chuck Lever <chuck.lever@oracle.com>
Date: Wed Jul 19 14:31:03 2023 -0400
SUNRPC: Convert svc_tcp_sendmsg to use bio_vecs directly
Add a helper to convert a whole xdr_buf directly into an array of
bio_vecs, then send this array instead of iterating piecemeal over
the xdr_buf containing the outbound RPC message.
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
I'll follow up on that thread. I think we may want to pull this patch
into mainline for v6.5.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next 4/6] tcp_bpf: Make tcp_bpf_sendpage() go through tcp_bpf_sendmsg(MSG_SPLICE_PAGES)
2023-06-09 10:02 [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes David Howells
` (2 preceding siblings ...)
2023-06-09 10:02 ` [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage David Howells
@ 2023-06-09 10:02 ` David Howells
2023-06-09 10:02 ` [PATCH net-next 5/6] kcm: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage David Howells
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2023-06-09 10:02 UTC (permalink / raw)
To: netdev
Cc: David Howells, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox,
Jens Axboe, linux-mm, linux-kernel, John Fastabend,
Jakub Sitnicki, bpf
Make tcp_bpf_sendpage() a wrapper around tcp_bpf_sendmsg(MSG_SPLICE_PAGES)
rather than a loop calling tcp_sendpage(). sendpage() will be removed in
the future.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: John Fastabend <john.fastabend@gmail.com>
cc: Jakub Sitnicki <jakub@cloudflare.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: bpf@vger.kernel.org
cc: netdev@vger.kernel.org
---
net/ipv4/tcp_bpf.c | 49 +++++++++-------------------------------------
1 file changed, 9 insertions(+), 40 deletions(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index e75023ea052f..5a84053ac62b 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -568,49 +568,18 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
static int tcp_bpf_sendpage(struct sock *sk, struct page *page, int offset,
size_t size, int flags)
{
- struct sk_msg tmp, *msg = NULL;
- int err = 0, copied = 0;
- struct sk_psock *psock;
- bool enospc = false;
-
- psock = sk_psock_get(sk);
- if (unlikely(!psock))
- return tcp_sendpage(sk, page, offset, size, flags);
+ struct bio_vec bvec;
+ struct msghdr msg = {
+ .msg_flags = flags | MSG_SPLICE_PAGES,
+ };
- lock_sock(sk);
- if (psock->cork) {
- msg = psock->cork;
- } else {
- msg = &tmp;
- sk_msg_init(msg);
- }
+ bvec_set_page(&bvec, page, size, offset);
+ iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, size);
- /* Catch case where ring is full and sendpage is stalled. */
- if (unlikely(sk_msg_full(msg)))
- goto out_err;
-
- sk_msg_page_add(msg, page, size, offset);
- sk_mem_charge(sk, size);
- copied = size;
- if (sk_msg_full(msg))
- enospc = true;
- if (psock->cork_bytes) {
- if (size > psock->cork_bytes)
- psock->cork_bytes = 0;
- else
- psock->cork_bytes -= size;
- if (psock->cork_bytes && !enospc)
- goto out_err;
- /* All cork bytes are accounted, rerun the prog. */
- psock->eval = __SK_NONE;
- psock->cork_bytes = 0;
- }
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ msg.msg_flags |= MSG_MORE;
- err = tcp_bpf_send_verdict(sk, psock, msg, &copied, flags);
-out_err:
- release_sock(sk);
- sk_psock_put(sk, psock);
- return copied ? copied : err;
+ return tcp_bpf_sendmsg(sk, &msg, size);
}
enum {
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net-next 5/6] kcm: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
2023-06-09 10:02 [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes David Howells
` (3 preceding siblings ...)
2023-06-09 10:02 ` [PATCH net-next 4/6] tcp_bpf: Make tcp_bpf_sendpage() go through tcp_bpf_sendmsg(MSG_SPLICE_PAGES) David Howells
@ 2023-06-09 10:02 ` David Howells
2023-06-09 10:02 ` [PATCH net-next 6/6] kcm: Send multiple frags in one sendmsg() David Howells
2023-06-13 5:10 ` [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes patchwork-bot+netdevbpf
6 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2023-06-09 10:02 UTC (permalink / raw)
To: netdev
Cc: David Howells, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox,
Jens Axboe, linux-mm, linux-kernel, Tom Herbert, Tom Herbert
When transmitting data, call down into the transport socket using sendmsg
with MSG_SPLICE_PAGES to indicate that content should be spliced rather
than using sendpage.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Tom Herbert <tom@herbertland.com>
cc: Tom Herbert <tom@quantonium.net>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: netdev@vger.kernel.org
---
net/kcm/kcmsock.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 7dee74430b59..3bcac1453f10 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -641,6 +641,10 @@ static int kcm_write_msgs(struct kcm_sock *kcm)
for (fragidx = 0; fragidx < skb_shinfo(skb)->nr_frags;
fragidx++) {
+ struct bio_vec bvec;
+ struct msghdr msg = {
+ .msg_flags = MSG_DONTWAIT | MSG_SPLICE_PAGES,
+ };
skb_frag_t *frag;
frag_offset = 0;
@@ -651,11 +655,13 @@ static int kcm_write_msgs(struct kcm_sock *kcm)
goto out;
}
- ret = kernel_sendpage(psock->sk->sk_socket,
- skb_frag_page(frag),
- skb_frag_off(frag) + frag_offset,
- skb_frag_size(frag) - frag_offset,
- MSG_DONTWAIT);
+ bvec_set_page(&bvec,
+ skb_frag_page(frag),
+ skb_frag_size(frag) - frag_offset,
+ skb_frag_off(frag) + frag_offset);
+ iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1,
+ bvec.bv_len);
+ ret = sock_sendmsg(psock->sk->sk_socket, &msg);
if (ret <= 0) {
if (ret == -EAGAIN) {
/* Save state to try again when there's
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net-next 6/6] kcm: Send multiple frags in one sendmsg()
2023-06-09 10:02 [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes David Howells
` (4 preceding siblings ...)
2023-06-09 10:02 ` [PATCH net-next 5/6] kcm: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage David Howells
@ 2023-06-09 10:02 ` David Howells
2023-06-13 5:10 ` [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes patchwork-bot+netdevbpf
6 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2023-06-09 10:02 UTC (permalink / raw)
To: netdev
Cc: David Howells, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox,
Jens Axboe, linux-mm, linux-kernel, Tom Herbert, Tom Herbert
Rewrite the AF_KCM transmission loop to send all the fragments in a single
skb or frag_list-skb in one sendmsg() with MSG_SPLICE_PAGES set. The list
of fragments in each skb is conveniently a bio_vec[] that can just be
attached to a BVEC iter.
Note: I'm working out the size of each fragment-skb by adding up bv_len for
all the bio_vecs in skb->frags[] - but surely this information is recorded
somewhere? For the skbs in head->frag_list, this is equal to
skb->data_len, but not for the head. head->data_len includes all the tail
frags too.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Tom Herbert <tom@herbertland.com>
cc: Tom Herbert <tom@quantonium.net>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: netdev@vger.kernel.org
---
include/net/kcm.h | 2 +-
net/kcm/kcmsock.c | 126 ++++++++++++++++++----------------------------
2 files changed, 51 insertions(+), 77 deletions(-)
diff --git a/include/net/kcm.h b/include/net/kcm.h
index 2d704f8f4905..90279e5e09a5 100644
--- a/include/net/kcm.h
+++ b/include/net/kcm.h
@@ -47,9 +47,9 @@ struct kcm_stats {
struct kcm_tx_msg {
unsigned int sent;
- unsigned int fragidx;
unsigned int frag_offset;
unsigned int msg_flags;
+ bool started_tx;
struct sk_buff *frag_skb;
struct sk_buff *last_skb;
};
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 3bcac1453f10..d75d775e9462 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -581,12 +581,10 @@ static void kcm_report_tx_retry(struct kcm_sock *kcm)
*/
static int kcm_write_msgs(struct kcm_sock *kcm)
{
+ unsigned int total_sent = 0;
struct sock *sk = &kcm->sk;
struct kcm_psock *psock;
- struct sk_buff *skb, *head;
- struct kcm_tx_msg *txm;
- unsigned short fragidx, frag_offset;
- unsigned int sent, total_sent = 0;
+ struct sk_buff *head;
int ret = 0;
kcm->tx_wait_more = false;
@@ -600,78 +598,57 @@ static int kcm_write_msgs(struct kcm_sock *kcm)
if (skb_queue_empty(&sk->sk_write_queue))
return 0;
- kcm_tx_msg(skb_peek(&sk->sk_write_queue))->sent = 0;
-
- } else if (skb_queue_empty(&sk->sk_write_queue)) {
- return 0;
+ kcm_tx_msg(skb_peek(&sk->sk_write_queue))->started_tx = false;
}
- head = skb_peek(&sk->sk_write_queue);
- txm = kcm_tx_msg(head);
-
- if (txm->sent) {
- /* Send of first skbuff in queue already in progress */
- if (WARN_ON(!psock)) {
- ret = -EINVAL;
- goto out;
+retry:
+ while ((head = skb_peek(&sk->sk_write_queue))) {
+ struct msghdr msg = {
+ .msg_flags = MSG_DONTWAIT | MSG_SPLICE_PAGES,
+ };
+ struct kcm_tx_msg *txm = kcm_tx_msg(head);
+ struct sk_buff *skb;
+ unsigned int msize;
+ int i;
+
+ if (!txm->started_tx) {
+ psock = reserve_psock(kcm);
+ if (!psock)
+ goto out;
+ skb = head;
+ txm->frag_offset = 0;
+ txm->sent = 0;
+ txm->started_tx = true;
+ } else {
+ if (WARN_ON(!psock)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ skb = txm->frag_skb;
}
- sent = txm->sent;
- frag_offset = txm->frag_offset;
- fragidx = txm->fragidx;
- skb = txm->frag_skb;
-
- goto do_frag;
- }
-
-try_again:
- psock = reserve_psock(kcm);
- if (!psock)
- goto out;
-
- do {
- skb = head;
- txm = kcm_tx_msg(head);
- sent = 0;
-do_frag_list:
if (WARN_ON(!skb_shinfo(skb)->nr_frags)) {
ret = -EINVAL;
goto out;
}
- for (fragidx = 0; fragidx < skb_shinfo(skb)->nr_frags;
- fragidx++) {
- struct bio_vec bvec;
- struct msghdr msg = {
- .msg_flags = MSG_DONTWAIT | MSG_SPLICE_PAGES,
- };
- skb_frag_t *frag;
-
- frag_offset = 0;
-do_frag:
- frag = &skb_shinfo(skb)->frags[fragidx];
- if (WARN_ON(!skb_frag_size(frag))) {
- ret = -EINVAL;
- goto out;
- }
+ msize = 0;
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
+ msize += skb_shinfo(skb)->frags[i].bv_len;
+
+ iov_iter_bvec(&msg.msg_iter, ITER_SOURCE,
+ skb_shinfo(skb)->frags, skb_shinfo(skb)->nr_frags,
+ msize);
+ iov_iter_advance(&msg.msg_iter, txm->frag_offset);
- bvec_set_page(&bvec,
- skb_frag_page(frag),
- skb_frag_size(frag) - frag_offset,
- skb_frag_off(frag) + frag_offset);
- iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1,
- bvec.bv_len);
+ do {
ret = sock_sendmsg(psock->sk->sk_socket, &msg);
if (ret <= 0) {
if (ret == -EAGAIN) {
/* Save state to try again when there's
* write space on the socket
*/
- txm->sent = sent;
- txm->frag_offset = frag_offset;
- txm->fragidx = fragidx;
txm->frag_skb = skb;
-
ret = 0;
goto out;
}
@@ -685,39 +662,36 @@ static int kcm_write_msgs(struct kcm_sock *kcm)
true);
unreserve_psock(kcm);
- txm->sent = 0;
+ txm->started_tx = false;
kcm_report_tx_retry(kcm);
ret = 0;
-
- goto try_again;
+ goto retry;
}
- sent += ret;
- frag_offset += ret;
+ txm->sent += ret;
+ txm->frag_offset += ret;
KCM_STATS_ADD(psock->stats.tx_bytes, ret);
- if (frag_offset < skb_frag_size(frag)) {
- /* Not finished with this frag */
- goto do_frag;
- }
- }
+ } while (msg.msg_iter.count > 0);
if (skb == head) {
if (skb_has_frag_list(skb)) {
- skb = skb_shinfo(skb)->frag_list;
- goto do_frag_list;
+ txm->frag_skb = skb_shinfo(skb)->frag_list;
+ txm->frag_offset = 0;
+ continue;
}
} else if (skb->next) {
- skb = skb->next;
- goto do_frag_list;
+ txm->frag_skb = skb->next;
+ txm->frag_offset = 0;
+ continue;
}
/* Successfully sent the whole packet, account for it. */
+ sk->sk_wmem_queued -= txm->sent;
+ total_sent += txm->sent;
skb_dequeue(&sk->sk_write_queue);
kfree_skb(head);
- sk->sk_wmem_queued -= sent;
- total_sent += sent;
KCM_STATS_INCR(psock->stats.tx_msgs);
- } while ((head = skb_peek(&sk->sk_write_queue)));
+ }
out:
if (!head) {
/* Done with all queued messages. */
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes
2023-06-09 10:02 [PATCH net-next 0/6] splice, net: Some miscellaneous MSG_SPLICE_PAGES changes David Howells
` (5 preceding siblings ...)
2023-06-09 10:02 ` [PATCH net-next 6/6] kcm: Send multiple frags in one sendmsg() David Howells
@ 2023-06-13 5:10 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-06-13 5:10 UTC (permalink / raw)
To: David Howells
Cc: netdev, kuba, davem, edumazet, pabeni, willemdebruijn.kernel,
dsahern, willy, axboe, linux-mm, linux-kernel
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 9 Jun 2023 11:02:15 +0100 you wrote:
> Now that the splice_to_socket() has been rewritten so that nothing now uses
> the ->sendpage() file op[1], some further changes can be made, so here are
> some miscellaneous changes that can now be done.
>
> (1) Remove the ->sendpage() file op.
>
> (2) Remove hash_sendpage*() from AF_ALG.
>
> [...]
Here is the summary with links:
- [net-next,1/6] Remove file->f_op->sendpage
https://git.kernel.org/netdev/net-next/c/a3bbdc52c38f
- [net-next,2/6] algif: Remove hash_sendpage*()
https://git.kernel.org/netdev/net-next/c/345ee3e8126a
- [net-next,3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
https://git.kernel.org/netdev/net-next/c/5df5dd03a8f7
- [net-next,4/6] tcp_bpf: Make tcp_bpf_sendpage() go through tcp_bpf_sendmsg(MSG_SPLICE_PAGES)
https://git.kernel.org/netdev/net-next/c/de17c6857301
- [net-next,5/6] kcm: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
https://git.kernel.org/netdev/net-next/c/264ba53fac79
- [net-next,6/6] kcm: Send multiple frags in one sendmsg()
https://git.kernel.org/netdev/net-next/c/c31a25e1db48
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 11+ messages in thread