From: Paolo Bonzini <pbonzini@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] win32: fix main-loop busy loop on socket/fd event
Date: Wed, 4 Jan 2017 16:19:31 -0500 (EST) [thread overview]
Message-ID: <119554983.4442989.1483564771761.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20170104205936.27279-1-marcandre.lureau@redhat.com>
> Paolo suggested to me on irc to call event_notifier_test_and_clear()
> after select() >0 from aio-win32.c's aio_prepare. Unfortunately, not all
> fds associated with ctx->notifiers are in AIO fd handlers set.
> (qemu_set_nonblock() in util/oslib-win32.c calls qemu_fd_register()).
That makes sense. Out of curiosity, what is a practical case of a socket
that is nonblocking but doesn't have an attached handler?
Another possibility (this one requires much more attention to avoid missing
some edge case; however, it should be easy to verify if it fixes the busy
loop) could be to move aio_notify_accept to just before setting ctx->notify_me.
This would work for both aio-posix and aio-win32.
Paolo
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/block/aio.h | 2 ++
> async.c | 6 ++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/include/block/aio.h b/include/block/aio.h
> index ca551e346f..a6da135bf3 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -100,6 +100,7 @@ struct AioContext {
> */
> int walking_bh;
>
> +#ifndef _WIN32
> /* Used by aio_notify.
> *
> * "notified" is used to avoid expensive event_notifier_test_and_clear
> @@ -113,6 +114,7 @@ struct AioContext {
> * in the docs/aio_notify_accept.promela formal model.
> */
> bool notified;
> +#endif
> EventNotifier notifier;
>
> /* Thread pool for performing work and receiving completion callbacks */
> diff --git a/async.c b/async.c
> index b2de360c23..8c2a68b6cc 100644
> --- a/async.c
> +++ b/async.c
> @@ -329,15 +329,21 @@ void aio_notify(AioContext *ctx)
> smp_mb();
> if (ctx->notify_me) {
> event_notifier_set(&ctx->notifier);
> +#ifndef _WIN32
> atomic_mb_set(&ctx->notified, true);
> +#endif
> }
> }
>
> void aio_notify_accept(AioContext *ctx)
> {
> +#ifndef _WIN32
> if (atomic_xchg(&ctx->notified, false)) {
> +#endif
> event_notifier_test_and_clear(&ctx->notifier);
> +#ifndef _WIN32
> }
> +#endif
> }
>
> static void aio_timerlist_notify(void *opaque)
> --
> 2.11.0
>
>
next prev parent reply other threads:[~2017-01-04 21:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-04 20:59 [Qemu-devel] [PATCH] win32: fix main-loop busy loop on socket/fd event Marc-André Lureau
2017-01-04 21:19 ` Paolo Bonzini [this message]
2017-01-04 21:39 ` Marc-André Lureau
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=119554983.4442989.1483564771761.JavaMail.zimbra@redhat.com \
--to=pbonzini@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=qemu-devel@nongnu.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).