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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.