All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Howells <dhowells@redhat.com>,
	netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	David Ahern <dsahern@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Jens Axboe <axboe@kernel.dk>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Chuck Lever <chuck.lever@oracle.com>,
	Boris Pismenny <borisp@nvidia.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: Bug in short splice to socket?
Date: Fri, 2 Jun 2023 10:38:09 -0700	[thread overview]
Message-ID: <20230602103809.1510cbef@kernel.org> (raw)
In-Reply-To: <CAHk-=wh=V579PDYvkpnTobCLGczbgxpMgGmmhqiTyE34Cpi5Gg@mail.gmail.com>

On Fri, 2 Jun 2023 13:05:14 -0400 Linus Torvalds wrote:
> Just to harp some more on this - if SPLICE_F_MORE is seen as purely a
> performance hit, with no real semantic value, and will still set
> random packet boundaries but we want big packets for all the _usual_
> cases, then I think something like "splice_end()" can be a fine
> solution regardless of exact semantics.
> 
> Alternatively, if we make it the rule that "splice_end()" is only
> called on EOF situations - so signals etc do not matter - then the
> semantics would be stable and sound fine to me too.
> 
> In that second case, I'd like to literally name it that way, and
> actually call it "splice_eof()". Because I'd like to really make it
> very clear what the semantics would be.
> 
> So a "splice_eof()" sounds fine to me, and we'd make the semantics be
> the current behavior:
> 
>  - splice() sets SPLICE_F_MORE if 'len > read_len'
> 
>  - splice() _clears_ SPLICE_F_MORE if we have hit 'len'
> 
>  - splice always sets SPLICE_F_MORE if it was passed by the user
> 
> BUT with the small new 'splice_eof()' rule that:
> 
>  - if the user did *not* set SPLICE_F_MORE *and* we didn't hit that
> "use all of len" case that cleared SPLICE_F_MORE, *and* we did a
> "->splice_in()" that returned EOF (ie zero), *then* we will also do
> that ->splice_eof() call.
> 
> The above sounds like "stable and possibly useful semantics" to me. It
> would just have to be documented.
> 
> Is that what people want?

->splice_eof() with the proposed semantics sounds perfect for the cases
testers complained about it the past, IMHO. We can pencil that in as the
contingency plan. Actually I like these semantics so much I'm tempted to
ask David to implement it already and save users potential debugging :D

> I don't think it's conceptually any different from my suggestion of
> saying "->splice_read() can set SPLICE_F_MORE if it has more to give",
> just a different implementation that doesn't require changes on the
> splice_read() side.

Setting SPLICE_F_MORE from the input side does feel much cleaner than
guessing in splice.c. But we may end up needing the eof() callback for 
the corner cases, anyway :(

  reply	other threads:[~2023-06-02 17:38 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-24 15:32 [PATCH net-next 00/12] splice, net: Replace sendpage with sendmsg(MSG_SPLICE_PAGES), part 3 David Howells
2023-05-24 15:33 ` [PATCH net-next 01/12] mm: Move the page fragment allocator from page_alloc.c into its own file David Howells
2023-05-24 15:33 ` [PATCH net-next 02/12] mm: Provide a page_frag_cache allocator cleanup function David Howells
2023-05-24 15:33   ` David Howells
2023-05-24 15:33 ` [PATCH net-next 03/12] mm: Make the page_frag_cache allocator alignment param a pow-of-2 David Howells
2023-05-24 15:33   ` David Howells
2023-05-27 15:54   ` Alexander H Duyck
2023-05-27 15:54     ` Alexander H Duyck
2023-06-16 15:28     ` David Howells
2023-06-16 15:28       ` David Howells
2023-06-16 16:06       ` Alexander Duyck
2023-06-16 16:06         ` Alexander Duyck
2023-11-30  9:00     ` Yunsheng Lin
2023-11-30  9:00       ` Yunsheng Lin
2023-05-24 15:33 ` [PATCH net-next 04/12] mm: Make the page_frag_cache allocator use multipage folios David Howells
2023-05-24 15:33   ` David Howells
2023-05-26 11:56   ` Yunsheng Lin
2023-05-26 11:56     ` Yunsheng Lin
2023-05-26 12:47     ` David Howells
2023-05-26 12:47       ` David Howells
2023-05-26 14:06       ` Mika Penttilä
2023-05-26 14:06         ` Mika Penttilä
2023-05-27 15:47     ` Alexander H Duyck
2023-05-27 15:47       ` Alexander H Duyck
2023-06-06  8:25       ` David Howells
2023-06-06  8:25         ` David Howells
2023-06-06 14:59         ` Alexander Duyck
2023-06-06 14:59           ` Alexander Duyck
2023-05-27  0:50   ` Jakub Kicinski
2023-05-27  0:50     ` Jakub Kicinski
2023-05-24 15:33 ` [PATCH net-next 05/12] mm: Make the page_frag_cache allocator handle __GFP_ZERO itself David Howells
2023-05-24 15:33   ` David Howells
2023-05-27  0:57   ` Jakub Kicinski
2023-05-27  0:57     ` Jakub Kicinski
2023-05-27 15:54     ` Alexander Duyck
2023-05-27 15:54       ` Alexander Duyck
2023-05-24 15:33 ` [PATCH net-next 06/12] mm: Make the page_frag_cache allocator use per-cpu David Howells
2023-05-24 15:33   ` David Howells
2023-05-27  1:02   ` Jakub Kicinski
2023-05-27  1:02     ` Jakub Kicinski
2023-05-24 15:33 ` [PATCH net-next 07/12] net: Clean up users of netdev_alloc_cache and napi_frag_cache David Howells
2023-05-24 15:33 ` [PATCH net-next 08/12] net: Copy slab data for sendmsg(MSG_SPLICE_PAGES) David Howells
2023-05-24 15:33 ` [PATCH net-next 09/12] tls/sw: Support MSG_SPLICE_PAGES David Howells
2023-05-27  1:08   ` Jakub Kicinski
2023-05-30 22:26     ` Bug in short splice to socket? David Howells
2023-05-31  0:32       ` Jakub Kicinski
2023-06-01 11:01       ` David Laight
2023-06-01 13:09       ` Linus Torvalds
2023-06-01 13:19         ` Linus Torvalds
2023-06-01 14:34           ` David Howells
2023-06-01 15:12             ` Linus Torvalds
2023-06-01 17:14               ` David Howells
2023-06-02  4:20                 ` Jakub Kicinski
2023-06-02  8:23                   ` David Howells
2023-06-02 11:28                     ` Linus Torvalds
2023-06-02 11:44                       ` David Howells
2023-06-02 12:11                         ` Linus Torvalds
2023-06-02 16:39                           ` Jakub Kicinski
2023-06-02 16:53                             ` Linus Torvalds
2023-06-02 17:05                               ` Linus Torvalds
2023-06-02 17:38                                 ` Jakub Kicinski [this message]
2023-06-02 20:38                                 ` David Howells
2023-06-02 20:50                                 ` David Howells
2023-06-05 11:03               ` David Laight
2023-06-05 15:52                 ` David Howells
2023-05-24 15:33 ` [PATCH net-next 10/12] tls/sw: Convert tls_sw_sendpage() to use MSG_SPLICE_PAGES David Howells
2023-05-27  1:13   ` Jakub Kicinski
2023-05-24 15:33 ` [PATCH net-next 11/12] tls/device: Support MSG_SPLICE_PAGES David Howells
2023-05-24 15:33 ` [PATCH net-next 12/12] tls/device: Convert tls_device_sendpage() to use MSG_SPLICE_PAGES David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230602103809.1510cbef@kernel.org \
    --to=kuba@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=borisp@nvidia.com \
    --cc=chuck.lever@oracle.com \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=hch@infradead.org \
    --cc=john.fastabend@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.