* [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio
@ 2011-09-19 11:54 Frediano Ziglio
2011-09-19 11:54 ` [Qemu-devel] [PATCH 1/2] block: avoid storing a constant in qemu_paiocb structure Frediano Ziglio
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Frediano Ziglio @ 2011-09-19 11:54 UTC (permalink / raw)
To: aliguori, kwolf; +Cc: qemu-devel, Frediano Ziglio
Now that iothread is always compiled sending a signal seems only an
additional step. This patch also avoid writing to two pipe (one from signal
and one in qemu_service_io).
Tested and works correctly with KVM enabled. Performances are only sligthly
better (as I expected). strace output is more readable.
Doubts:
- any sense having two patches and not only last one?
- comment is perhaps wrong as is also affect main core
- is ok if KVM is disabled? more testing required
Frediano Ziglio (2):
block: avoid storing a constant in qemu_paiocb structure
block: avoid SIGUSR2
cpus.c | 5 -----
posix-aio-compat.c | 17 ++++-------------
2 files changed, 4 insertions(+), 18 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 1/2] block: avoid storing a constant in qemu_paiocb structure 2011-09-19 11:54 [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio Frediano Ziglio @ 2011-09-19 11:54 ` Frediano Ziglio 2011-09-19 11:54 ` [Qemu-devel] [PATCH 2/2] block: avoid SIGUSR2 Frediano Ziglio 2011-09-19 14:08 ` [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio Stefan Hajnoczi 2 siblings, 0 replies; 6+ messages in thread From: Frediano Ziglio @ 2011-09-19 11:54 UTC (permalink / raw) To: aliguori, kwolf; +Cc: qemu-devel, Frediano Ziglio Signed-off-by: Frediano Ziglio <freddy77@gmail.com> --- posix-aio-compat.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/posix-aio-compat.c b/posix-aio-compat.c index 3193dbf..7ea63a1 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -42,7 +42,6 @@ struct qemu_paiocb { int aio_niov; size_t aio_nbytes; #define aio_ioctl_cmd aio_nbytes /* for QEMU_AIO_IOCTL */ - int ev_signo; off_t aio_offset; QTAILQ_ENTRY(qemu_paiocb) node; @@ -381,7 +380,7 @@ static void *aio_thread(void *unused) aiocb->ret = ret; mutex_unlock(&lock); - if (kill(pid, aiocb->ev_signo)) die("kill failed"); + if (kill(pid, SIGUSR2)) die("kill failed"); } cur_threads--; @@ -623,7 +622,6 @@ BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd, return NULL; acb->aio_type = type; acb->aio_fildes = fd; - acb->ev_signo = SIGUSR2; if (qiov) { acb->aio_iov = qiov->iov; @@ -651,7 +649,6 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd, return NULL; acb->aio_type = QEMU_AIO_IOCTL; acb->aio_fildes = fd; - acb->ev_signo = SIGUSR2; acb->aio_offset = 0; acb->aio_ioctl_buf = buf; acb->aio_ioctl_cmd = req; -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] block: avoid SIGUSR2 2011-09-19 11:54 [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio Frediano Ziglio 2011-09-19 11:54 ` [Qemu-devel] [PATCH 1/2] block: avoid storing a constant in qemu_paiocb structure Frediano Ziglio @ 2011-09-19 11:54 ` Frediano Ziglio 2011-09-19 14:05 ` Stefan Hajnoczi 2011-09-19 14:08 ` [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio Stefan Hajnoczi 2 siblings, 1 reply; 6+ messages in thread From: Frediano Ziglio @ 2011-09-19 11:54 UTC (permalink / raw) To: aliguori, kwolf; +Cc: qemu-devel, Frediano Ziglio Signed-off-by: Frediano Ziglio <freddy77@gmail.com> --- cpus.c | 5 ----- posix-aio-compat.c | 14 ++++---------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/cpus.c b/cpus.c index 54c188c..d0cfe91 100644 --- a/cpus.c +++ b/cpus.c @@ -380,11 +380,6 @@ static int qemu_signal_init(void) int sigfd; sigset_t set; - /* SIGUSR2 used by posix-aio-compat.c */ - sigemptyset(&set); - sigaddset(&set, SIGUSR2); - pthread_sigmask(SIG_UNBLOCK, &set, NULL); - /* * SIG_IPI must be blocked in the main thread and must not be caught * by sigwait() in the signal thread. Otherwise, the cpu thread will diff --git a/posix-aio-compat.c b/posix-aio-compat.c index 7ea63a1..72c6dbb 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -308,6 +308,8 @@ static ssize_t handle_aiocb_rw(struct qemu_paiocb *aiocb) return nbytes; } +static void posix_aio_notify_event(void); + static void *aio_thread(void *unused) { pid_t pid; @@ -380,7 +382,7 @@ static void *aio_thread(void *unused) aiocb->ret = ret; mutex_unlock(&lock); - if (kill(pid, SIGUSR2)) die("kill failed"); + posix_aio_notify_event(); } cur_threads--; @@ -547,7 +549,7 @@ static int posix_aio_flush(void *opaque) static PosixAioState *posix_aio_state; -static void aio_signal_handler(int signum) +static void posix_aio_notify_event(void) { if (posix_aio_state) { char byte = 0; @@ -557,8 +559,6 @@ static void aio_signal_handler(int signum) if (ret < 0 && errno != EAGAIN) die("write()"); } - - qemu_service_io(); } static void paio_remove(struct qemu_paiocb *acb) @@ -662,7 +662,6 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd, int paio_init(void) { - struct sigaction act; PosixAioState *s; int fds[2]; int ret; @@ -672,11 +671,6 @@ int paio_init(void) s = g_malloc(sizeof(PosixAioState)); - sigfillset(&act.sa_mask); - act.sa_flags = 0; /* do not restart syscalls to interrupt select() */ - act.sa_handler = aio_signal_handler; - sigaction(SIGUSR2, &act, NULL); - s->first_aio = NULL; if (qemu_pipe(fds) == -1) { fprintf(stderr, "failed to create pipe\n"); -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block: avoid SIGUSR2 2011-09-19 11:54 ` [Qemu-devel] [PATCH 2/2] block: avoid SIGUSR2 Frediano Ziglio @ 2011-09-19 14:05 ` Stefan Hajnoczi 2011-09-20 18:08 ` Christoph Hellwig 0 siblings, 1 reply; 6+ messages in thread From: Stefan Hajnoczi @ 2011-09-19 14:05 UTC (permalink / raw) To: Frediano Ziglio; +Cc: kwolf, aliguori, qemu-devel On Mon, Sep 19, 2011 at 12:54 PM, Frediano Ziglio <freddy77@gmail.com> wrote: > @@ -547,7 +549,7 @@ static int posix_aio_flush(void *opaque) > > static PosixAioState *posix_aio_state; > > -static void aio_signal_handler(int signum) > +static void posix_aio_notify_event(void) > { > if (posix_aio_state) { Seems like this if statement is always true, hence the condition can be removed. The posix_aio_state global is always non-NULL here. Stefan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block: avoid SIGUSR2 2011-09-19 14:05 ` Stefan Hajnoczi @ 2011-09-20 18:08 ` Christoph Hellwig 0 siblings, 0 replies; 6+ messages in thread From: Christoph Hellwig @ 2011-09-20 18:08 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: kwolf, aliguori, Frediano Ziglio, qemu-devel On Mon, Sep 19, 2011 at 03:05:47PM +0100, Stefan Hajnoczi wrote: > On Mon, Sep 19, 2011 at 12:54 PM, Frediano Ziglio <freddy77@gmail.com> wrote: > > @@ -547,7 +549,7 @@ static int posix_aio_flush(void *opaque) > > > > ?static PosixAioState *posix_aio_state; > > > > -static void aio_signal_handler(int signum) > > +static void posix_aio_notify_event(void) > > ?{ > > ? ? if (posix_aio_state) { > > Seems like this if statement is always true, hence the condition can > be removed. The posix_aio_state global is always non-NULL here. It is. In addition to dropping it I'd also recommend moving the function up so that it's above it's caller. Btw, what is the point of the qemu_service_io? Calling qemu_notify_event directly would seem a lot more obvious, given that the name actually gives a hint on what it actually does. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio 2011-09-19 11:54 [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio Frediano Ziglio 2011-09-19 11:54 ` [Qemu-devel] [PATCH 1/2] block: avoid storing a constant in qemu_paiocb structure Frediano Ziglio 2011-09-19 11:54 ` [Qemu-devel] [PATCH 2/2] block: avoid SIGUSR2 Frediano Ziglio @ 2011-09-19 14:08 ` Stefan Hajnoczi 2 siblings, 0 replies; 6+ messages in thread From: Stefan Hajnoczi @ 2011-09-19 14:08 UTC (permalink / raw) To: Frediano Ziglio; +Cc: kwolf, aliguori, qemu-devel On Mon, Sep 19, 2011 at 12:54 PM, Frediano Ziglio <freddy77@gmail.com> wrote: > Now that iothread is always compiled sending a signal seems only an > additional step. This patch also avoid writing to two pipe (one from signal > and one in qemu_service_io). > > Tested and works correctly with KVM enabled. Performances are only sligthly > better (as I expected). strace output is more readable. > > Doubts: > - any sense having two patches and not only last one? Personally I would just send one here. > - is ok if KVM is disabled? more testing required If I understand correctly it used to interrupt guest code execution whereas now it simply invoked a callback in the I/O thread. Normally this callback will raise a guest interrupt and get the vCPU to notice that I/O has completed. So I think this should work. Stefan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-20 18:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-19 11:54 [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio Frediano Ziglio 2011-09-19 11:54 ` [Qemu-devel] [PATCH 1/2] block: avoid storing a constant in qemu_paiocb structure Frediano Ziglio 2011-09-19 11:54 ` [Qemu-devel] [PATCH 2/2] block: avoid SIGUSR2 Frediano Ziglio 2011-09-19 14:05 ` Stefan Hajnoczi 2011-09-20 18:08 ` Christoph Hellwig 2011-09-19 14:08 ` [Qemu-devel] [PATCH 0/2][RFC?] Remove SIGUSR2 from posix-aio Stefan Hajnoczi
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).