public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: Anoop C S <[email protected]>,
	Stefan Metzmacher <[email protected]>,
	[email protected]
Cc: [email protected], [email protected]
Subject: Re: [PATCHSET v2 0/2] io_uring: handle short reads internally
Date: Tue, 18 Aug 2020 08:23:31 -0700	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 8/18/20 7:53 AM, Jens Axboe wrote:
> On 8/18/20 7:49 AM, Anoop C S wrote:
>> On Tue, 2020-08-18 at 07:44 -0700, Jens Axboe wrote:
>>> 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/[email protected]/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?
>>
>> That would be great.
>>
>> I took those two patches and tried to apply on top of 5.7.y. I had to
>> manually resolve very few conflicts. I am not sure whether it is OK or
>> not to test such a patched version(because of conflicts).
> 
> I pushed out two branches:
> 
> 5.8-stable: current 5.8-stable rc queue + the three patches for this
> 5.7-stable: 5.7 ditto
> 
> So pick which one you want to use, and then pull it.
> 
> git://git.kernel.dk/linux-block 5.8-stable
> 
> git://git.kernel.dk/linux-block 5.7-stable
> 
> Hope that helps!

Ran these through the liburing regression testing as well, and found a
case where 'ret2' isn't initialized. So pushed out new branches. The
correct sha for testing should be:

5.7-stable: a451911d530075352fbc7ef9bb2df68145a747ad
5.8-stable: b101e651782a60eb1e96b64e523e51358b77f94f

-- 
Jens Axboe


  reply	other threads:[~2020-08-18 15:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14 19:54 [PATCHSET v2 0/2] io_uring: handle short reads internally Jens Axboe
2020-08-14 19:54 ` [PATCH 1/2] io_uring: retain iov_iter state over io_read/io_write calls Jens Axboe
2020-08-14 19:54 ` [PATCH 2/2] io_uring: internally retry short reads Jens Axboe
2020-08-17  9:25 ` [PATCHSET v2 0/2] io_uring: handle short reads internally Stefan Metzmacher
2020-08-18  3:29   ` Jens Axboe
2020-08-18  4:12     ` Jens Axboe
2020-08-18  4:30       ` Jens Axboe
2020-08-18  7:40         ` Stefan Metzmacher
2020-08-18 14:44           ` Jens Axboe
2020-08-18 14:49             ` Anoop C S
2020-08-18 14:53               ` Jens Axboe
2020-08-18 15:23                 ` Jens Axboe [this message]
     [not found]                   ` <[email protected]>
2020-08-19  8:31                     ` Stefan Metzmacher
2020-08-19 12:48                       ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox