On Wed, Aug 21, 2013 at 05:40:11PM +0200, Paolo Bonzini wrote:
>
> We could just use a bottom half, too. Add a bottom half to acb,
> schedule it in gluster_finish_aiocb, delete it in the bottom half's own
> callback.
I tried this approach (the patch at the end of this mail), but see this
occasional errors, doesn't happen always. Any clues on what am I missing here ?
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffcbfff700 (LWP 11301)]
0x000055555597156b in event_notifier_set (e=0xa4)
at util/event_notifier-posix.c:97
97 ret = write(e->wfd, &value, sizeof(value));
(gdb) bt
#0 0x000055555597156b in event_notifier_set (e=0xa4)
at util/event_notifier-posix.c:97
#1 0x00005555555dd3cd in aio_notify (ctx=0x0) at async.c:233
#2 0x00005555555dd00f in qemu_bh_schedule (bh=0x7fff300008e0) at async.c:128
#3 0x00005555556002da in gluster_finish_aiocb (fd=0x5555562ae5d0, ret=4096,
arg=0x7fffd00419c0) at block/gluster.c:409
#4 0x00007ffff5e70cdc in glfs_io_async_cbk (ret=4096, frame=0x0, data=
0x555556443ee0) at glfs-fops.c:567
#5 0x00007ffff5c2843e in synctask_wrap (old_task=0x555556365940)
at syncop.c:133
#6 0x00007ffff3b03370 in ?? () from /lib64/libc.so.6
#7 0x0000000000000000 in ?? ()
(gdb) up
#1 0x00005555555dd3cd in aio_notify (ctx=0x0) at async.c:233
233 event_notifier_set(&ctx->notifier);
(gdb) up
#2 0x00005555555dd00f in qemu_bh_schedule (bh=0x7fff300008e0) at async.c:128
128 aio_notify(bh->ctx);
(gdb) p *bh
$1 = {ctx = 0x0, cb = 0x5555555ffdcd <qemu_gluster_aio_bh>, opaque =
0x7fffd00419c0, next = 0x555556345e70, scheduled = false, idle = false,
deleted = true}
gluster: Use BH mechanism for AIO completion
Replace the existing pipe based AIO completion handing by BH based method.
Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
---
block/gluster.c | 69 ++++++-------------------------------------------------
1 file changed, 8 insertions(+), 61 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 46f36f8..598b335 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -30,7 +30,6 @@ typedef struct GlusterAIOCB {
typedef struct BDRVGlusterState {
struct glfs *glfs;
- int fds[2];
struct glfs_fd *fd;
int event_reader_pos;
GlusterAIOCB *event_acb;
@@ -231,12 +230,13 @@ out:
return NULL;
}
-static void qemu_gluster_complete_aio(GlusterAIOCB *acb, BDRVGlusterState *s)
+static void qemu_gluster_aio_bh(void *opaque)
{
int ret;
+ GlusterAIOCB *acb = opaque;
bool *finished = acb->finished;
BlockDriverCompletionFunc *cb = acb->common.cb;
- void *opaque = acb->common.opaque;
+ void *cb_opaque = acb->common.opaque;
if (!acb->ret || acb->ret == acb->size) {
ret = 0; /* Success */
@@ -246,33 +246,15 @@ static void qemu_gluster_complete_aio(GlusterAIOCB *acb,
BDRVGlusterState *s)
ret = -EIO; /* Partial read/write - fail it */
}
+ qemu_bh_delete(acb->bh);
qemu_aio_release(acb);
- cb(opaque, ret);
+
+ cb(cb_opaque, ret);
if (finished) {
*finished = true;
}
}
-static void qemu_gluster_aio_event_reader(void *opaque)
-{
- BDRVGlusterState *s = opaque;
- ssize_t ret;
-
- do {
- char *p = (char *)&s->event_acb;
-
- ret = read(s->fds[GLUSTER_FD_READ], p + s->event_reader_pos,
- sizeof(s->event_acb) - s->event_reader_pos);
- if (ret > 0) {
- s->event_reader_pos += ret;
- if (s->event_reader_pos == sizeof(s->event_acb)) {
- s->event_reader_pos = 0;
- qemu_gluster_complete_aio(s->event_acb, s);
- }
- }
- } while (ret < 0 && errno == EINTR);
-}
-
/* TODO Convert to fine grained options */
static QemuOptsList runtime_opts = {
.name = "gluster",
@@ -329,17 +311,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict
*options,
s->fd = glfs_open(s->glfs, gconf->image, open_flags);
if (!s->fd) {
ret = -errno;
- goto out;
- }
-
- ret = qemu_pipe(s->fds);
- if (ret < 0) {
- ret = -errno;
- goto out;
}
- fcntl(s->fds[GLUSTER_FD_READ], F_SETFL, O_NONBLOCK);
- qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ],
- qemu_gluster_aio_event_reader, NULL, s);
out:
qemu_opts_del(opts);
@@ -417,31 +389,10 @@ static const AIOCBInfo gluster_aiocb_info = {
static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg)
{
GlusterAIOCB *acb = (GlusterAIOCB *)arg;
- BlockDriverState *bs = acb->common.bs;
- BDRVGlusterState *s = bs->opaque;
- int retval;
acb->ret = ret;
- retval = qemu_write_full(s->fds[GLUSTER_FD_WRITE], &acb, sizeof(acb));
- if (retval != sizeof(acb)) {
- /*
- * Gluster AIO callback thread failed to notify the waiting
- * QEMU thread about IO completion.
- *
- * Complete this IO request and make the disk inaccessible for
- * subsequent reads and writes.
- */
- error_report("Gluster failed to notify QEMU about IO completion");
-
- qemu_mutex_lock_iothread(); /* We are in gluster thread context */
- acb->common.cb(acb->common.opaque, -EIO);
- qemu_aio_release(acb);
- close(s->fds[GLUSTER_FD_READ]);
- close(s->fds[GLUSTER_FD_WRITE]);
- qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ], NULL, NULL, NULL);
- bs->drv = NULL; /* Make the disk inaccessible */
- qemu_mutex_unlock_iothread();
- }
+ acb->bh = qemu_bh_new(qemu_gluster_aio_bh, acb);
+ qemu_bh_schedule(acb->bh);
}
static BlockDriverAIOCB *qemu_gluster_aio_rw(BlockDriverState *bs,
@@ -592,10 +543,6 @@ static void qemu_gluster_close(BlockDriverState *bs)
{
BDRVGlusterState *s = bs->opaque;
- close(s->fds[GLUSTER_FD_READ]);
- close(s->fds[GLUSTER_FD_WRITE]);
- qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ], NULL, NULL, NULL);
-
if (s->fd) {
glfs_close(s->fd);
s->fd = NULL;
