Re: [PATCH 0/3] vmsplice: make vmsplice a trivial wrapper for preadv2/pwritev2
Linus Torvalds <[email protected]>
| Newsgroups | org.kernel.vger.linux-api,dev.linux.lists.patches,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kvack.linux-mm |
|---|---|
| Message-ID | <CAHk-=wgV-j-G3d+899Zm1pQ=NaJrddPz=GKcL5Yw5DTUM=GaUw@mail.gmail.com> |
On Wed, 3 Jun 2026 at 06:40, Christian Brauner <[email protected]> wrote: > > Prior to the change add_to_pipe() returns -EAGAIN the moment the pipe is > full. So iter_to_pipe stops and returns a partial count capped at pipe > capacity. For a 128K buffer over a 64K pipe the first call returns 64K, > the test drains it, call 2 returns the remaining 64K. Done. > > After this change do_writev(... flags & SPLICE_F_NONBLOCK ? RWF_NOWAIT : > 0) then calls pipe_write which does not stop when the pipe fills. It > blocks until the entire iovec is consumed. > > I kinda think we need to preserve similar semantics. Ack. We definitely do need to keep the old semantics. Looking at the patch again, I think it's that (flags & SPLICE_F_NONBLOCK) ? RWF_NOWAIT : 0 thing that is broken. I think splice_to_pipe is *always* nowait - but has the special conditional _initial_ wait. So I think the RWF_NOWAIT should be unconditional to the do_writev(), and instead the code should do something like ret = wait_for_space(pipe, flags); if (!ret) do_writev(...RWF_NOWAIT); but admittedly I did not think very much about the details, so I might miss something. Which also then probably measn that we should just keep the legacy wrapper in fs/splice.c and we'd just need to make do_writev() and do_readv() non-static. Because I'd rather keep wait_for_space() internal to splice (or alternatively we'd move it to pipe.c, rename it to "pipe_wait_for_space()", and change the 'flags' argument to be a boolean to not make it use that splice-specific flags etc). Linus