* [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane
@ 2015-07-23 6:32 Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 01/11] aio: Introduce "type" in aio_set_fd_handler and aio_set_event_notifier Fam Zheng
` (11 more replies)
0 siblings, 12 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
Most existing nested aio_poll()'s in block layer are inconsiderate of
dispatching potential new r/w requests from ioeventfds and nbd exports, which
might result in responsiveness issues (e.g. bdrv_drain_all will not return when
new requests keep coming), or even wrong semantics (e.g. qmp_transaction cannot
enforce atomicity due to aio_poll in bdrv_drain_all).
Previous attampts to address this issue include new op blocker[1], bdrv_lock[2]
and nested AioContext (patches not posted to qemu-devel).
This approach is based on the idea proposed by Paolo Bonzini. The original idea
is introducing "aio_context_disable_client / aio_context_enable_client to
filter AioContext handlers according to the "client", e.g.
AIO_CLIENT_DATAPLANE (ioeventfd), AIO_CLIENT_PROTOCOL, AIO_CLIENT_NBD_SERVER,
AIO_CLIENT_CONTEXT, ... Extend aio_set_{event_notifier,fd}_handler to pass a
client (type)."
What is realized in this series is similar except the "disable, poll, enable"
method, instead the bit mask of interesting client types is passed to
aio_poll() (to be exact, passed to aio_poll_clients()). That is because,
aio_poll may release the AioContext lock around ppoll, avoiding state will make
the interface clearer.
After this series, block layer aio_poll() will only process those "protocol"
fds that are used in block I/O, plus the ctx->notifier for aio_notify(); other
aio_poll()'s keep unchanged.
The biggest advantage over approaches [1] and [2] is, no change is needed in
virtio-{blk,scsi}-dataplane code, also this doesn't depend on converting QMP to
coroutines.
The windows implementation is not tested except for compiling.
[1]: https://lists.gnu.org/archive/html/qemu-block/2015-05/msg00800.html
[2]: http://lists.nongnu.org/archive/html/qemu-block/2015-06/msg00027.html
Fam Zheng (11):
aio: Introduce "type" in aio_set_fd_handler and aio_set_event_notifier
aio: Save type to AioHandler
aio-posix: Introduce aio_poll_clients
aio-win32: Implement aio_poll_clients
block: Mark fd handlers as "protocol"
nbd: Mark fd handlers client type as "nbd server"
aio: Mark ctx->notifier's client type as "context"
dataplane: Mark host notifiers' client type as "dataplane"
block: Introduce bdrv_aio_poll
block: Replace nested aio_poll with bdrv_aio_poll
block: Only poll block layer fds in bdrv_aio_poll
aio-posix.c | 28 +++++++++++++++-----
aio-win32.c | 42 +++++++++++++++++++++++------
async.c | 3 ++-
block.c | 2 +-
block/curl.c | 16 +++++++-----
block/io.c | 24 ++++++++++-------
block/iscsi.c | 9 +++----
block/linux-aio.c | 5 ++--
block/nbd-client.c | 10 ++++---
block/nfs.c | 19 ++++++--------
block/qed-table.c | 8 +++---
block/sheepdog.c | 32 +++++++++++++++--------
block/ssh.c | 5 ++--
block/win32-aio.c | 5 ++--
blockjob.c | 2 +-
hw/block/dataplane/virtio-blk.c | 6 +++--
hw/scsi/virtio-scsi-dataplane.c | 24 +++++++++++------
include/block/aio.h | 20 +++++++++++++-
include/block/block.h | 2 ++
nbd.c | 4 ++-
qemu-img.c | 2 +-
qemu-io-cmds.c | 4 +--
tests/test-aio.c | 58 +++++++++++++++++++++++------------------
23 files changed, 215 insertions(+), 115 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 01/11] aio: Introduce "type" in aio_set_fd_handler and aio_set_event_notifier
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 02/11] aio: Save type to AioHandler Fam Zheng
` (10 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
The parameter is added but not used.
The callers are converted with following coccinelle semantic patch:
@@
expression E1, E2, E3, E4, E5;
@@
(
-aio_set_event_notifier(E1, E2, E3)
+aio_set_event_notifier(E1, E2, AIO_CLIENT_UNSPECIFIED, E3)
|
-aio_set_fd_handler(E1, E2, E3, E4, E5)
+aio_set_fd_handler(E1, E2, AIO_CLIENT_UNSPECIFIED, E3, E4, E5)
)
Signed-off-by: Fam Zheng <famz@redhat.com>
---
aio-posix.c | 4 ++-
aio-win32.c | 2 ++
async.c | 3 ++-
block/curl.c | 14 +++++-----
block/iscsi.c | 9 +++----
block/linux-aio.c | 5 ++--
block/nbd-client.c | 10 ++++---
block/nfs.c | 17 +++++-------
block/sheepdog.c | 32 +++++++++++++++--------
block/ssh.c | 5 ++--
block/win32-aio.c | 5 ++--
hw/block/dataplane/virtio-blk.c | 6 +++--
hw/scsi/virtio-scsi-dataplane.c | 24 +++++++++++------
include/block/aio.h | 5 ++++
nbd.c | 4 ++-
tests/test-aio.c | 58 +++++++++++++++++++++++------------------
16 files changed, 122 insertions(+), 81 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index d477033..56f2bce 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -43,6 +43,7 @@ static AioHandler *find_aio_handler(AioContext *ctx, int fd)
void aio_set_fd_handler(AioContext *ctx,
int fd,
+ int type,
IOHandler *io_read,
IOHandler *io_write,
void *opaque)
@@ -92,10 +93,11 @@ void aio_set_fd_handler(AioContext *ctx,
void aio_set_event_notifier(AioContext *ctx,
EventNotifier *notifier,
+ int type,
EventNotifierHandler *io_read)
{
aio_set_fd_handler(ctx, event_notifier_get_fd(notifier),
- (IOHandler *)io_read, NULL, notifier);
+ type, (IOHandler *)io_read, NULL, notifier);
}
bool aio_prepare(AioContext *ctx)
diff --git a/aio-win32.c b/aio-win32.c
index 50a6867..90e7a4b 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -33,6 +33,7 @@ struct AioHandler {
void aio_set_fd_handler(AioContext *ctx,
int fd,
+ int type,
IOHandler *io_read,
IOHandler *io_write,
void *opaque)
@@ -98,6 +99,7 @@ void aio_set_fd_handler(AioContext *ctx,
void aio_set_event_notifier(AioContext *ctx,
EventNotifier *e,
+ int type,
EventNotifierHandler *io_notify)
{
AioHandler *node;
diff --git a/async.c b/async.c
index 9a98a74..43f9425 100644
--- a/async.c
+++ b/async.c
@@ -231,7 +231,7 @@ aio_ctx_finalize(GSource *source)
AioContext *ctx = (AioContext *) source;
thread_pool_free(ctx->thread_pool);
- aio_set_event_notifier(ctx, &ctx->notifier, NULL);
+ aio_set_event_notifier(ctx, &ctx->notifier, AIO_CLIENT_UNSPECIFIED, NULL);
event_notifier_cleanup(&ctx->notifier);
rfifolock_destroy(&ctx->lock);
qemu_mutex_destroy(&ctx->bh_lock);
@@ -306,6 +306,7 @@ AioContext *aio_context_new(Error **errp)
}
g_source_set_can_recurse(&ctx->source, true);
aio_set_event_notifier(ctx, &ctx->notifier,
+ AIO_CLIENT_UNSPECIFIED,
(EventNotifierHandler *)
event_notifier_dummy_cb);
ctx->thread_pool = NULL;
diff --git a/block/curl.c b/block/curl.c
index 032cc8a..6925672 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -154,18 +154,20 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, fd);
switch (action) {
case CURL_POLL_IN:
- aio_set_fd_handler(s->aio_context, fd, curl_multi_read,
- NULL, state);
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ curl_multi_read, NULL, state);
break;
case CURL_POLL_OUT:
- aio_set_fd_handler(s->aio_context, fd, NULL, curl_multi_do, state);
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ NULL, curl_multi_do, state);
break;
case CURL_POLL_INOUT:
- aio_set_fd_handler(s->aio_context, fd, curl_multi_read,
- curl_multi_do, state);
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ curl_multi_read, curl_multi_do, state);
break;
case CURL_POLL_REMOVE:
- aio_set_fd_handler(s->aio_context, fd, NULL, NULL, NULL);
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ NULL, NULL, NULL);
break;
}
diff --git a/block/iscsi.c b/block/iscsi.c
index 5002916..0ee1295 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -291,8 +291,8 @@ iscsi_set_events(IscsiLun *iscsilun)
int ev = iscsi_which_events(iscsi);
if (ev != iscsilun->events) {
- aio_set_fd_handler(iscsilun->aio_context,
- iscsi_get_fd(iscsi),
+ aio_set_fd_handler(iscsilun->aio_context, iscsi_get_fd(iscsi),
+ AIO_CLIENT_UNSPECIFIED,
(ev & POLLIN) ? iscsi_process_read : NULL,
(ev & POLLOUT) ? iscsi_process_write : NULL,
iscsilun);
@@ -1276,9 +1276,8 @@ static void iscsi_detach_aio_context(BlockDriverState *bs)
{
IscsiLun *iscsilun = bs->opaque;
- aio_set_fd_handler(iscsilun->aio_context,
- iscsi_get_fd(iscsilun->iscsi),
- NULL, NULL, NULL);
+ aio_set_fd_handler(iscsilun->aio_context, iscsi_get_fd(iscsilun->iscsi),
+ AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
iscsilun->events = 0;
if (iscsilun->nop_timer) {
diff --git a/block/linux-aio.c b/block/linux-aio.c
index c991443..0921bde 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -287,7 +287,7 @@ void laio_detach_aio_context(void *s_, AioContext *old_context)
{
struct qemu_laio_state *s = s_;
- aio_set_event_notifier(old_context, &s->e, NULL);
+ aio_set_event_notifier(old_context, &s->e, AIO_CLIENT_UNSPECIFIED, NULL);
qemu_bh_delete(s->completion_bh);
}
@@ -296,7 +296,8 @@ void laio_attach_aio_context(void *s_, AioContext *new_context)
struct qemu_laio_state *s = s_;
s->completion_bh = aio_bh_new(new_context, qemu_laio_completion_bh, s);
- aio_set_event_notifier(new_context, &s->e, qemu_laio_completion_cb);
+ aio_set_event_notifier(new_context, &s->e, AIO_CLIENT_UNSPECIFIED,
+ qemu_laio_completion_cb);
}
void *laio_init(void)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index e1bb919..36c46c5 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -124,7 +124,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
s->send_coroutine = qemu_coroutine_self();
aio_context = bdrv_get_aio_context(bs);
- aio_set_fd_handler(aio_context, s->sock,
+ aio_set_fd_handler(aio_context, s->sock, AIO_CLIENT_UNSPECIFIED,
nbd_reply_ready, nbd_restart_write, bs);
if (qiov) {
if (!s->is_unix) {
@@ -144,7 +144,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
} else {
rc = nbd_send_request(s->sock, request);
}
- aio_set_fd_handler(aio_context, s->sock, nbd_reply_ready, NULL, bs);
+ aio_set_fd_handler(aio_context, s->sock, AIO_CLIENT_UNSPECIFIED,
+ nbd_reply_ready, NULL, bs);
s->send_coroutine = NULL;
qemu_co_mutex_unlock(&s->send_mutex);
return rc;
@@ -348,14 +349,15 @@ int nbd_client_co_discard(BlockDriverState *bs, int64_t sector_num,
void nbd_client_detach_aio_context(BlockDriverState *bs)
{
aio_set_fd_handler(bdrv_get_aio_context(bs),
- nbd_get_client_session(bs)->sock, NULL, NULL, NULL);
+ nbd_get_client_session(bs)->sock,
+ AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
}
void nbd_client_attach_aio_context(BlockDriverState *bs,
AioContext *new_context)
{
aio_set_fd_handler(new_context, nbd_get_client_session(bs)->sock,
- nbd_reply_ready, NULL, bs);
+ AIO_CLIENT_UNSPECIFIED, nbd_reply_ready, NULL, bs);
}
void nbd_client_close(BlockDriverState *bs)
diff --git a/block/nfs.c b/block/nfs.c
index c026ff6..a21dd6f 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -62,11 +62,10 @@ static void nfs_set_events(NFSClient *client)
{
int ev = nfs_which_events(client->context);
if (ev != client->events) {
- aio_set_fd_handler(client->aio_context,
- nfs_get_fd(client->context),
+ aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
+ AIO_CLIENT_UNSPECIFIED,
(ev & POLLIN) ? nfs_process_read : NULL,
- (ev & POLLOUT) ? nfs_process_write : NULL,
- client);
+ (ev & POLLOUT) ? nfs_process_write : NULL, client);
}
client->events = ev;
@@ -241,9 +240,8 @@ static void nfs_detach_aio_context(BlockDriverState *bs)
{
NFSClient *client = bs->opaque;
- aio_set_fd_handler(client->aio_context,
- nfs_get_fd(client->context),
- NULL, NULL, NULL);
+ aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
+ AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
client->events = 0;
}
@@ -262,9 +260,8 @@ static void nfs_client_close(NFSClient *client)
if (client->fh) {
nfs_close(client->context, client->fh);
}
- aio_set_fd_handler(client->aio_context,
- nfs_get_fd(client->context),
- NULL, NULL, NULL);
+ aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
+ AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
nfs_destroy_context(client->context);
}
memset(client, 0, sizeof(NFSClient));
diff --git a/block/sheepdog.c b/block/sheepdog.c
index bd7cbed..e0552b7 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -624,14 +624,16 @@ static coroutine_fn void do_co_req(void *opaque)
unsigned int *rlen = srco->rlen;
co = qemu_coroutine_self();
- aio_set_fd_handler(srco->aio_context, sockfd, NULL, restart_co_req, co);
+ aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_UNSPECIFIED,
+ NULL, restart_co_req, co);
ret = send_co_req(sockfd, hdr, data, wlen);
if (ret < 0) {
goto out;
}
- aio_set_fd_handler(srco->aio_context, sockfd, restart_co_req, NULL, co);
+ aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_UNSPECIFIED,
+ restart_co_req, NULL, co);
ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
if (ret != sizeof(*hdr)) {
@@ -656,7 +658,8 @@ static coroutine_fn void do_co_req(void *opaque)
out:
/* there is at most one request for this sockfd, so it is safe to
* set each handler to NULL. */
- aio_set_fd_handler(srco->aio_context, sockfd, NULL, NULL, NULL);
+ aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_UNSPECIFIED,
+ NULL, NULL, NULL);
srco->ret = ret;
srco->finished = true;
@@ -740,7 +743,8 @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
BDRVSheepdogState *s = opaque;
AIOReq *aio_req, *next;
- aio_set_fd_handler(s->aio_context, s->fd, NULL, NULL, NULL);
+ aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_UNSPECIFIED, NULL,
+ NULL, NULL);
close(s->fd);
s->fd = -1;
@@ -953,7 +957,8 @@ static int get_sheep_fd(BDRVSheepdogState *s, Error **errp)
return fd;
}
- aio_set_fd_handler(s->aio_context, fd, co_read_response, NULL, s);
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ co_read_response, NULL, s);
return fd;
}
@@ -1208,7 +1213,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
qemu_co_mutex_lock(&s->lock);
s->co_send = qemu_coroutine_self();
- aio_set_fd_handler(s->aio_context, s->fd,
+ aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_UNSPECIFIED,
co_read_response, co_write_request, s);
socket_set_cork(s->fd, 1);
@@ -1227,7 +1232,8 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
}
out:
socket_set_cork(s->fd, 0);
- aio_set_fd_handler(s->aio_context, s->fd, co_read_response, NULL, s);
+ aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_UNSPECIFIED,
+ co_read_response, NULL, s);
s->co_send = NULL;
qemu_co_mutex_unlock(&s->lock);
}
@@ -1405,7 +1411,8 @@ static void sd_detach_aio_context(BlockDriverState *bs)
{
BDRVSheepdogState *s = bs->opaque;
- aio_set_fd_handler(s->aio_context, s->fd, NULL, NULL, NULL);
+ aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_UNSPECIFIED, NULL,
+ NULL, NULL);
}
static void sd_attach_aio_context(BlockDriverState *bs,
@@ -1414,7 +1421,8 @@ static void sd_attach_aio_context(BlockDriverState *bs,
BDRVSheepdogState *s = bs->opaque;
s->aio_context = new_context;
- aio_set_fd_handler(new_context, s->fd, co_read_response, NULL, s);
+ aio_set_fd_handler(new_context, s->fd, AIO_CLIENT_UNSPECIFIED,
+ co_read_response, NULL, s);
}
/* TODO Convert to fine grained options */
@@ -1528,7 +1536,8 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
g_free(buf);
return 0;
out:
- aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd, NULL, NULL, NULL);
+ aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd,
+ AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
if (s->fd >= 0) {
closesocket(s->fd);
}
@@ -1912,7 +1921,8 @@ static void sd_close(BlockDriverState *bs)
error_report("%s, %s", sd_strerror(rsp->result), s->name);
}
- aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd, NULL, NULL, NULL);
+ aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd,
+ AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
closesocket(s->fd);
g_free(s->host_spec);
}
diff --git a/block/ssh.c b/block/ssh.c
index aebb18c..71d7ffe 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -803,14 +803,15 @@ static coroutine_fn void set_fd_handler(BDRVSSHState *s, BlockDriverState *bs)
rd_handler, wr_handler);
aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock,
- rd_handler, wr_handler, co);
+ AIO_CLIENT_UNSPECIFIED, rd_handler, wr_handler, co);
}
static coroutine_fn void clear_fd_handler(BDRVSSHState *s,
BlockDriverState *bs)
{
DPRINTF("s->sock=%d", s->sock);
- aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock, NULL, NULL, NULL);
+ aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock,
+ AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
}
/* A non-blocking call returned EAGAIN, so yield, ensuring the
diff --git a/block/win32-aio.c b/block/win32-aio.c
index 64e8682..0081886 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -174,7 +174,7 @@ int win32_aio_attach(QEMUWin32AIOState *aio, HANDLE hfile)
void win32_aio_detach_aio_context(QEMUWin32AIOState *aio,
AioContext *old_context)
{
- aio_set_event_notifier(old_context, &aio->e, NULL);
+ aio_set_event_notifier(old_context, &aio->e, AIO_CLIENT_UNSPECIFIED, NULL);
aio->is_aio_context_attached = false;
}
@@ -182,7 +182,8 @@ void win32_aio_attach_aio_context(QEMUWin32AIOState *aio,
AioContext *new_context)
{
aio->is_aio_context_attached = true;
- aio_set_event_notifier(new_context, &aio->e, win32_aio_completion_cb);
+ aio_set_event_notifier(new_context, &aio->e, AIO_CLIENT_UNSPECIFIED,
+ win32_aio_completion_cb);
}
QEMUWin32AIOState *win32_aio_init(void)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 3db139b..e472154 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -283,7 +283,8 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
/* Get this show started by hooking up our callbacks */
aio_context_acquire(s->ctx);
- aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify);
+ aio_set_event_notifier(s->ctx, &s->host_notifier, AIO_CLIENT_UNSPECIFIED,
+ handle_notify);
aio_context_release(s->ctx);
return;
@@ -319,7 +320,8 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
aio_context_acquire(s->ctx);
/* Stop notifications for new requests from guest */
- aio_set_event_notifier(s->ctx, &s->host_notifier, NULL);
+ aio_set_event_notifier(s->ctx, &s->host_notifier, AIO_CLIENT_UNSPECIFIED,
+ NULL);
/* Drain and switch bs back to the QEMU main loop */
blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 5575648..f7bab09 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -60,7 +60,8 @@ static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSI *s,
r = g_slice_new(VirtIOSCSIVring);
r->host_notifier = *virtio_queue_get_host_notifier(vq);
r->guest_notifier = *virtio_queue_get_guest_notifier(vq);
- aio_set_event_notifier(s->ctx, &r->host_notifier, handler);
+ aio_set_event_notifier(s->ctx, &r->host_notifier, AIO_CLIENT_UNSPECIFIED,
+ handler);
r->parent = s;
@@ -71,7 +72,8 @@ static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSI *s,
return r;
fail_vring:
- aio_set_event_notifier(s->ctx, &r->host_notifier, NULL);
+ aio_set_event_notifier(s->ctx, &r->host_notifier, AIO_CLIENT_UNSPECIFIED,
+ NULL);
k->set_host_notifier(qbus->parent, n, false);
g_slice_free(VirtIOSCSIVring, r);
return NULL;
@@ -162,14 +164,17 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s)
int i;
if (s->ctrl_vring) {
- aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier, NULL);
+ aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier,
+ AIO_CLIENT_UNSPECIFIED, NULL);
}
if (s->event_vring) {
- aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier, NULL);
+ aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier,
+ AIO_CLIENT_UNSPECIFIED, NULL);
}
if (s->cmd_vrings) {
for (i = 0; i < vs->conf.num_queues && s->cmd_vrings[i]; i++) {
- aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier, NULL);
+ aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier,
+ AIO_CLIENT_UNSPECIFIED, NULL);
}
}
}
@@ -290,10 +295,13 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
aio_context_acquire(s->ctx);
- aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier, NULL);
- aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier, NULL);
+ aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier,
+ AIO_CLIENT_UNSPECIFIED, NULL);
+ aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier,
+ AIO_CLIENT_UNSPECIFIED, NULL);
for (i = 0; i < vs->conf.num_queues; i++) {
- aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier, NULL);
+ aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier,
+ AIO_CLIENT_UNSPECIFIED, NULL);
}
blk_drain_all(); /* ensure there are no in-flight requests */
diff --git a/include/block/aio.h b/include/block/aio.h
index 9dd32e0..bd1d44b 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -272,6 +272,9 @@ bool aio_pending(AioContext *ctx);
*/
bool aio_dispatch(AioContext *ctx);
+#define AIO_CLIENT_UNSPECIFIED (1 << 0)
+#define AIO_CLIENT_MASK_ALL -1
+
/* Progress in completing AIO work to occur. This can issue new pending
* aio as a result of executing I/O completion or bh callbacks.
*
@@ -296,6 +299,7 @@ bool aio_poll(AioContext *ctx, bool blocking);
*/
void aio_set_fd_handler(AioContext *ctx,
int fd,
+ int type,
IOHandler *io_read,
IOHandler *io_write,
void *opaque);
@@ -309,6 +313,7 @@ void aio_set_fd_handler(AioContext *ctx,
*/
void aio_set_event_notifier(AioContext *ctx,
EventNotifier *notifier,
+ int type,
EventNotifierHandler *io_read);
/* Return a GSource that lets the main loop poll the file descriptors attached
diff --git a/nbd.c b/nbd.c
index 06b501b..64ed91b 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1437,6 +1437,7 @@ static void nbd_set_handlers(NBDClient *client)
{
if (client->exp && client->exp->ctx) {
aio_set_fd_handler(client->exp->ctx, client->sock,
+ AIO_CLIENT_UNSPECIFIED,
client->can_read ? nbd_read : NULL,
client->send_coroutine ? nbd_restart_write : NULL,
client);
@@ -1446,7 +1447,8 @@ static void nbd_set_handlers(NBDClient *client)
static void nbd_unset_handlers(NBDClient *client)
{
if (client->exp && client->exp->ctx) {
- aio_set_fd_handler(client->exp->ctx, client->sock, NULL, NULL, NULL);
+ aio_set_fd_handler(client->exp->ctx, client->sock,
+ AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
}
}
diff --git a/tests/test-aio.c b/tests/test-aio.c
index 217e337..770119f 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -118,6 +118,12 @@ static void *test_acquire_thread(void *opaque)
return NULL;
}
+static void set_event_notifier(AioContext *ctx, EventNotifier *notifier,
+ EventNotifierHandler *handler)
+{
+ aio_set_event_notifier(ctx, notifier, AIO_CLIENT_UNSPECIFIED, handler);
+}
+
static void dummy_notifier_read(EventNotifier *unused)
{
g_assert(false); /* should never be invoked */
@@ -131,7 +137,7 @@ static void test_acquire(void)
/* Dummy event notifier ensures aio_poll() will block */
event_notifier_init(¬ifier, false);
- aio_set_event_notifier(ctx, ¬ifier, dummy_notifier_read);
+ set_event_notifier(ctx, ¬ifier, dummy_notifier_read);
g_assert(!aio_poll(ctx, false)); /* consume aio_notify() */
qemu_mutex_init(&data.start_lock);
@@ -149,7 +155,7 @@ static void test_acquire(void)
aio_context_release(ctx);
qemu_thread_join(&thread);
- aio_set_event_notifier(ctx, ¬ifier, NULL);
+ set_event_notifier(ctx, ¬ifier, NULL);
event_notifier_cleanup(¬ifier);
g_assert(data.thread_acquired);
@@ -308,11 +314,11 @@ static void test_set_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 0 };
event_notifier_init(&data.e, false);
- aio_set_event_notifier(ctx, &data.e, event_ready_cb);
+ set_event_notifier(ctx, &data.e, event_ready_cb);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
- aio_set_event_notifier(ctx, &data.e, NULL);
+ set_event_notifier(ctx, &data.e, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
event_notifier_cleanup(&data.e);
@@ -322,7 +328,7 @@ static void test_wait_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 1 };
event_notifier_init(&data.e, false);
- aio_set_event_notifier(ctx, &data.e, event_ready_cb);
+ set_event_notifier(ctx, &data.e, event_ready_cb);
while (aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 1);
@@ -336,7 +342,7 @@ static void test_wait_event_notifier(void)
g_assert_cmpint(data.n, ==, 1);
g_assert_cmpint(data.active, ==, 0);
- aio_set_event_notifier(ctx, &data.e, NULL);
+ set_event_notifier(ctx, &data.e, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 1);
@@ -347,7 +353,7 @@ static void test_flush_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 10, .auto_set = true };
event_notifier_init(&data.e, false);
- aio_set_event_notifier(ctx, &data.e, event_ready_cb);
+ set_event_notifier(ctx, &data.e, event_ready_cb);
while (aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 10);
@@ -363,7 +369,7 @@ static void test_flush_event_notifier(void)
g_assert_cmpint(data.active, ==, 0);
g_assert(!aio_poll(ctx, false));
- aio_set_event_notifier(ctx, &data.e, NULL);
+ set_event_notifier(ctx, &data.e, NULL);
g_assert(!aio_poll(ctx, false));
event_notifier_cleanup(&data.e);
}
@@ -374,7 +380,7 @@ static void test_wait_event_notifier_noflush(void)
EventNotifierTestData dummy = { .n = 0, .active = 1 };
event_notifier_init(&data.e, false);
- aio_set_event_notifier(ctx, &data.e, event_ready_cb);
+ set_event_notifier(ctx, &data.e, event_ready_cb);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
@@ -387,7 +393,7 @@ static void test_wait_event_notifier_noflush(void)
/* An active event notifier forces aio_poll to look at EventNotifiers. */
event_notifier_init(&dummy.e, false);
- aio_set_event_notifier(ctx, &dummy.e, event_ready_cb);
+ set_event_notifier(ctx, &dummy.e, event_ready_cb);
event_notifier_set(&data.e);
g_assert(aio_poll(ctx, false));
@@ -407,10 +413,10 @@ static void test_wait_event_notifier_noflush(void)
g_assert_cmpint(dummy.n, ==, 1);
g_assert_cmpint(dummy.active, ==, 0);
- aio_set_event_notifier(ctx, &dummy.e, NULL);
+ set_event_notifier(ctx, &dummy.e, NULL);
event_notifier_cleanup(&dummy.e);
- aio_set_event_notifier(ctx, &data.e, NULL);
+ set_event_notifier(ctx, &data.e, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 2);
@@ -428,7 +434,7 @@ static void test_timer_schedule(void)
* an fd to wait on. Fixing this breaks other tests. So create a dummy one.
*/
event_notifier_init(&e, false);
- aio_set_event_notifier(ctx, &e, dummy_io_handler_read);
+ set_event_notifier(ctx, &e, dummy_io_handler_read);
aio_poll(ctx, false);
aio_timer_init(ctx, &data.timer, data.clock_type,
@@ -467,7 +473,7 @@ static void test_timer_schedule(void)
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 2);
- aio_set_event_notifier(ctx, &e, NULL);
+ set_event_notifier(ctx, &e, NULL);
event_notifier_cleanup(&e);
timer_del(&data.timer);
@@ -638,11 +644,11 @@ static void test_source_set_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 0 };
event_notifier_init(&data.e, false);
- aio_set_event_notifier(ctx, &data.e, event_ready_cb);
+ set_event_notifier(ctx, &data.e, event_ready_cb);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
- aio_set_event_notifier(ctx, &data.e, NULL);
+ set_event_notifier(ctx, &data.e, NULL);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
event_notifier_cleanup(&data.e);
@@ -652,7 +658,7 @@ static void test_source_wait_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 1 };
event_notifier_init(&data.e, false);
- aio_set_event_notifier(ctx, &data.e, event_ready_cb);
+ set_event_notifier(ctx, &data.e, event_ready_cb);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 1);
@@ -666,7 +672,7 @@ static void test_source_wait_event_notifier(void)
g_assert_cmpint(data.n, ==, 1);
g_assert_cmpint(data.active, ==, 0);
- aio_set_event_notifier(ctx, &data.e, NULL);
+ set_event_notifier(ctx, &data.e, NULL);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 1);
@@ -677,7 +683,7 @@ static void test_source_flush_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 10, .auto_set = true };
event_notifier_init(&data.e, false);
- aio_set_event_notifier(ctx, &data.e, event_ready_cb);
+ set_event_notifier(ctx, &data.e, event_ready_cb);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 10);
@@ -693,7 +699,7 @@ static void test_source_flush_event_notifier(void)
g_assert_cmpint(data.active, ==, 0);
g_assert(!g_main_context_iteration(NULL, false));
- aio_set_event_notifier(ctx, &data.e, NULL);
+ set_event_notifier(ctx, &data.e, NULL);
while (g_main_context_iteration(NULL, false));
event_notifier_cleanup(&data.e);
}
@@ -704,7 +710,7 @@ static void test_source_wait_event_notifier_noflush(void)
EventNotifierTestData dummy = { .n = 0, .active = 1 };
event_notifier_init(&data.e, false);
- aio_set_event_notifier(ctx, &data.e, event_ready_cb);
+ set_event_notifier(ctx, &data.e, event_ready_cb);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
@@ -717,7 +723,7 @@ static void test_source_wait_event_notifier_noflush(void)
/* An active event notifier forces aio_poll to look at EventNotifiers. */
event_notifier_init(&dummy.e, false);
- aio_set_event_notifier(ctx, &dummy.e, event_ready_cb);
+ set_event_notifier(ctx, &dummy.e, event_ready_cb);
event_notifier_set(&data.e);
g_assert(g_main_context_iteration(NULL, false));
@@ -737,10 +743,10 @@ static void test_source_wait_event_notifier_noflush(void)
g_assert_cmpint(dummy.n, ==, 1);
g_assert_cmpint(dummy.active, ==, 0);
- aio_set_event_notifier(ctx, &dummy.e, NULL);
+ set_event_notifier(ctx, &dummy.e, NULL);
event_notifier_cleanup(&dummy.e);
- aio_set_event_notifier(ctx, &data.e, NULL);
+ set_event_notifier(ctx, &data.e, NULL);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 2);
@@ -759,7 +765,7 @@ static void test_source_timer_schedule(void)
* an fd to wait on. Fixing this breaks other tests. So create a dummy one.
*/
event_notifier_init(&e, false);
- aio_set_event_notifier(ctx, &e, dummy_io_handler_read);
+ set_event_notifier(ctx, &e, dummy_io_handler_read);
do {} while (g_main_context_iteration(NULL, false));
aio_timer_init(ctx, &data.timer, data.clock_type,
@@ -784,7 +790,7 @@ static void test_source_timer_schedule(void)
g_assert_cmpint(data.n, ==, 2);
g_assert(qemu_clock_get_ns(data.clock_type) > expiry);
- aio_set_event_notifier(ctx, &e, NULL);
+ set_event_notifier(ctx, &e, NULL);
event_notifier_cleanup(&e);
timer_del(&data.timer);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 02/11] aio: Save type to AioHandler
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 01/11] aio: Introduce "type" in aio_set_fd_handler and aio_set_event_notifier Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 03/11] aio-posix: Introduce aio_poll_clients Fam Zheng
` (9 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
So it can be used by aio_poll later.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
aio-posix.c | 2 ++
aio-win32.c | 3 +++
2 files changed, 5 insertions(+)
diff --git a/aio-posix.c b/aio-posix.c
index 56f2bce..d25fcfc 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -25,6 +25,7 @@ struct AioHandler
IOHandler *io_write;
int deleted;
void *opaque;
+ int type;
QLIST_ENTRY(AioHandler) node;
};
@@ -83,6 +84,7 @@ void aio_set_fd_handler(AioContext *ctx,
node->io_read = io_read;
node->io_write = io_write;
node->opaque = opaque;
+ node->type = type;
node->pfd.events = (io_read ? G_IO_IN | G_IO_HUP | G_IO_ERR : 0);
node->pfd.events |= (io_write ? G_IO_OUT | G_IO_ERR : 0);
diff --git a/aio-win32.c b/aio-win32.c
index 90e7a4b..f5ecf57 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -28,6 +28,7 @@ struct AioHandler {
GPollFD pfd;
int deleted;
void *opaque;
+ int type;
QLIST_ENTRY(AioHandler) node;
};
@@ -87,6 +88,7 @@ void aio_set_fd_handler(AioContext *ctx,
node->opaque = opaque;
node->io_read = io_read;
node->io_write = io_write;
+ node->type = type;
event = event_notifier_get_handle(&ctx->notifier);
WSAEventSelect(node->pfd.fd, event,
@@ -135,6 +137,7 @@ void aio_set_event_notifier(AioContext *ctx,
node->e = e;
node->pfd.fd = (uintptr_t)event_notifier_get_handle(e);
node->pfd.events = G_IO_IN;
+ node->type = type;
QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node);
g_source_add_poll(&ctx->source, &node->pfd);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 03/11] aio-posix: Introduce aio_poll_clients
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 01/11] aio: Introduce "type" in aio_set_fd_handler and aio_set_event_notifier Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 02/11] aio: Save type to AioHandler Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 04/11] aio-win32: Implement aio_poll_clients Fam Zheng
` (8 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
This new API will allow limiting the scope of polled fds. The parameter
client_mask is a bit mask of the polled client types.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
aio-posix.c | 19 ++++++++++++++-----
include/block/aio.h | 11 ++++++++++-
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index d25fcfc..fca905f 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -126,7 +126,7 @@ bool aio_pending(AioContext *ctx)
return false;
}
-bool aio_dispatch(AioContext *ctx)
+static bool aio_dispatch_clients(AioContext *ctx, int client_mask)
{
AioHandler *node;
bool progress = false;
@@ -148,13 +148,14 @@ bool aio_dispatch(AioContext *ctx)
while (node) {
AioHandler *tmp;
int revents;
+ int dispatch = (node->type & client_mask) == node->type;
ctx->walking_handlers++;
revents = node->pfd.revents & node->pfd.events;
node->pfd.revents = 0;
- if (!node->deleted &&
+ if (dispatch && !node->deleted &&
(revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) &&
node->io_read) {
node->io_read(node->opaque);
@@ -164,7 +165,7 @@ bool aio_dispatch(AioContext *ctx)
progress = true;
}
}
- if (!node->deleted &&
+ if (dispatch && !node->deleted &&
(revents & (G_IO_OUT | G_IO_ERR)) &&
node->io_write) {
node->io_write(node->opaque);
@@ -188,6 +189,11 @@ bool aio_dispatch(AioContext *ctx)
return progress;
}
+bool aio_dispatch(AioContext *ctx)
+{
+ return aio_dispatch_clients(ctx, AIO_CLIENT_MASK_ALL);
+}
+
/* These thread-local variables are used only in a small part of aio_poll
* around the call to the poll() system call. In particular they are not
* used while aio_poll is performing callbacks, which makes it much easier
@@ -234,7 +240,7 @@ static void add_pollfd(AioHandler *node)
npfd++;
}
-bool aio_poll(AioContext *ctx, bool blocking)
+bool aio_poll_clients(AioContext *ctx, bool blocking, int client_mask)
{
AioHandler *node;
int i, ret;
@@ -261,6 +267,9 @@ bool aio_poll(AioContext *ctx, bool blocking)
/* fill pollfds */
QLIST_FOREACH(node, &ctx->aio_handlers, node) {
+ if ((node->type & client_mask) != node->type) {
+ continue;
+ }
if (!node->deleted && node->pfd.events) {
add_pollfd(node);
}
@@ -293,7 +302,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
ctx->walking_handlers--;
/* Run dispatch even if there were no readable fds to run timers */
- if (aio_dispatch(ctx)) {
+ if (aio_dispatch_clients(ctx, client_mask)) {
progress = true;
}
diff --git a/include/block/aio.h b/include/block/aio.h
index bd1d44b..7a1c63e 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -282,13 +282,22 @@ bool aio_dispatch(AioContext *ctx);
* handlers. If @blocking == true, this should always be true except
* if someone called aio_notify.
*
+ * client_mask is a bit mask for AIO_CLIENT types, otherwise only the types
+ * corresponding to the set bits will be polled.
+ *
* If there are no pending bottom halves, but there are pending AIO
* operations, it may not be possible to make any progress without
* blocking. If @blocking is true, this function will wait until one
* or more AIO events have completed, to ensure something has moved
* before returning.
*/
-bool aio_poll(AioContext *ctx, bool blocking);
+bool aio_poll_clients(AioContext *ctx, bool blocking, int client_mask);
+
+/* Poll all types of clients. */
+static inline bool aio_poll(AioContext *ctx, bool blocking)
+{
+ return aio_poll_clients(ctx, blocking, AIO_CLIENT_MASK_ALL);
+}
/* Register a file descriptor and associated callbacks. Behaves very similarly
* to qemu_set_fd_handler. Unlike qemu_set_fd_handler, these callbacks will
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 04/11] aio-win32: Implement aio_poll_clients
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (2 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 03/11] aio-posix: Introduce aio_poll_clients Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 05/11] block: Mark fd handlers as "protocol" Fam Zheng
` (7 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
This is the counterpart of for windows.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
aio-win32.c | 34 ++++++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/aio-win32.c b/aio-win32.c
index f5ecf57..c925085 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -149,7 +149,7 @@ void aio_set_event_notifier(AioContext *ctx,
aio_notify(ctx);
}
-bool aio_prepare(AioContext *ctx)
+static bool aio_prepare_clients(AioContext *ctx, int client_mask)
{
static struct timeval tv0;
AioHandler *node;
@@ -160,6 +160,9 @@ bool aio_prepare(AioContext *ctx)
FD_ZERO(&rfds);
FD_ZERO(&wfds);
QLIST_FOREACH(node, &ctx->aio_handlers, node) {
+ if ((node->type & client_mask) != node->type) {
+ continue;
+ }
if (node->io_read) {
FD_SET ((SOCKET)node->pfd.fd, &rfds);
}
@@ -170,6 +173,9 @@ bool aio_prepare(AioContext *ctx)
if (select(0, &rfds, &wfds, NULL, &tv0) > 0) {
QLIST_FOREACH(node, &ctx->aio_handlers, node) {
+ if ((node->type & client_mask) != node->type) {
+ continue;
+ }
node->pfd.revents = 0;
if (FD_ISSET(node->pfd.fd, &rfds)) {
node->pfd.revents |= G_IO_IN;
@@ -186,6 +192,11 @@ bool aio_prepare(AioContext *ctx)
return have_select_revents;
}
+bool aio_prepare(AioContext *ctx)
+{
+ return aio_prepare_clients(ctx, AIO_CLIENT_MASK_ALL);
+}
+
bool aio_pending(AioContext *ctx)
{
AioHandler *node;
@@ -206,7 +217,8 @@ bool aio_pending(AioContext *ctx)
return false;
}
-static bool aio_dispatch_handlers(AioContext *ctx, HANDLE event)
+static bool aio_dispatch_handlers(AioContext *ctx, HANDLE event,
+ int client_mask)
{
AioHandler *node;
bool progress = false;
@@ -219,10 +231,11 @@ static bool aio_dispatch_handlers(AioContext *ctx, HANDLE event)
while (node) {
AioHandler *tmp;
int revents = node->pfd.revents;
+ bool dispatch = (node->type & client_mask) == node->type;
ctx->walking_handlers++;
- if (!node->deleted &&
+ if (dispatch && !node->deleted &&
(revents || event_notifier_get_handle(node->e) == event) &&
node->io_notify) {
node->pfd.revents = 0;
@@ -234,7 +247,7 @@ static bool aio_dispatch_handlers(AioContext *ctx, HANDLE event)
}
}
- if (!node->deleted &&
+ if (dispatch && !node->deleted &&
(node->io_read || node->io_write)) {
node->pfd.revents = 0;
if ((revents & G_IO_IN) && node->io_read) {
@@ -256,6 +269,7 @@ static bool aio_dispatch_handlers(AioContext *ctx, HANDLE event)
}
}
+next:
tmp = node;
node = QLIST_NEXT(node, node);
@@ -275,12 +289,13 @@ bool aio_dispatch(AioContext *ctx)
bool progress;
progress = aio_bh_poll(ctx);
- progress |= aio_dispatch_handlers(ctx, INVALID_HANDLE_VALUE);
+ progress |= aio_dispatch_handlers(ctx, INVALID_HANDLE_VALUE,
+ AIO_CLIENT_MASK_ALL);
progress |= timerlistgroup_run_timers(&ctx->tlg);
return progress;
}
-bool aio_poll(AioContext *ctx, bool blocking)
+bool aio_poll_clients(AioContext *ctx, bool blocking, int client_mask)
{
AioHandler *node;
HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
@@ -302,13 +317,16 @@ bool aio_poll(AioContext *ctx, bool blocking)
atomic_add(&ctx->notify_me, 2);
}
- have_select_revents = aio_prepare(ctx);
+ have_select_revents = aio_prepare_clients(ctx, client_mask);
ctx->walking_handlers++;
/* fill fd sets */
count = 0;
QLIST_FOREACH(node, &ctx->aio_handlers, node) {
+ if ((node->type & client_mask) != node->type) {
+ continue;
+ }
if (!node->deleted && node->io_notify) {
events[count++] = event_notifier_get_handle(node->e);
}
@@ -360,7 +378,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
have_select_revents = false;
blocking = false;
- progress |= aio_dispatch_handlers(ctx, event);
+ progress |= aio_dispatch_handlers(ctx, event, client_mask);
} while (count > 0);
progress |= timerlistgroup_run_timers(&ctx->tlg);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 05/11] block: Mark fd handlers as "protocol"
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (3 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 04/11] aio-win32: Implement aio_poll_clients Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 06/11] nbd: Mark fd handlers client type as "nbd server" Fam Zheng
` (6 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
The "protocol" type includes all the fd handlers and event notifiers used by
block layer, especially those that should be polled in a nested event loop.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/curl.c | 8 ++++----
block/iscsi.c | 4 ++--
block/linux-aio.c | 4 ++--
block/nbd-client.c | 8 ++++----
block/nfs.c | 6 +++---
block/sheepdog.c | 22 +++++++++++-----------
block/ssh.c | 4 ++--
block/win32-aio.c | 4 ++--
include/block/aio.h | 1 +
9 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index 6925672..75d237c 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -154,19 +154,19 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, fd);
switch (action) {
case CURL_POLL_IN:
- aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_PROTOCOL,
curl_multi_read, NULL, state);
break;
case CURL_POLL_OUT:
- aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_PROTOCOL,
NULL, curl_multi_do, state);
break;
case CURL_POLL_INOUT:
- aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_PROTOCOL,
curl_multi_read, curl_multi_do, state);
break;
case CURL_POLL_REMOVE:
- aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_PROTOCOL,
NULL, NULL, NULL);
break;
}
diff --git a/block/iscsi.c b/block/iscsi.c
index 0ee1295..1713625 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -292,7 +292,7 @@ iscsi_set_events(IscsiLun *iscsilun)
if (ev != iscsilun->events) {
aio_set_fd_handler(iscsilun->aio_context, iscsi_get_fd(iscsi),
- AIO_CLIENT_UNSPECIFIED,
+ AIO_CLIENT_PROTOCOL,
(ev & POLLIN) ? iscsi_process_read : NULL,
(ev & POLLOUT) ? iscsi_process_write : NULL,
iscsilun);
@@ -1277,7 +1277,7 @@ static void iscsi_detach_aio_context(BlockDriverState *bs)
IscsiLun *iscsilun = bs->opaque;
aio_set_fd_handler(iscsilun->aio_context, iscsi_get_fd(iscsilun->iscsi),
- AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
+ AIO_CLIENT_PROTOCOL, NULL, NULL, NULL);
iscsilun->events = 0;
if (iscsilun->nop_timer) {
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 0921bde..1491684 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -287,7 +287,7 @@ void laio_detach_aio_context(void *s_, AioContext *old_context)
{
struct qemu_laio_state *s = s_;
- aio_set_event_notifier(old_context, &s->e, AIO_CLIENT_UNSPECIFIED, NULL);
+ aio_set_event_notifier(old_context, &s->e, AIO_CLIENT_PROTOCOL, NULL);
qemu_bh_delete(s->completion_bh);
}
@@ -296,7 +296,7 @@ void laio_attach_aio_context(void *s_, AioContext *new_context)
struct qemu_laio_state *s = s_;
s->completion_bh = aio_bh_new(new_context, qemu_laio_completion_bh, s);
- aio_set_event_notifier(new_context, &s->e, AIO_CLIENT_UNSPECIFIED,
+ aio_set_event_notifier(new_context, &s->e, AIO_CLIENT_PROTOCOL,
qemu_laio_completion_cb);
}
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 36c46c5..edf2199 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -124,7 +124,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
s->send_coroutine = qemu_coroutine_self();
aio_context = bdrv_get_aio_context(bs);
- aio_set_fd_handler(aio_context, s->sock, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(aio_context, s->sock, AIO_CLIENT_PROTOCOL,
nbd_reply_ready, nbd_restart_write, bs);
if (qiov) {
if (!s->is_unix) {
@@ -144,7 +144,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
} else {
rc = nbd_send_request(s->sock, request);
}
- aio_set_fd_handler(aio_context, s->sock, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(aio_context, s->sock, AIO_CLIENT_PROTOCOL,
nbd_reply_ready, NULL, bs);
s->send_coroutine = NULL;
qemu_co_mutex_unlock(&s->send_mutex);
@@ -350,14 +350,14 @@ void nbd_client_detach_aio_context(BlockDriverState *bs)
{
aio_set_fd_handler(bdrv_get_aio_context(bs),
nbd_get_client_session(bs)->sock,
- AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
+ AIO_CLIENT_PROTOCOL, NULL, NULL, NULL);
}
void nbd_client_attach_aio_context(BlockDriverState *bs,
AioContext *new_context)
{
aio_set_fd_handler(new_context, nbd_get_client_session(bs)->sock,
- AIO_CLIENT_UNSPECIFIED, nbd_reply_ready, NULL, bs);
+ AIO_CLIENT_PROTOCOL, nbd_reply_ready, NULL, bs);
}
void nbd_client_close(BlockDriverState *bs)
diff --git a/block/nfs.c b/block/nfs.c
index a21dd6f..4d12067 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -63,7 +63,7 @@ static void nfs_set_events(NFSClient *client)
int ev = nfs_which_events(client->context);
if (ev != client->events) {
aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
- AIO_CLIENT_UNSPECIFIED,
+ AIO_CLIENT_PROTOCOL,
(ev & POLLIN) ? nfs_process_read : NULL,
(ev & POLLOUT) ? nfs_process_write : NULL, client);
@@ -241,7 +241,7 @@ static void nfs_detach_aio_context(BlockDriverState *bs)
NFSClient *client = bs->opaque;
aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
- AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
+ AIO_CLIENT_PROTOCOL, NULL, NULL, NULL);
client->events = 0;
}
@@ -261,7 +261,7 @@ static void nfs_client_close(NFSClient *client)
nfs_close(client->context, client->fh);
}
aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
- AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
+ AIO_CLIENT_PROTOCOL, NULL, NULL, NULL);
nfs_destroy_context(client->context);
}
memset(client, 0, sizeof(NFSClient));
diff --git a/block/sheepdog.c b/block/sheepdog.c
index e0552b7..de9f8be 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -624,7 +624,7 @@ static coroutine_fn void do_co_req(void *opaque)
unsigned int *rlen = srco->rlen;
co = qemu_coroutine_self();
- aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_PROTOCOL,
NULL, restart_co_req, co);
ret = send_co_req(sockfd, hdr, data, wlen);
@@ -632,7 +632,7 @@ static coroutine_fn void do_co_req(void *opaque)
goto out;
}
- aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_PROTOCOL,
restart_co_req, NULL, co);
ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
@@ -658,7 +658,7 @@ static coroutine_fn void do_co_req(void *opaque)
out:
/* there is at most one request for this sockfd, so it is safe to
* set each handler to NULL. */
- aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(srco->aio_context, sockfd, AIO_CLIENT_PROTOCOL,
NULL, NULL, NULL);
srco->ret = ret;
@@ -743,7 +743,7 @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
BDRVSheepdogState *s = opaque;
AIOReq *aio_req, *next;
- aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_UNSPECIFIED, NULL,
+ aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_PROTOCOL, NULL,
NULL, NULL);
close(s->fd);
s->fd = -1;
@@ -957,7 +957,7 @@ static int get_sheep_fd(BDRVSheepdogState *s, Error **errp)
return fd;
}
- aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(s->aio_context, fd, AIO_CLIENT_PROTOCOL,
co_read_response, NULL, s);
return fd;
}
@@ -1213,7 +1213,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
qemu_co_mutex_lock(&s->lock);
s->co_send = qemu_coroutine_self();
- aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_PROTOCOL,
co_read_response, co_write_request, s);
socket_set_cork(s->fd, 1);
@@ -1232,7 +1232,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
}
out:
socket_set_cork(s->fd, 0);
- aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_PROTOCOL,
co_read_response, NULL, s);
s->co_send = NULL;
qemu_co_mutex_unlock(&s->lock);
@@ -1411,7 +1411,7 @@ static void sd_detach_aio_context(BlockDriverState *bs)
{
BDRVSheepdogState *s = bs->opaque;
- aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_UNSPECIFIED, NULL,
+ aio_set_fd_handler(s->aio_context, s->fd, AIO_CLIENT_PROTOCOL, NULL,
NULL, NULL);
}
@@ -1421,7 +1421,7 @@ static void sd_attach_aio_context(BlockDriverState *bs,
BDRVSheepdogState *s = bs->opaque;
s->aio_context = new_context;
- aio_set_fd_handler(new_context, s->fd, AIO_CLIENT_UNSPECIFIED,
+ aio_set_fd_handler(new_context, s->fd, AIO_CLIENT_PROTOCOL,
co_read_response, NULL, s);
}
@@ -1537,7 +1537,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
return 0;
out:
aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd,
- AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
+ AIO_CLIENT_PROTOCOL, NULL, NULL, NULL);
if (s->fd >= 0) {
closesocket(s->fd);
}
@@ -1922,7 +1922,7 @@ static void sd_close(BlockDriverState *bs)
}
aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd,
- AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
+ AIO_CLIENT_PROTOCOL, NULL, NULL, NULL);
closesocket(s->fd);
g_free(s->host_spec);
}
diff --git a/block/ssh.c b/block/ssh.c
index 71d7ffe..3e721d2 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -803,7 +803,7 @@ static coroutine_fn void set_fd_handler(BDRVSSHState *s, BlockDriverState *bs)
rd_handler, wr_handler);
aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock,
- AIO_CLIENT_UNSPECIFIED, rd_handler, wr_handler, co);
+ AIO_CLIENT_PROTOCOL, rd_handler, wr_handler, co);
}
static coroutine_fn void clear_fd_handler(BDRVSSHState *s,
@@ -811,7 +811,7 @@ static coroutine_fn void clear_fd_handler(BDRVSSHState *s,
{
DPRINTF("s->sock=%d", s->sock);
aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock,
- AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
+ AIO_CLIENT_PROTOCOL, NULL, NULL, NULL);
}
/* A non-blocking call returned EAGAIN, so yield, ensuring the
diff --git a/block/win32-aio.c b/block/win32-aio.c
index 0081886..57b1916 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -174,7 +174,7 @@ int win32_aio_attach(QEMUWin32AIOState *aio, HANDLE hfile)
void win32_aio_detach_aio_context(QEMUWin32AIOState *aio,
AioContext *old_context)
{
- aio_set_event_notifier(old_context, &aio->e, AIO_CLIENT_UNSPECIFIED, NULL);
+ aio_set_event_notifier(old_context, &aio->e, AIO_CLIENT_PROTOCOL, NULL);
aio->is_aio_context_attached = false;
}
@@ -182,7 +182,7 @@ void win32_aio_attach_aio_context(QEMUWin32AIOState *aio,
AioContext *new_context)
{
aio->is_aio_context_attached = true;
- aio_set_event_notifier(new_context, &aio->e, AIO_CLIENT_UNSPECIFIED,
+ aio_set_event_notifier(new_context, &aio->e, AIO_CLIENT_PROTOCOL,
win32_aio_completion_cb);
}
diff --git a/include/block/aio.h b/include/block/aio.h
index 7a1c63e..1895a74 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -273,6 +273,7 @@ bool aio_pending(AioContext *ctx);
bool aio_dispatch(AioContext *ctx);
#define AIO_CLIENT_UNSPECIFIED (1 << 0)
+#define AIO_CLIENT_PROTOCOL (1 << 1)
#define AIO_CLIENT_MASK_ALL -1
/* Progress in completing AIO work to occur. This can issue new pending
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 06/11] nbd: Mark fd handlers client type as "nbd server"
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (4 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 05/11] block: Mark fd handlers as "protocol" Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 07/11] aio: Mark ctx->notifier's client type as "context" Fam Zheng
` (5 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
So we could distinguish it from "protocol" to avoid handling in nested aio
polls.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
include/block/aio.h | 1 +
nbd.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/block/aio.h b/include/block/aio.h
index 1895a74..088f9ce 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -274,6 +274,7 @@ bool aio_dispatch(AioContext *ctx);
#define AIO_CLIENT_UNSPECIFIED (1 << 0)
#define AIO_CLIENT_PROTOCOL (1 << 1)
+#define AIO_CLIENT_NBD_SERVER (1 << 2)
#define AIO_CLIENT_MASK_ALL -1
/* Progress in completing AIO work to occur. This can issue new pending
diff --git a/nbd.c b/nbd.c
index 64ed91b..7b38334 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1437,7 +1437,7 @@ static void nbd_set_handlers(NBDClient *client)
{
if (client->exp && client->exp->ctx) {
aio_set_fd_handler(client->exp->ctx, client->sock,
- AIO_CLIENT_UNSPECIFIED,
+ AIO_CLIENT_NBD_SERVER,
client->can_read ? nbd_read : NULL,
client->send_coroutine ? nbd_restart_write : NULL,
client);
@@ -1448,7 +1448,7 @@ static void nbd_unset_handlers(NBDClient *client)
{
if (client->exp && client->exp->ctx) {
aio_set_fd_handler(client->exp->ctx, client->sock,
- AIO_CLIENT_UNSPECIFIED, NULL, NULL, NULL);
+ AIO_CLIENT_NBD_SERVER, NULL, NULL, NULL);
}
}
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 07/11] aio: Mark ctx->notifier's client type as "context"
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (5 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 06/11] nbd: Mark fd handlers client type as "nbd server" Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 08/11] dataplane: Mark host notifiers' client type as "dataplane" Fam Zheng
` (4 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
It is important to include this for any blocking poll, on the other hand it is
also OK to exclude it otherwise.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
aio-posix.c | 3 +++
aio-win32.c | 3 +++
async.c | 4 ++--
include/block/aio.h | 1 +
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index fca905f..4c0328b 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -247,6 +247,9 @@ bool aio_poll_clients(AioContext *ctx, bool blocking, int client_mask)
bool progress;
int64_t timeout;
+ /* Blocking poll must handle waking up. */
+ assert(!blocking || (client_mask & AIO_CLIENT_CONTEXT));
+
aio_context_acquire(ctx);
progress = false;
diff --git a/aio-win32.c b/aio-win32.c
index c925085..036eeac 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -303,6 +303,9 @@ bool aio_poll_clients(AioContext *ctx, bool blocking, int client_mask)
int count;
int timeout;
+ /* Blocking poll must handle waking up. */
+ assert(!blocking || (client_mask & AIO_CLIENT_CONTEXT));
+
aio_context_acquire(ctx);
progress = false;
diff --git a/async.c b/async.c
index 43f9425..0bea602 100644
--- a/async.c
+++ b/async.c
@@ -231,7 +231,7 @@ aio_ctx_finalize(GSource *source)
AioContext *ctx = (AioContext *) source;
thread_pool_free(ctx->thread_pool);
- aio_set_event_notifier(ctx, &ctx->notifier, AIO_CLIENT_UNSPECIFIED, NULL);
+ aio_set_event_notifier(ctx, &ctx->notifier, AIO_CLIENT_CONTEXT, NULL);
event_notifier_cleanup(&ctx->notifier);
rfifolock_destroy(&ctx->lock);
qemu_mutex_destroy(&ctx->bh_lock);
@@ -306,7 +306,7 @@ AioContext *aio_context_new(Error **errp)
}
g_source_set_can_recurse(&ctx->source, true);
aio_set_event_notifier(ctx, &ctx->notifier,
- AIO_CLIENT_UNSPECIFIED,
+ AIO_CLIENT_CONTEXT,
(EventNotifierHandler *)
event_notifier_dummy_cb);
ctx->thread_pool = NULL;
diff --git a/include/block/aio.h b/include/block/aio.h
index 088f9ce..4b53151 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -275,6 +275,7 @@ bool aio_dispatch(AioContext *ctx);
#define AIO_CLIENT_UNSPECIFIED (1 << 0)
#define AIO_CLIENT_PROTOCOL (1 << 1)
#define AIO_CLIENT_NBD_SERVER (1 << 2)
+#define AIO_CLIENT_CONTEXT (1 << 3)
#define AIO_CLIENT_MASK_ALL -1
/* Progress in completing AIO work to occur. This can issue new pending
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 08/11] dataplane: Mark host notifiers' client type as "dataplane"
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (6 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 07/11] aio: Mark ctx->notifier's client type as "context" Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 09/11] block: Introduce bdrv_aio_poll Fam Zheng
` (3 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/block/dataplane/virtio-blk.c | 4 ++--
hw/scsi/virtio-scsi-dataplane.c | 16 ++++++++--------
include/block/aio.h | 1 +
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index e472154..5419f1c 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -283,7 +283,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
/* Get this show started by hooking up our callbacks */
aio_context_acquire(s->ctx);
- aio_set_event_notifier(s->ctx, &s->host_notifier, AIO_CLIENT_UNSPECIFIED,
+ aio_set_event_notifier(s->ctx, &s->host_notifier, AIO_CLIENT_DATAPLANE,
handle_notify);
aio_context_release(s->ctx);
return;
@@ -320,7 +320,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
aio_context_acquire(s->ctx);
/* Stop notifications for new requests from guest */
- aio_set_event_notifier(s->ctx, &s->host_notifier, AIO_CLIENT_UNSPECIFIED,
+ aio_set_event_notifier(s->ctx, &s->host_notifier, AIO_CLIENT_DATAPLANE,
NULL);
/* Drain and switch bs back to the QEMU main loop */
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index f7bab09..55c2524 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -60,7 +60,7 @@ static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSI *s,
r = g_slice_new(VirtIOSCSIVring);
r->host_notifier = *virtio_queue_get_host_notifier(vq);
r->guest_notifier = *virtio_queue_get_guest_notifier(vq);
- aio_set_event_notifier(s->ctx, &r->host_notifier, AIO_CLIENT_UNSPECIFIED,
+ aio_set_event_notifier(s->ctx, &r->host_notifier, AIO_CLIENT_DATAPLANE,
handler);
r->parent = s;
@@ -72,7 +72,7 @@ static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSI *s,
return r;
fail_vring:
- aio_set_event_notifier(s->ctx, &r->host_notifier, AIO_CLIENT_UNSPECIFIED,
+ aio_set_event_notifier(s->ctx, &r->host_notifier, AIO_CLIENT_DATAPLANE,
NULL);
k->set_host_notifier(qbus->parent, n, false);
g_slice_free(VirtIOSCSIVring, r);
@@ -165,16 +165,16 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s)
if (s->ctrl_vring) {
aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier,
- AIO_CLIENT_UNSPECIFIED, NULL);
+ AIO_CLIENT_DATAPLANE, NULL);
}
if (s->event_vring) {
aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier,
- AIO_CLIENT_UNSPECIFIED, NULL);
+ AIO_CLIENT_DATAPLANE, NULL);
}
if (s->cmd_vrings) {
for (i = 0; i < vs->conf.num_queues && s->cmd_vrings[i]; i++) {
aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier,
- AIO_CLIENT_UNSPECIFIED, NULL);
+ AIO_CLIENT_DATAPLANE, NULL);
}
}
}
@@ -296,12 +296,12 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
aio_context_acquire(s->ctx);
aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier,
- AIO_CLIENT_UNSPECIFIED, NULL);
+ AIO_CLIENT_DATAPLANE, NULL);
aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier,
- AIO_CLIENT_UNSPECIFIED, NULL);
+ AIO_CLIENT_DATAPLANE, NULL);
for (i = 0; i < vs->conf.num_queues; i++) {
aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier,
- AIO_CLIENT_UNSPECIFIED, NULL);
+ AIO_CLIENT_DATAPLANE, NULL);
}
blk_drain_all(); /* ensure there are no in-flight requests */
diff --git a/include/block/aio.h b/include/block/aio.h
index 4b53151..cd9a210 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -276,6 +276,7 @@ bool aio_dispatch(AioContext *ctx);
#define AIO_CLIENT_PROTOCOL (1 << 1)
#define AIO_CLIENT_NBD_SERVER (1 << 2)
#define AIO_CLIENT_CONTEXT (1 << 3)
+#define AIO_CLIENT_DATAPLANE (1 << 4)
#define AIO_CLIENT_MASK_ALL -1
/* Progress in completing AIO work to occur. This can issue new pending
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 09/11] block: Introduce bdrv_aio_poll
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (7 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 08/11] dataplane: Mark host notifiers' client type as "dataplane" Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 10/11] block: Replace nested aio_poll with bdrv_aio_poll Fam Zheng
` (2 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
This call is introduced simply as a wrapper of aio_poll, but it makes it
is easy to change the polled client types.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/io.c | 5 +++++
include/block/block.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/block/io.c b/block/io.c
index d4bc83b..fbf9e0f 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2608,3 +2608,8 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
}
bdrv_start_throttled_reqs(bs);
}
+
+bool bdrv_aio_poll(AioContext *ctx, bool blocking)
+{
+ return aio_poll(ctx, blocking);
+}
diff --git a/include/block/block.h b/include/block/block.h
index 37916f7..be99e6d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -616,4 +616,6 @@ void bdrv_flush_io_queue(BlockDriverState *bs);
BlockAcctStats *bdrv_get_stats(BlockDriverState *bs);
+bool bdrv_aio_poll(AioContext *ctx, bool blocking);
+
#endif
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 10/11] block: Replace nested aio_poll with bdrv_aio_poll
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (8 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 09/11] block: Introduce bdrv_aio_poll Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 11/11] block: Only poll block layer fds in bdrv_aio_poll Fam Zheng
2015-07-23 8:15 ` [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Paolo Bonzini
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
Just a manual search and replace. No semantic change here.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 2 +-
block/curl.c | 2 +-
block/io.c | 18 +++++++++---------
block/nfs.c | 2 +-
block/qed-table.c | 8 ++++----
blockjob.c | 2 +-
qemu-img.c | 2 +-
qemu-io-cmds.c | 4 ++--
8 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/block.c b/block.c
index d088ee0..564c43b 100644
--- a/block.c
+++ b/block.c
@@ -371,7 +371,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
co = qemu_coroutine_create(bdrv_create_co_entry);
qemu_coroutine_enter(co, &cco);
while (cco.ret == NOT_DONE) {
- aio_poll(qemu_get_aio_context(), true);
+ bdrv_aio_poll(qemu_get_aio_context(), true);
}
}
diff --git a/block/curl.c b/block/curl.c
index 75d237c..2223091 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -392,7 +392,7 @@ static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s)
break;
}
if (!state) {
- aio_poll(bdrv_get_aio_context(bs), true);
+ bdrv_aio_poll(bdrv_get_aio_context(bs), true);
}
} while(!state);
diff --git a/block/io.c b/block/io.c
index fbf9e0f..5f5e575 100644
--- a/block/io.c
+++ b/block/io.c
@@ -251,7 +251,7 @@ void bdrv_drain(BlockDriverState *bs)
/* Keep iterating */
bdrv_flush_io_queue(bs);
busy = bdrv_requests_pending(bs);
- busy |= aio_poll(bdrv_get_aio_context(bs), busy);
+ busy |= bdrv_aio_poll(bdrv_get_aio_context(bs), busy);
}
}
@@ -301,11 +301,11 @@ void bdrv_drain_all(void)
bdrv_flush_io_queue(bs);
if (bdrv_requests_pending(bs)) {
busy = true;
- aio_poll(aio_context, busy);
+ bdrv_aio_poll(aio_context, busy);
}
}
}
- busy |= aio_poll(aio_context, false);
+ busy |= bdrv_aio_poll(aio_context, false);
aio_context_release(aio_context);
}
}
@@ -559,7 +559,7 @@ static int bdrv_prwv_co(BlockDriverState *bs, int64_t offset,
co = qemu_coroutine_create(bdrv_rw_co_entry);
qemu_coroutine_enter(co, &rwco);
while (rwco.ret == NOT_DONE) {
- aio_poll(aio_context, true);
+ bdrv_aio_poll(aio_context, true);
}
}
return rwco.ret;
@@ -1594,7 +1594,7 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry);
qemu_coroutine_enter(co, &data);
while (!data.done) {
- aio_poll(aio_context, true);
+ bdrv_aio_poll(aio_context, true);
}
}
return data.ret;
@@ -1962,9 +1962,9 @@ void bdrv_aio_cancel(BlockAIOCB *acb)
bdrv_aio_cancel_async(acb);
while (acb->refcnt > 1) {
if (acb->aiocb_info->get_aio_context) {
- aio_poll(acb->aiocb_info->get_aio_context(acb), true);
+ bdrv_aio_poll(acb->aiocb_info->get_aio_context(acb), true);
} else if (acb->bs) {
- aio_poll(bdrv_get_aio_context(acb->bs), true);
+ bdrv_aio_poll(bdrv_get_aio_context(acb->bs), true);
} else {
abort();
}
@@ -2376,7 +2376,7 @@ int bdrv_flush(BlockDriverState *bs)
co = qemu_coroutine_create(bdrv_flush_co_entry);
qemu_coroutine_enter(co, &rwco);
while (rwco.ret == NOT_DONE) {
- aio_poll(aio_context, true);
+ bdrv_aio_poll(aio_context, true);
}
}
@@ -2489,7 +2489,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
co = qemu_coroutine_create(bdrv_discard_co_entry);
qemu_coroutine_enter(co, &rwco);
while (rwco.ret == NOT_DONE) {
- aio_poll(aio_context, true);
+ bdrv_aio_poll(aio_context, true);
}
}
diff --git a/block/nfs.c b/block/nfs.c
index 4d12067..6740661 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -469,7 +469,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
while (!task.complete) {
nfs_set_events(client);
- aio_poll(client->aio_context, true);
+ bdrv_aio_poll(client->aio_context, true);
}
return (task.ret < 0 ? task.ret : st.st_blocks * st.st_blksize);
diff --git a/block/qed-table.c b/block/qed-table.c
index 513aa87..6421ce7 100644
--- a/block/qed-table.c
+++ b/block/qed-table.c
@@ -173,7 +173,7 @@ int qed_read_l1_table_sync(BDRVQEDState *s)
qed_read_table(s, s->header.l1_table_offset,
s->l1_table, qed_sync_cb, &ret);
while (ret == -EINPROGRESS) {
- aio_poll(bdrv_get_aio_context(s->bs), true);
+ bdrv_aio_poll(bdrv_get_aio_context(s->bs), true);
}
return ret;
@@ -194,7 +194,7 @@ int qed_write_l1_table_sync(BDRVQEDState *s, unsigned int index,
qed_write_l1_table(s, index, n, qed_sync_cb, &ret);
while (ret == -EINPROGRESS) {
- aio_poll(bdrv_get_aio_context(s->bs), true);
+ bdrv_aio_poll(bdrv_get_aio_context(s->bs), true);
}
return ret;
@@ -267,7 +267,7 @@ int qed_read_l2_table_sync(BDRVQEDState *s, QEDRequest *request, uint64_t offset
qed_read_l2_table(s, request, offset, qed_sync_cb, &ret);
while (ret == -EINPROGRESS) {
- aio_poll(bdrv_get_aio_context(s->bs), true);
+ bdrv_aio_poll(bdrv_get_aio_context(s->bs), true);
}
return ret;
@@ -289,7 +289,7 @@ int qed_write_l2_table_sync(BDRVQEDState *s, QEDRequest *request,
qed_write_l2_table(s, request, index, n, flush, qed_sync_cb, &ret);
while (ret == -EINPROGRESS) {
- aio_poll(bdrv_get_aio_context(s->bs), true);
+ bdrv_aio_poll(bdrv_get_aio_context(s->bs), true);
}
return ret;
diff --git a/blockjob.c b/blockjob.c
index 62bb906..4e52652 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -210,7 +210,7 @@ static int block_job_finish_sync(BlockJob *job,
return -EBUSY;
}
while (data.ret == -EINPROGRESS) {
- aio_poll(bdrv_get_aio_context(bs), true);
+ bdrv_aio_poll(bdrv_get_aio_context(bs), true);
}
return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
}
diff --git a/qemu-img.c b/qemu-img.c
index 75f4ee4..5c0f73b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -654,7 +654,7 @@ static void run_block_job(BlockJob *job, Error **errp)
AioContext *aio_context = bdrv_get_aio_context(job->bs);
do {
- aio_poll(aio_context, true);
+ bdrv_aio_poll(aio_context, true);
qemu_progress_print((float)job->offset / job->len * 100.f, 0);
} while (!job->ready);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 53477e1..b7c0e64 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -478,7 +478,7 @@ static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int count,
co = qemu_coroutine_create(co_write_zeroes_entry);
qemu_coroutine_enter(co, &data);
while (!data.done) {
- aio_poll(blk_get_aio_context(blk), true);
+ bdrv_aio_poll(blk_get_aio_context(blk), true);
}
if (data.ret < 0) {
return data.ret;
@@ -2046,7 +2046,7 @@ static const cmdinfo_t resume_cmd = {
static int wait_break_f(BlockBackend *blk, int argc, char **argv)
{
while (!bdrv_debug_is_suspended(blk_bs(blk), argv[1])) {
- aio_poll(blk_get_aio_context(blk), true);
+ bdrv_aio_poll(blk_get_aio_context(blk), true);
}
return 0;
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [RFC PATCH 11/11] block: Only poll block layer fds in bdrv_aio_poll
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (9 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 10/11] block: Replace nested aio_poll with bdrv_aio_poll Fam Zheng
@ 2015-07-23 6:32 ` Fam Zheng
2015-07-23 8:15 ` [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Paolo Bonzini
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 6:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, qemu-block
So that external events are not processed in nested event loops.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/io.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/io.c b/block/io.c
index 5f5e575..c98fa92 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2611,5 +2611,6 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
bool bdrv_aio_poll(AioContext *ctx, bool blocking)
{
- return aio_poll(ctx, blocking);
+ return aio_poll_clients(ctx, blocking,
+ AIO_CLIENT_PROTOCOL | AIO_CLIENT_CONTEXT);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
` (10 preceding siblings ...)
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 11/11] block: Only poll block layer fds in bdrv_aio_poll Fam Zheng
@ 2015-07-23 8:15 ` Paolo Bonzini
2015-07-23 11:43 ` Fam Zheng
11 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2015-07-23 8:15 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: kwolf, stefanha, qemu-block
On 23/07/2015 08:32, Fam Zheng wrote:
>
> What is realized in this series is similar except the "disable, poll, enable"
> method, instead the bit mask of interesting client types is passed to
> aio_poll() (to be exact, passed to aio_poll_clients()). That is because,
> aio_poll may release the AioContext lock around ppoll, avoiding state will make
> the interface clearer.
However, this means that you must keep the AioContext lock during all
the long-running operations. Is this actually necessary? The original
purpose of keeping the lock (which didn't quite work) was to block out
dataplane operations.
Also, this requirements means you really need the AioContext lock to be
recursive. This is not a huge deal, but I'd prefer that requirement not
to be too ingrained.
Paolo
> After this series, block layer aio_poll() will only process those "protocol"
> fds that are used in block I/O, plus the ctx->notifier for aio_notify(); other
> aio_poll()'s keep unchanged.
>
> The biggest advantage over approaches [1] and [2] is, no change is needed in
> virtio-{blk,scsi}-dataplane code, also this doesn't depend on converting QMP to
> coroutines.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane
2015-07-23 8:15 ` [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Paolo Bonzini
@ 2015-07-23 11:43 ` Fam Zheng
2015-07-24 7:35 ` Paolo Bonzini
0 siblings, 1 reply; 17+ messages in thread
From: Fam Zheng @ 2015-07-23 11:43 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, stefanha, qemu-devel, qemu-block
On Thu, 07/23 10:15, Paolo Bonzini wrote:
>
>
> On 23/07/2015 08:32, Fam Zheng wrote:
> >
> > What is realized in this series is similar except the "disable, poll, enable"
> > method, instead the bit mask of interesting client types is passed to
> > aio_poll() (to be exact, passed to aio_poll_clients()). That is because,
> > aio_poll may release the AioContext lock around ppoll, avoiding state will make
> > the interface clearer.
>
> However, this means that you must keep the AioContext lock during all
> the long-running operations. Is this actually necessary? The original
> purpose of keeping the lock (which didn't quite work) was to block out
> dataplane operations.
I don't really see how we can avoid taking the lock around nested aio_poll
either way. Could you elaborate how it works in your original proposal?
>
> Also, this requirements means you really need the AioContext lock to be
> recursive. This is not a huge deal, but I'd prefer that requirement not
> to be too ingrained.
Regarding the AioContext lock, I see the point that it's an obstacle to
enabling virtio-scsi-dataplane MMIO.
Thinking out loud, maybe the recursive lock is too general: as you said, it has
always been used under BQL to block dataplane operations, instead of being
arbitrarily used by all kinds of threads. I'm wondering if we could just
replace it with some pause / resume mechanism of the dataplane iothreads (e.g.
iothread_pause / iothread_resume, a bit similar to block_job_pause)? That way,
aio_context_acquire can be dropped by:
/* @pause_owner_thread: a callback which will be called when _main thread_
* wants exclusively operate on the AioContext, for example with a nested
* aio_poll().
* @resume_owner_thread: a callback which will be called when _main thread_
* has done the exclusive operation.
*/
AioContext *aio_context_new(AioContextPauseResumeFn *pause_owner_thread,
AioContextPauseResumeFn *resume_owner_thread,
void *opaque,
Error **errp):
/* Try to pause the owning thread */
void aio_context_pause(AioContext *ctx, Error **errp);
/* Try to resume the paused owning thread, cannot fail */
void aio_context_resume(AioContext *ctx);
Then, in iothread.c:
iothread->ctx = aio_context_new(iothread_pause, iothread_resume,
iothread, &local_err);
Where iothread_pause can block the thread with a QemuCond.
Fam
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane
2015-07-23 11:43 ` Fam Zheng
@ 2015-07-24 7:35 ` Paolo Bonzini
2015-07-27 6:55 ` Fam Zheng
0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2015-07-24 7:35 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, stefanha, qemu-devel, qemu-block
On 23/07/2015 13:43, Fam Zheng wrote:
> On Thu, 07/23 10:15, Paolo Bonzini wrote:
>>
>>
>> On 23/07/2015 08:32, Fam Zheng wrote:
>>>
>>> What is realized in this series is similar except the "disable, poll, enable"
>>> method, instead the bit mask of interesting client types is passed to
>>> aio_poll() (to be exact, passed to aio_poll_clients()). That is because,
>>> aio_poll may release the AioContext lock around ppoll, avoiding state will make
>>> the interface clearer.
>>
>> However, this means that you must keep the AioContext lock during all
>> the long-running operations. Is this actually necessary? The original
>> purpose of keeping the lock (which didn't quite work) was to block out
>> dataplane operations.
>
> I don't really see how we can avoid taking the lock around nested aio_poll
> either way. Could you elaborate how it works in your original proposal?
There are two parts to the answer. First, aio_poll is taking locks
already, so the data structures are protected. The question then is
whether recursive locking is necessary in general or just in the current
implementation.
To answer this, let's look at how aio_poll is used, for example in
bdrv_prwv_co:
co = qemu_coroutine_create(bdrv_rw_co_entry);
qemu_coroutine_enter(co, &rwco);
while (rwco.ret == NOT_DONE) {
aio_poll(aio_context, true);
}
There are three cases, with the critical sections becoming smaller and
smaller:
1) the caller takes the lock for the whole duration of the operation.
In this case, the caller works just as in the non-dataplane case. It
can use bdrv_aio_poll as in your patches. This is the way
aio_context_acquire/release are currently used.
2) the caller takes the lock just around bdrv_read. In this case,
bdrv_prwv_co will never be interrupted but I/O from the guest might
sneak in between one bdrv_* call and the next. If I/O from the guest is
blocked with aio_disable_clients + bdrv_drain, on the other hand, this
can work.
3) the caller doesn't have to take the lock; bdrv_rw_co_entry takes care
of locking and also releases the lock around yields. Note that in this
case the locking can be finer-grained. Ultimately you could have a
different lock per BDS. The AioContext lock would only protect the
AioContext's own data structures (bottom halves, file descriptors, etc.)
and it probably would not need to be recursive.
Case 3 is the interesting one, because of the two aio_poll invocations
(from bdrv_read and from the dataplane thread) you don't know which
aio_poll completes the coroutine and sets rwco.ret. However, *it does
not matter*. Whenever I/O finishes, both invocations will be woken up;
one of them will acquire the lock and set rwco.ret, the other will wait
on the lock, find it has nothing to do and, release the lock, and when
it returns the while loop will exit.
So it is not important to lock out other executions of aio_poll, as long
as all executions of aio_poll do exactly the same thing.
aio_disable/enable_clients lets all executions of aio_poll do exactly
the same thing.
>> Also, this requirements means you really need the AioContext lock to be
>> recursive. This is not a huge deal, but I'd prefer that requirement not
>> to be too ingrained.
>
> Regarding the AioContext lock, I see the point that it's an obstacle to
> enabling virtio-scsi-dataplane MMIO.
>
> Thinking out loud, maybe the recursive lock is too general: as you said, it has
> always been used under BQL to block dataplane operations, instead of being
> arbitrarily used by all kinds of threads. I'm wondering if we could just
> replace it with some pause / resume mechanism of the dataplane iothreads (e.g.
> iothread_pause / iothread_resume, a bit similar to block_job_pause)?
That's also an idea, and it lets you use bdrv_aio_poll as in this series.
> That way, aio_context_acquire can be dropped by:
>
> /* @pause_owner_thread: a callback which will be called when _main thread_
> * wants exclusively operate on the AioContext, for example with a nested
> * aio_poll().
> * @resume_owner_thread: a callback which will be called when _main thread_
> * has done the exclusive operation.
> */
> AioContext *aio_context_new(AioContextPauseResumeFn *pause_owner_thread,
> AioContextPauseResumeFn *resume_owner_thread,
> void *opaque,
> Error **errp):
>
> /* Try to pause the owning thread */
> void aio_context_pause(AioContext *ctx, Error **errp);
>
> /* Try to resume the paused owning thread, cannot fail */
> void aio_context_resume(AioContext *ctx);
>
> Then, in iothread.c:
>
> iothread->ctx = aio_context_new(iothread_pause, iothread_resume,
> iothread, &local_err);
>
> Where iothread_pause can block the thread with a QemuCond.
Condition variables don't mix well with recursive mutexes... Once we
have bottom halves outside the AioContext lock, however, we could use a
separate lock for this condition variable. That would work.
I like it, but I still ask myself... what's the difference between
aio_context_pause/resume and aio_disable/enable_clients? :) There is
still state, just in the iothread rather than in the AioContext.
Paolo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane
2015-07-24 7:35 ` Paolo Bonzini
@ 2015-07-27 6:55 ` Fam Zheng
2015-07-27 13:23 ` Paolo Bonzini
0 siblings, 1 reply; 17+ messages in thread
From: Fam Zheng @ 2015-07-27 6:55 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, stefanha, qemu-devel, qemu-block
On Fri, 07/24 09:35, Paolo Bonzini wrote:
> > That way, aio_context_acquire can be dropped by:
> >
> > /* @pause_owner_thread: a callback which will be called when _main thread_
> > * wants exclusively operate on the AioContext, for example with a nested
> > * aio_poll().
> > * @resume_owner_thread: a callback which will be called when _main thread_
> > * has done the exclusive operation.
> > */
> > AioContext *aio_context_new(AioContextPauseResumeFn *pause_owner_thread,
> > AioContextPauseResumeFn *resume_owner_thread,
> > void *opaque,
> > Error **errp):
> >
> > /* Try to pause the owning thread */
> > void aio_context_pause(AioContext *ctx, Error **errp);
> >
> > /* Try to resume the paused owning thread, cannot fail */
> > void aio_context_resume(AioContext *ctx);
> >
> > Then, in iothread.c:
> >
> > iothread->ctx = aio_context_new(iothread_pause, iothread_resume,
> > iothread, &local_err);
> >
> > Where iothread_pause can block the thread with a QemuCond.
>
> Condition variables don't mix well with recursive mutexes... Once we
> have bottom halves outside the AioContext lock, however, we could use a
> separate lock for this condition variable. That would work.
Yes, I thought so.
>
> I like it, but I still ask myself... what's the difference between
> aio_context_pause/resume and aio_disable/enable_clients? :) There is
> still state, just in the iothread rather than in the AioContext.
I don't know, maybe this will make aio_context_acquire no longer necessary so
we get virtio-scsi dataplane fixed?
Fam
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane
2015-07-27 6:55 ` Fam Zheng
@ 2015-07-27 13:23 ` Paolo Bonzini
0 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2015-07-27 13:23 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, stefanha, qemu-devel, qemu-block
On 27/07/2015 08:55, Fam Zheng wrote:
> On Fri, 07/24 09:35, Paolo Bonzini wrote:
>>> That way, aio_context_acquire can be dropped by:
>>>
>>> /* @pause_owner_thread: a callback which will be called when _main thread_
>>> * wants exclusively operate on the AioContext, for example with a nested
>>> * aio_poll().
>>> * @resume_owner_thread: a callback which will be called when _main thread_
>>> * has done the exclusive operation.
>>> */
>>> AioContext *aio_context_new(AioContextPauseResumeFn *pause_owner_thread,
>>> AioContextPauseResumeFn *resume_owner_thread,
>>> void *opaque,
>>> Error **errp):
>>>
>>> /* Try to pause the owning thread */
>>> void aio_context_pause(AioContext *ctx, Error **errp);
>>>
>>> /* Try to resume the paused owning thread, cannot fail */
>>> void aio_context_resume(AioContext *ctx);
>>>
>>> Then, in iothread.c:
>>>
>>> iothread->ctx = aio_context_new(iothread_pause, iothread_resume,
>>> iothread, &local_err);
>>>
>>> Where iothread_pause can block the thread with a QemuCond.
>>
>> Condition variables don't mix well with recursive mutexes... Once we
>> have bottom halves outside the AioContext lock, however, we could use a
>> separate lock for this condition variable. That would work.
>
> Yes, I thought so.
>
>> I like it, but I still ask myself... what's the difference between
>> aio_context_pause/resume and aio_disable/enable_clients? :) There is
>> still state, just in the iothread rather than in the AioContext.
>
> I don't know, maybe this will make aio_context_acquire no longer necessary so
> we get virtio-scsi dataplane fixed?
What you would have is:
1) the main thread calls aio_context_pause/resume, which wait for the
dataplane thread to pause
2) a paused I/O thread use a condition variable or something to wait for
the main thread to call aio_context_resume
In the end, this is still a lock. For example in RFifoLock the critical
sections "look like" they are contained within rfifolock_lock and
rfifolock_unlock, but still RFifoLock is a lock and (as far as deadlocks
are concerned) behaves as if there is a single critical section between
rfifolock_lock and rfifolock_unlock.
So, the bdrv_aio_poll approach works great with the huge critical
sections that we have now. We're effectively using the AioContext lock
_not_ to protect data, but just to shield iothread's usage of block
devices from the dataplane and vice versa.
That worked well as an initial step towards thread-safety (in fact, the
BQL itself is a single huge lock that protects code). However, we will
however move towards fine-grained critical sections sooner or later;
besides virtio-scsi dataplane, they are also necessary in order to do
"real" multiqueue with multiple iothreads per BDS.
Once you make your locks protect data, things do become more complex
(more locks, more critical sections, etc.) but they can at the same time
be less scary. You can reason in terms of invariants that hold at the
end of each critical section, and concurrency is not a big deal anymore
because your new tool (invariants) is both powerful and independent of
concurrency.
You don't have to worry about that now, however.
aio_disable_clients/aio_enable_clients for now can be protected by the
AioContext lock. Later, it can be protected by whatever fine-grained
lock will protect the list of AioHandlers.
Paolo
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-07-27 13:24 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-23 6:32 [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 01/11] aio: Introduce "type" in aio_set_fd_handler and aio_set_event_notifier Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 02/11] aio: Save type to AioHandler Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 03/11] aio-posix: Introduce aio_poll_clients Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 04/11] aio-win32: Implement aio_poll_clients Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 05/11] block: Mark fd handlers as "protocol" Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 06/11] nbd: Mark fd handlers client type as "nbd server" Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 07/11] aio: Mark ctx->notifier's client type as "context" Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 08/11] dataplane: Mark host notifiers' client type as "dataplane" Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 09/11] block: Introduce bdrv_aio_poll Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 10/11] block: Replace nested aio_poll with bdrv_aio_poll Fam Zheng
2015-07-23 6:32 ` [Qemu-devel] [RFC PATCH 11/11] block: Only poll block layer fds in bdrv_aio_poll Fam Zheng
2015-07-23 8:15 ` [Qemu-devel] [RFC PATCH 00/11] aio: Introduce handler type to fix nested aio_poll for dataplane Paolo Bonzini
2015-07-23 11:43 ` Fam Zheng
2015-07-24 7:35 ` Paolo Bonzini
2015-07-27 6:55 ` Fam Zheng
2015-07-27 13:23 ` Paolo Bonzini
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).