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: <83712767-82b2-42f6-c86f-9e3d4edd44d5@kernel.dk>
Date:   Mon, 24 Apr 2023 16:03:21 -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
From:   Jens Axboe <axboe@kernel.dk>
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>
 <ae8ee8f3-9960-1fd9-5471-433acacb6521@kernel.dk>
In-Reply-To: <ae8ee8f3-9960-1fd9-5471-433acacb6521@kernel.dk>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@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.

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