@@ -40,8 +40,9 @@ typedef struct {
int max_queues;
const VuDevIface *vu_iface;
+ unsigned int in_flight; /* atomic */
+
/* Protected by ctx lock */
- unsigned int in_flight;
bool wait_idle;
VuDev vu_dev;
QIOChannel *ioc; /* The I/O channel with the client */
@@ -62,6 +63,7 @@ void vhost_user_server_stop(VuServer *server);
void vhost_user_server_inc_in_flight(VuServer *server);
void vhost_user_server_dec_in_flight(VuServer *server);
+bool vhost_user_server_has_in_flight(VuServer *server);
void vhost_user_server_attach_aio_context(VuServer *server, AioContext *ctx);
void vhost_user_server_detach_aio_context(VuServer *server);
@@ -272,7 +272,20 @@ static void vu_blk_exp_resize(void *opaque)
vu_config_change_msg(&vexp->vu_server.vu_dev);
}
+/*
+ * Ensures that bdrv_drained_begin() waits until in-flight requests complete.
+ *
+ * Called with vexp->export.ctx acquired.
+ */
+static bool vu_blk_drained_poll(void *opaque)
+{
+ VuBlkExport *vexp = opaque;
+
+ return vhost_user_server_has_in_flight(&vexp->vu_server);
+}
+
static const BlockDevOps vu_blk_dev_ops = {
+ .drained_poll = vu_blk_drained_poll,
.resize_cb = vu_blk_exp_resize,
};
@@ -78,17 +78,23 @@ static void panic_cb(VuDev *vu_dev, const char *buf)
void vhost_user_server_inc_in_flight(VuServer *server)
{
assert(!server->wait_idle);
- server->in_flight++;
+ qatomic_inc(&server->in_flight);
}
void vhost_user_server_dec_in_flight(VuServer *server)
{
- server->in_flight--;
- if (server->wait_idle && !server->in_flight) {
- aio_co_wake(server->co_trip);
+ if (qatomic_fetch_dec(&server->in_flight) == 1) {
+ if (server->wait_idle) {
+ aio_co_wake(server->co_trip);
+ }
}
}
+bool vhost_user_server_has_in_flight(VuServer *server)
+{
+ return qatomic_load_acquire(&server->in_flight) > 0;
+}
+
static bool coroutine_fn
vu_message_read(VuDev *vu_dev, int conn_fd, VhostUserMsg *vmsg)
{
@@ -192,13 +198,13 @@ static coroutine_fn void vu_client_trip(void *opaque)
/* Keep running */
}
- if (server->in_flight) {
+ if (vhost_user_server_has_in_flight(server)) {
/* Wait for requests to complete before we can unmap the memory */
server->wait_idle = true;
qemu_coroutine_yield();
server->wait_idle = false;
}
- assert(server->in_flight == 0);
+ assert(!vhost_user_server_has_in_flight(server));
vu_deinit(vu_dev);
Each vhost-user-blk request runs in a coroutine. When the BlockBackend enters a drained section we need to enter a quiescent state. Currently any in-flight requests race with bdrv_drained_begin() because it is unaware of vhost-user-blk requests. When blk_co_preadv/pwritev()/etc returns it wakes the bdrv_drained_begin() thread but vhost-user-blk request processing has not yet finished. The request coroutine continues executing while the main loop thread thinks it is in a drained section. One example where this is unsafe is for blk_set_aio_context() where bdrv_drained_begin() is called before .aio_context_detached() and .aio_context_attach(). If request coroutines are still running after bdrv_drained_begin(), then the AioContext could change underneath them and they race with new requests processed in the new AioContext. This could lead to virtqueue corruption, for example. (This example is theoretical, I came across this while reading the code and have not tried to reproduce it.) It's easy to make bdrv_drained_begin() wait for in-flight requests: add a .drained_poll() callback that checks the VuServer's in-flight counter. VuServer just needs an API that returns true when there are requests in flight. The in-flight counter needs to be atomic. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- v5: - Use atomic accesses for in_flight counter in vhost-user-server.c [Kevin] --- include/qemu/vhost-user-server.h | 4 +++- block/export/vhost-user-blk-server.c | 13 +++++++++++++ util/vhost-user-server.c | 18 ++++++++++++------ 3 files changed, 28 insertions(+), 7 deletions(-)