diff mbox series

[net-next,3/4] kcm: Support MSG_SPLICE_PAGES

Message ID 20230524144923.3623536-4-dhowells@redhat.com (mailing list archive)
State New
Headers show
Series splice, net: Replace sendpage with sendmsg(MSG_SPLICE_PAGES), part 2 | expand

Commit Message

David Howells May 24, 2023, 2:49 p.m. UTC 
Make AF_KCM sendmsg() support MSG_SPLICE_PAGES.  This causes pages to be
spliced from the source iterator if possible.

This allows ->sendpage() to be replaced by something that can handle
multiple multipage folios in a single transaction.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Tom Herbert <tom@herbertland.com>
cc: Tom Herbert <tom@quantonium.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
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 | 55 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 17 deletions(-)

Comments

Jakub Kicinski May 27, 2023, 3:12 a.m. UTC | #1 
On Wed, 24 May 2023 15:49:22 +0100 David Howells wrote:
> +			err = skb_splice_from_iter(skb, &msg->msg_iter, copy,
> +						   sk->sk_allocation);
> +			if (err < 0) {
> +				if (err == -EMSGSIZE)
> +					goto wait_for_memory;
> +				goto out_error;
> +			}
>  

should there be a:

		copy = err;
or:
		copy -= msg_data_left(msg);

or some such here? Can we safely assume that skb_splice_from_iter() will
copy all or nothing? 

> -		err = skb_copy_to_page_nocache(sk, &msg->msg_iter, skb,
> -					       pfrag->page,
> -					       pfrag->offset,
> -					       copy);
> -		if (err)
> -			goto out_error;
> +			skb_shinfo(skb)->flags |= SKBFL_SHARED_FRAG;
David Howells May 31, 2023, 8:51 a.m. UTC | #2 
Jakub Kicinski <kuba@kernel.org> wrote:

> On Wed, 24 May 2023 15:49:22 +0100 David Howells wrote:
> > +			err = skb_splice_from_iter(skb, &msg->msg_iter, copy,
> > +						   sk->sk_allocation);
> > +			if (err < 0) {
> > +				if (err == -EMSGSIZE)
> > +					goto wait_for_memory;
> > +				goto out_error;
> > +			}
> >  
> 
> should there be a:
> 
> 		copy = err;
> or:
> 		copy -= msg_data_left(msg);
> 
> or some such here? Can we safely assume that skb_splice_from_iter() will
> copy all or nothing? 

Yeah.  Good point.  I didn't add one because the normal operation code doesn't
do that - but I guess that's all-or-nothing.

David
diff mbox series

Patch

diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index cfe828bd7fc6..411726d830c0 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -989,29 +989,50 @@  static int kcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 			merge = false;
 		}
 
-		copy = min_t(int, msg_data_left(msg),
-			     pfrag->size - pfrag->offset);
+		if (msg->msg_flags & MSG_SPLICE_PAGES) {
+			copy = msg_data_left(msg);
+			if (!sk_wmem_schedule(sk, copy))
+				goto wait_for_memory;
 
-		if (!sk_wmem_schedule(sk, copy))
-			goto wait_for_memory;
+			err = skb_splice_from_iter(skb, &msg->msg_iter, copy,
+						   sk->sk_allocation);
+			if (err < 0) {
+				if (err == -EMSGSIZE)
+					goto wait_for_memory;
+				goto out_error;
+			}
 
-		err = skb_copy_to_page_nocache(sk, &msg->msg_iter, skb,
-					       pfrag->page,
-					       pfrag->offset,
-					       copy);
-		if (err)
-			goto out_error;
+			skb_shinfo(skb)->flags |= SKBFL_SHARED_FRAG;
+			sk_wmem_queued_add(sk, copy);
+			sk_mem_charge(sk, copy);
 
-		/* Update the skb. */
-		if (merge) {
-			skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
+			if (head != skb)
+				head->truesize += copy;
 		} else {
-			skb_fill_page_desc(skb, i, pfrag->page,
-					   pfrag->offset, copy);
-			get_page(pfrag->page);
+			copy = min_t(int, msg_data_left(msg),
+				     pfrag->size - pfrag->offset);
+			if (!sk_wmem_schedule(sk, copy))
+				goto wait_for_memory;
+
+			err = skb_copy_to_page_nocache(sk, &msg->msg_iter, skb,
+						       pfrag->page,
+						       pfrag->offset,
+						       copy);
+			if (err)
+				goto out_error;
+
+			/* Update the skb. */
+			if (merge) {
+				skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
+			} else {
+				skb_fill_page_desc(skb, i, pfrag->page,
+						   pfrag->offset, copy);
+				get_page(pfrag->page);
+			}
+
+			pfrag->offset += copy;
 		}
 
-		pfrag->offset += copy;
 		copied += copy;
 		if (head != skb) {
 			head->len += copy;