Re: [PATCHSET 0/6] Enable NO_OFFLOAD support

[Jens Axboe <axboe@kernel.dk>]

On 4/19/23 6:43?PM, Pavel Begunkov wrote:
> On 4/19/23 17:25, Jens Axboe wrote:
>> Hi,
>>
>> This series enables support for forcing no-offload for requests that
>> otherwise would have been punted to io-wq. In essence, it bypasses
>> the normal non-blocking issue in favor of just letting the issue block.
>> This is only done for requests that would've otherwise hit io-wq in
>> the offload path, anything pollable will still be doing non-blocking
>> issue. See patch 3 for details.
> 
> That's shooting ourselves in the leg.
> 
> 1) It has never been easier to lock up userspace. They might be able
> to deal with simple cases like read(pipe) + write(pipe), though even
> that in a complex enough framework would cause debugging and associated
> headache.
> 
> Now let's assume that the userspace submits nvme passthrough requests,
> it exhausts tags and a request is left waiting there. To progress
> forward one of the previous reqs should complete, but it's only putting
> task in tw, which will never be run with DEFER_TASKRUN.
> 
> It's not enough for the userspace to be careful, for DEFER_TASKRUN
> there will always be a chance to get locked .
> 
> 2) It's not limited only to requests we're submitting, but also
> already queued async requests. Inline submission holds uring_lock,
> and so if io-wq thread needs to grab a registered file for the
> request, it'll io_ring_submit_lock() and wait until the submission
> ends. Same for provided buffers and some other cases.
> 
> Even task exit will actively try to grab the lock.

One thing I pondered was making the inline submissions similar to io-wq
submissions - eg don't hold uring_lock over them. To make useful, I
suspect we'd want to prep all SQ entries upfront, and then drop for
submission.

We'd also want to make this mutually exclusive with IOPOLL, obviously.
Doesn't make any sense to do anyway for IOPOLL, but it needs to be
explicitly disallowed.

-- 
Jens Axboe