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;
Message-ID: <ae8ee8f3-9960-1fd9-5471-433acacb6521@kernel.dk>
Date:   Mon, 24 Apr 2023 15:55:48 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [GIT PULL] pipe: nonblocking rw for io_uring
Content-Language: en-US
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Christian Brauner <brauner@kernel.org>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230421-seilbahn-vorpreschen-bd73ac3c88d7@brauner>
 <CAHk-=wgyL9OujQ72er7oXt_VsMeno4bMKCTydBT1WSaagZ_5CA@mail.gmail.com>
 <6882b74e-874a-c116-62ac-564104c5ad34@kernel.dk>
 <CAHk-=wiQ8g+B0bCPJ9fxZ+Oa0LPAUAyryw9i+-fBUe72LoA+QQ@mail.gmail.com>
From:   Jens Axboe <axboe@kernel.dk>
In-Reply-To: <CAHk-=wiQ8g+B0bCPJ9fxZ+Oa0LPAUAyryw9i+-fBUe72LoA+QQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/24/23 3:37?PM, Linus Torvalds wrote:
> On Mon, Apr 24, 2023 at 2:22?PM Jens Axboe <axboe@kernel.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.

-- 
Jens Axboe