Re: [PATCHSET v2 0/2] io_uring: handle short reads internally

Jens Axboe
2020-08-18T14:44:36Z

On 8/18/20 12:40 AM, Stefan Metzmacher wrote:
> Hi Jens,
>
>>>>> Will this be backported?
>>>>
>>>> I can, but not really in an efficient manner. It depends on the async
>>>> buffered work to make progress, and the task_work handling retry. The
>>>> latter means it's 5.7+, while the former is only in 5.9+...
>>>>
>>>> We can make it work for earlier kernels by just using the thread offload
>>>> for that, and that may be worth doing. That would enable it in
>>>> 5.7-stable and 5.8-stable. For that, you just need these two patches.
>>>> Patch 1 would work as-is, while patch 2 would need a small bit of
>>>> massaging since io_read() doesn't have the retry parts.
>>>>
>>>> I'll give it a whirl just out of curiosity, then we can debate it after
>>>> that.
>>>
>>> Here are the two patches against latest 5.7-stable (the rc branch, as
>>> we had quite a few queued up after 5.9-rc1). Totally untested, just
>>> wanted to see if it was doable.
>>>
>>> First patch is mostly just applied, with various bits removed that we
>>> don't have in 5.7. The second patch just does -EAGAIN punt for the
>>> short read case, which will queue the remainder with io-wq for
>>> async execution.
>>>
>>> Obviously needs quite a bit of testing before it can go anywhere else,
>>> but wanted to throw this out there in case you were interested in
>>> giving it a go...
>>
>> Actually passes basic testing, and doesn't return short reads. So at
>> least it's not half bad, and it should be safe for you to test.
>>
>> I quickly looked at 5.8 as well, and the good news is that the same
>> patches will apply there without changes.
>
> Thanks, but I was just curios and I currently don't have the environment to test, sorry.
>
> Anoop: you helped a lot reproducing the problem with 5.6, would you be able to
> test the kernel patches against 5.7 or 5.8, while reverting the samba patches?
> See https://lore.kernel.org/io-uring/e22220a8-669a-d302-f454-03a35c9582b4@kernel.dk/T/#t for the
> whole discussion?

I'm actually not too worried about the short reads not working, it'll
naturally fall out correctly if the rest of the path is sane. The latter
is what I'd be worried about! I ran some synthetic testing and haven't
seen any issues so far, so maybe (just maybe) it's actually good.

I can setup two branches with the 5.7-stable + patches and 5.8-stable +
patches if that helps facilitate testing?

--
Jens Axboe