linux-kernel - Re: [GIT PULL] pipe: nonblocking rw for io

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <83712767-82b2-42f6-c86f-9e3d4edd44d5@kernel.dk>
Date:   Mon, 24 Apr 2023 16:03:21 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Christian Brauner <brauner@...nel.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] pipe: nonblocking rw for io_uring

On 4/24/23 3:55?PM, Jens Axboe wrote:
> On 4/24/23 3:37?PM, Linus Torvalds wrote:
>> On Mon, Apr 24, 2023 at 2:22?PM Jens Axboe <axboe@...nel.dk> wrote:
>>>
>>> If we don't ever wait for IO with the pipe lock held, then we can skip
>>> the conditional locking. But with splice, that's not at all the case! We
>>> most certainly wait for IO there with the pipe lock held.
>>
>> I think that then needs to just be fixed.
> 
> I took another look at this, and the main issue is in fact splice
> confirming buffers. So I do think that we can make this work by simply
> having the non-block nature of it being passed down the ->confirm()
> callback as that's the one that'll be waiting for IO. If we have that,
> then we can disregard the pipe locking as we won't be holding it over
> IO.
> 
> Which is what part of this series does, notably patch 1.
> 
> Only other oddity is pipe_to_sendpage(), which we can probably sanely
> ignore.
> 
> IOW, would you be fine with a v2 of this pull request where patch 2
> drops the conditional locking and just passes it to ->confirm()? That's
> certainly sane, and just makes the ultimate page locking conditional to
> avoid waiting on IO. I'd really hate to still be missing out on pipe
> performance with io_uring.

I guess that would still have blocking if you have someone doing splice
in a blocking fashion, and someone else trying to do RWF_NOWAIT reads or
writes to the pipe... The very thing the conditional pipe locking would
sort out.

Only fool proof alternative would seem to be having splice use a
specific pipe lock rather then pipe->mutex. And honestly pipes and
splice are so tied together than I'm not sure that doing separate
locking would be feasible.

-- 
Jens Axboe