From: Anthony Liguori <anthony@codemonkey.ws>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
Stefan Hajnoczi <stefanha@gmail.com>, Avi Kivity <avi@redhat.com>,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] event_notifier: move to top-level directory
Date: Tue, 27 Sep 2011 11:05:15 -0500 [thread overview]
Message-ID: <4E81F43B.1040708@codemonkey.ws> (raw)
In-Reply-To: <4E81EB8A.3070202@siemens.com>
[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]
On 09/27/2011 10:28 AM, Jan Kiszka wrote:
> On 2011-09-27 17:17, Stefan Hajnoczi wrote:
>> On Tue, Sep 27, 2011 at 3:26 PM, Avi Kivity<avi@redhat.com> wrote:
>>> Has no business in hw/.
>>>
>>> Signed-off-by: Avi Kivity<avi@redhat.com>
>>> ---
>>> hw/event_notifier.c => event_notifier.c | 1 -
>>> hw/event_notifier.h => event_notifier.h | 0
>>> 2 files changed, 0 insertions(+), 1 deletions(-)
>>> rename hw/event_notifier.c => event_notifier.c (98%)
>>> rename hw/event_notifier.h => event_notifier.h (100%)
>>
>> Yay. Now perhaps we can kill qemu_eventfd(), whose users are
>> typically poking around with write(2) and read(2) when really they
>> could use the high-level event_notifier.h API.
>
> EventNotifiers will have to be superseded by something more generic
> first. It's not fully covering the use cases of cpus.c and
> posix-aio-compat.c.
Actually, for posix-aio, we can just switch to using g_idle_add(). g_idle_add()
uses g_source_attach which is thread safe. g_idle_add() gives you a thread safe
mechanism to defer a piece of work to the main loop which is really what we want
here.
This can actually be made to work with sync I/O emulation too by having another
GMainLoop in the sync I/O loop although I thought I recalled a patch series to
remove that stuff.
Kevin/Stefan, what's the plans for sync I/O emulation?
See untested patch below.
Regards,
Anthony Liguori
>
> Jan
>
[-- Attachment #2: 0001-posix-aio-compat-use-g_idle_add.patch --]
[-- Type: text/x-patch, Size: 2858 bytes --]
>From cf036a192f09ea76d89648b83ec84a4226d14172 Mon Sep 17 00:00:00 2001
From: Anthony Liguori <aliguori@us.ibm.com>
Date: Tue, 27 Sep 2011 11:01:51 -0500
Subject: [PATCH] posix-aio-compat: use g_idle_add()
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
posix-aio-compat.c | 51 +++++++--------------------------------------------
1 files changed, 7 insertions(+), 44 deletions(-)
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index d3c1174..1f746be 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -52,7 +52,6 @@ struct qemu_paiocb {
};
typedef struct PosixAioState {
- int rfd, wfd;
struct qemu_paiocb *first_aio;
} PosixAioState;
@@ -466,7 +465,7 @@ static int qemu_paio_error(struct qemu_paiocb *aiocb)
return ret;
}
-static int posix_aio_process_queue(void *opaque)
+static gboolean posix_aio_process_queue(void *opaque)
{
PosixAioState *s = opaque;
struct qemu_paiocb *acb, **pacb;
@@ -477,8 +476,9 @@ static int posix_aio_process_queue(void *opaque)
pacb = &s->first_aio;
for(;;) {
acb = *pacb;
- if (!acb)
- return result;
+ if (!acb) {
+ goto out;
+ }
ret = qemu_paio_error(acb);
if (ret == ECANCELED) {
@@ -513,27 +513,8 @@ static int posix_aio_process_queue(void *opaque)
}
}
- return result;
-}
-
-static void posix_aio_read(void *opaque)
-{
- PosixAioState *s = opaque;
- ssize_t len;
-
- /* read all bytes from signal pipe */
- for (;;) {
- char bytes[16];
-
- len = read(s->rfd, bytes, sizeof(bytes));
- if (len == -1 && errno == EINTR)
- continue; /* try again */
- if (len == sizeof(bytes))
- continue; /* more to read */
- break;
- }
-
- posix_aio_process_queue(s);
+out:
+ return FALSE;
}
static int posix_aio_flush(void *opaque)
@@ -546,12 +527,7 @@ static PosixAioState *posix_aio_state;
static void posix_aio_notify_event(void)
{
- char byte = 0;
- ssize_t ret;
-
- ret = write(posix_aio_state->wfd, &byte, sizeof(byte));
- if (ret < 0 && errno != EAGAIN)
- die("write()");
+ g_idle_add(posix_aio_process_queue, posix_aio_state);
}
static void paio_remove(struct qemu_paiocb *acb)
@@ -665,19 +641,6 @@ int paio_init(void)
s = g_malloc(sizeof(PosixAioState));
s->first_aio = NULL;
- if (qemu_pipe(fds) == -1) {
- fprintf(stderr, "failed to create pipe\n");
- return -1;
- }
-
- s->rfd = fds[0];
- s->wfd = fds[1];
-
- fcntl(s->rfd, F_SETFL, O_NONBLOCK);
- fcntl(s->wfd, F_SETFL, O_NONBLOCK);
-
- qemu_aio_set_fd_handler(s->rfd, posix_aio_read, NULL, posix_aio_flush,
- posix_aio_process_queue, s);
ret = pthread_attr_init(&attr);
if (ret)
--
1.7.4.1
next prev parent reply other threads:[~2011-09-27 16:05 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-27 14:26 [Qemu-devel] [PATCH] event_notifier: move to top-level directory Avi Kivity
2011-09-27 15:17 ` Stefan Hajnoczi
2011-09-27 15:28 ` Jan Kiszka
2011-09-27 16:05 ` Anthony Liguori [this message]
2011-09-27 16:39 ` Paolo Bonzini
2011-09-27 21:23 ` Anthony Liguori
2011-09-28 6:27 ` Paolo Bonzini
2011-09-28 7:52 ` Jan Kiszka
2011-09-28 8:00 ` Paolo Bonzini
2011-09-28 9:12 ` Stefan Hajnoczi
2011-11-01 22:18 ` Anthony Liguori
2011-11-02 18:10 ` Alexander Graf
2011-11-02 17:12 ` Avi Kivity
2011-11-02 17:13 ` Avi Kivity
2011-11-02 18:23 ` Alexander Graf
2011-11-02 17:22 ` Avi Kivity
2011-11-02 17:27 ` Alexander Graf
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=4E81F43B.1040708@codemonkey.ws \
--to=anthony@codemonkey.ws \
--cc=avi@redhat.com \
--cc=jan.kiszka@siemens.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@gmail.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;
as well as URLs for NNTP newsgroup(s).