From: Benjamin LaHaise <bcrl@kvack.org>
To: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: avagin@openvz.org, linux-kernel@vger.kernel.org,
viro@zeniv.linux.org.uk, gorcunov@openvz.org,
akpm@linux-foundation.org, xemul@virtuozzo.com
Subject: Re: [PATCH] aio: Add command to wait completion of all requests
Date: Wed, 14 Jun 2017 10:47:57 -0400 [thread overview]
Message-ID: <20170614144757.GC7230@kvack.org> (raw)
In-Reply-To: <8bc01429-0041-523f-8b36-d30c7b84408a@virtuozzo.com>
On Wed, Jun 14, 2017 at 12:11:38PM +0300, Kirill Tkhai wrote:
> On 14.06.2017 02:26, Benjamin LaHaise wrote:
...
> > Nope. An aio may not complete in a timely fashion, in which case your
> > wait for all aios to complete will simply wait forever. I take it this is
> > not the desired behaviour of checkpointing.
>
> But read_events() does it. What the difference between them?
read_events() does not wait for all aios to complete. The model here is
event driving programming: the program waits for an event, wakes up, does
some work, goes back to sleep until another event comes in. Some of the
aios in flight won't complete for significant amounts of time, but that
doesn't matter since those which do complete will be processed and cause
state machines to make progress.
> >> Could you please describe how will cancelling aio requests will help to wait
> >> till their completion? Is there is guarantee, they will be easily queued back?
> >> I suppose, no, because there are may be a memory limit or some low level
> >> drivers limitations, dependent on internal conditions.
> >
> > This is the problem you're facing. Quiescing aios is complicated!
>
> It's too generic words and they may refer to anything. And it's a problem,
> which is not connected with small aio engine, because it's impossible to guarantee
> availability of kernel resources from there. Imagine, parallel task eats memory
> of your just cancelled request: then you just can't queue request back.
>
> This argument makes your suggestion not rock-stable suitable for all cases:
> it will break user applications in such situations.
Again, you're failing to understand what the programming model is for aio.
> >> Also, it's not seems good to overload aio with the functionality of obtaining
> >> closed file descriptors of submitted requests.
> >>
> >> Do you mean this way, or I misunderstood you? Could you please to concretize your
> >> idea?
> >
> > Yes, this is what I'm talking about.
> >
> >> In my vision cancelling requests does not allow to implement the need I described.
> >> If we can't queue request back, it breaks snapshotting and user application in
> >> general.
> >
> > This is what you have to figure out. This is why your patch is incomplete
> > and cannot be accepted. You can't punt the complexity your feature
> > requires onto other maintainers -- your implementation has to be reasonably
> > complete at time of patch inclusion. Can you see now why your patch can't
> > be included as-is? The assumption you made that aios complete in a timely
> > fashion is incorrect. Everything else stems from that.
>
> I just can't see why read_events() just waits for requests completion not paying
> attention of the all above you said. Could you please clarify the difference
> between two these situations?
Read my above comments about event driven programming. Waiting for all
aios to complete is very different from waiting for a non-specific
completion event to come along.
-ben
--
"Thought is the essence of where you are now."
prev parent reply other threads:[~2017-06-14 14:47 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-09 9:49 [PATCH] aio: Add command to wait completion of all requests Kirill Tkhai
2017-06-13 8:14 ` Cyrill Gorcunov
2017-06-13 9:45 ` Kirill Tkhai
2017-06-13 10:32 ` Cyrill Gorcunov
2017-06-13 14:42 ` Benjamin LaHaise
2017-06-13 15:11 ` Kirill Tkhai
2017-06-13 15:26 ` Benjamin LaHaise
2017-06-13 16:17 ` Kirill Tkhai
2017-06-13 23:26 ` Benjamin LaHaise
2017-06-14 9:11 ` Kirill Tkhai
2017-06-14 14:47 ` Benjamin LaHaise [this message]
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=20170614144757.GC7230@kvack.org \
--to=bcrl@kvack.org \
--cc=akpm@linux-foundation.org \
--cc=avagin@openvz.org \
--cc=gorcunov@openvz.org \
--cc=ktkhai@virtuozzo.com \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
--cc=xemul@virtuozzo.com \
/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