Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <qk6hjuam54khlaikf2ssom6custxf5is2ekkaequf4hvode3ls@zgf7j5j4ubvw>
 <20230626-vorverlegen-setzen-c7f96e10df34@brauner> <4sdy3yn462gdvubecjp4u7wj7hl5aah4kgsxslxlyqfnv67i72@euauz57cr3ex>
 <20230626-fazit-campen-d54e428aa4d6@brauner> <qyohloajo5pvnql3iadez4fzgiuztmx7hgokizp546lrqw3axt@ui5s6kfizj3j>
 <CAHk-=wgmLd78uSLU9A9NspXyTM9s6C23OVDiN2YjA-d8_S0zRg@mail.gmail.com>
 <20230707-konsens-ruckartig-211a4fb24e27@brauner> <CAHk-=whHXogGiPkGFwQQBtn364M4caVNcBTs7hLNfa_X67ouzA@mail.gmail.com>
 <zu7gnignulf7qqnoblpzjbu6cx5xtk2qum2uqr7q52ahxjbtdx@4ergovgpfuxt>
In-Reply-To: <zu7gnignulf7qqnoblpzjbu6cx5xtk2qum2uqr7q52ahxjbtdx@4ergovgpfuxt>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri, 7 Jul 2023 15:57:40 -0700
Message-ID: <CAHk-=wjEC_Rh8+-rtEi8C45upO-Ffw=M_i1211qS_3AvWZCbOg@mail.gmail.com>
Subject: Re: Pending splice(file -> FIFO) excludes all other FIFO operations
 forever (was: ... always blocks read(FIFO), regardless of O_NONBLOCK on read side?)
To:     =?UTF-8?Q?Ahelenia_Ziemia=C5=84ska?= 
        <nabijaczleweli@nabijaczleweli.xyz>
Cc:     Christian Brauner <brauner@kernel.org>,
        Jens Axboe <axboe@kernel.dk>,
        David Howells <dhowells@redhat.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: multipart/mixed; boundary="00000000000053b4c805ffed902d"
Precedence: bulk

--00000000000053b4c805ffed902d
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, 7 Jul 2023 at 15:41, Ahelenia Ziemia=C5=84ska
<nabijaczleweli@nabijaczleweli.xyz> wrote:
>
> (inlined by) eat_empty_buffer at fs/splice.c:594

Ahh, eat_empty_buffer() ends up releasing the buffer without waiting for it=
.

And the reason for that is actually somewhat interesting: we do have that

        while (!pipe_readable(pipe)) {
             ..

above it, but the logic for this all is that pipes with pipe buffers
are by *default* considered readable until they try to actually
confirm the buffer, and at that point they might say "oh, I have to
return -EAGAIN and set 'not_ready'".

And that splice_from_pipe_next() doesn't do that.

End result: it will happily free that pipe buffer that is still in the
process of being filled.

The good news is that I think the fix is probably trivial. Something
like the attached?

Again - NOT TESTED.

> Besides that, this doesn't solve the original issue, inasmuch as
>   ./v > fifo &
>   head fifo &
>   echo zupa > fifo
> (where ./v splices from an empty pty to stdout; v.c attached)
> echo still sleeps until ./v dies, though it also succumbs to ^C now.

Yeah, I concentrated on just making everything interruptible,

But the fact that the echo has to wait for the previous write to
finish is kind of fundamental. We can't just magically do writes out
of order. 'v' is busy writing to the fifo, we can't let some other
write just come in.

(We *could* make the splice in ./v not fill the whole pipe buffer, and
allow some other writes to fill in buffers afterwards, but at _some_
point you'll hit the "pipe buffers are full and busy, can't add any
more without waiting for them to empty").

One thing we could possibly do is to say that we just don't accept any
new writes if there are old busy splices in process. So we could make
new writes return -EBUSY or something, I guess.

             Linus

--00000000000053b4c805ffed902d
Content-Type: text/x-patch; charset="US-ASCII"; name="patch.diff"
Content-Disposition: attachment; filename="patch.diff"
Content-Transfer-Encoding: base64
Content-ID: <f_ljt6c8rj0>
X-Attachment-Id: f_ljt6c8rj0

IGZzL3NwbGljZS5jIHwgNyArKysrKysrCiAxIGZpbGUgY2hhbmdlZCwgNyBpbnNlcnRpb25zKCsp
CgpkaWZmIC0tZ2l0IGEvZnMvc3BsaWNlLmMgYi9mcy9zcGxpY2UuYwppbmRleCA0OTEzOTQxMzQ1
N2QuLmRmNmQzNGRiZjExNiAxMDA2NDQKLS0tIGEvZnMvc3BsaWNlLmMKKysrIGIvZnMvc3BsaWNl
LmMKQEAgLTU5MCw2ICs1OTAsMTMgQEAgc3RhdGljIGlubGluZSBib29sIGVhdF9lbXB0eV9idWZm
ZXIoc3RydWN0IHBpcGVfaW5vZGVfaW5mbyAqcGlwZSkKIAl1bnNpZ25lZCBpbnQgbWFzayA9IHBp
cGUtPnJpbmdfc2l6ZSAtIDE7CiAJc3RydWN0IHBpcGVfYnVmZmVyICpidWYgPSAmcGlwZS0+YnVm
c1t0YWlsICYgbWFza107CiAKKwkvKgorCSAqIERvIGEgbm9uLWJsb2NraW5nIGJ1ZmZlciBjb25m
aXJtLiBXZSBtYXkgbmVlZAorCSAqIHRvIGdvIGJhY2sgdG8gd2FpdGluZyBmb3IgdGhlIHBpcGUg
dG8gYmUgcmVhZGFibGUuCisJICovCisJaWYgKHBpcGVfYnVmX2NvbmZpcm0ocGlwZSwgYnVmLCB0
cnVlKSA9PSAtRUFHQUlOKQorCQlyZXR1cm4gdHJ1ZTsKKwogCWlmICh1bmxpa2VseSghYnVmLT5s
ZW4pKSB7CiAJCXBpcGVfYnVmX3JlbGVhc2UocGlwZSwgYnVmKTsKIAkJcGlwZS0+dGFpbCA9IHRh
aWwrMTsK
--00000000000053b4c805ffed902d--