io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Stefan Metzmacher <metze@samba.org>,
	io-uring@vger.kernel.org, Anoop C S <anoopcs@cryptolab.net>
Cc: david@fromorbit.com, jmoyer@redhat.com
Subject: Re: [PATCHSET v2 0/2] io_uring: handle short reads internally
Date: Tue, 18 Aug 2020 07:44:27 -0700	[thread overview]
Message-ID: <db051fac-da0f-9546-2c32-1756d9e74529@kernel.dk> (raw)
In-Reply-To: <5f6d3f16-cd0c-9598-4484-6003101eb47a@samba.org>

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


  reply	other threads:[~2020-08-18 14:44 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 [this message]
2020-08-18 14:49             ` Anoop C S
2020-08-18 14:53               ` Jens Axboe
2020-08-18 15:23                 ` Jens Axboe
     [not found]                   ` <ad6cd95adb2e7622860fd9a80c19e48230ae2747.camel@cryptolab.net>
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 \
    --in-reply-to=db051fac-da0f-9546-2c32-1756d9e74529@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=anoopcs@cryptolab.net \
    --cc=david@fromorbit.com \
    --cc=io-uring@vger.kernel.org \
    --cc=jmoyer@redhat.com \
    --cc=metze@samba.org \
    /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;
as well as URLs for NNTP newsgroup(s).