* Ensure the PV ring is drained on disconnect @ 2023-03-29 10:53 Mark Syms via 2023-03-29 10:53 ` [PATCH] " Mark Syms via 0 siblings, 1 reply; 4+ messages in thread From: Mark Syms via @ 2023-03-29 10:53 UTC (permalink / raw) To: qemu-devel; +Cc: tim.smith, mark.syms If the Xen PV guest VM sends a close whilst there is outstanding I/O being processed that IO needs to be completed and drained before unrealizing the rings or SEGVs will occurr when the I/O does complete and tries to update an already unmapped grant entry. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Ensure the PV ring is drained on disconnect 2023-03-29 10:53 Ensure the PV ring is drained on disconnect Mark Syms via @ 2023-03-29 10:53 ` Mark Syms via 2023-04-20 10:20 ` [PATCH 0/1] Updated: Ensure PV ring is drained on disconenct Mark Syms via 0 siblings, 1 reply; 4+ messages in thread From: Mark Syms via @ 2023-03-29 10:53 UTC (permalink / raw) To: qemu-devel; +Cc: tim.smith, mark.syms, Mark Syms Also ensure all pending AIO is complete. Signed-off-by: Mark Syms <mark.syms@citrix.com> --- hw/block/dataplane/xen-block.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index 734da42ea7..067f8e2f45 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -523,6 +523,10 @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane) dataplane->more_work = 0; + if (dataplane->sring == 0) { + return done_something; + } + rc = dataplane->rings.common.req_cons; rp = dataplane->rings.common.sring->req_prod; xen_rmb(); /* Ensure we see queued requests up to 'rp'. */ @@ -666,11 +670,23 @@ void xen_block_dataplane_destroy(XenBlockDataPlane *dataplane) void xen_block_dataplane_stop(XenBlockDataPlane *dataplane) { XenDevice *xendev; + XenBlockRequest *request, *next; if (!dataplane) { return; } + /* Ensure we have drained the ring */ + do { + xen_block_handle_requests(dataplane); + } while (dataplane->more_work); + + /* Now ensure that all inflight requests are complete */ + QLIST_FOREACH_SAFE(request, &dataplane->inflight, list, next) { + blk_aio_flush(request->dataplane->blk, xen_block_complete_aio, + request); + } + xendev = dataplane->xendev; aio_context_acquire(dataplane->ctx); -- 2.39.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 0/1] Updated: Ensure PV ring is drained on disconenct 2023-03-29 10:53 ` [PATCH] " Mark Syms via @ 2023-04-20 10:20 ` Mark Syms via 2023-05-06 1:13 ` Stefano Stabellini 0 siblings, 1 reply; 4+ messages in thread From: Mark Syms via @ 2023-04-20 10:20 UTC (permalink / raw) To: qemu-devel; +Cc: sstabellini, anthony.perard, paul, xen-devel, Mark Syms Updated patch to address intermittent SIGSEGV on domain disconnect/shutdown. Mark Syms (1): Ensure the PV ring is drained on disconnect hw/block/dataplane/xen-block.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) -- 2.40.0 From 21724baa15a72534d98aa2653e9ec39e83559319 Mon Sep 17 00:00:00 2001 From: Mark Syms <mark.syms@citrix.com> Date: Thu, 20 Apr 2023 11:08:34 +0100 Subject: [PATCH 1/1] Ensure the PV ring is drained on disconnect Also ensure all pending AIO is complete. Signed-off-by: Mark Syms <mark.syms@citrix.com> --- hw/block/dataplane/xen-block.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index 734da42ea7..d9da4090bf 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -523,6 +523,10 @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane) dataplane->more_work = 0; + if (dataplane->sring == 0) { + return done_something; + } + rc = dataplane->rings.common.req_cons; rp = dataplane->rings.common.sring->req_prod; xen_rmb(); /* Ensure we see queued requests up to 'rp'. */ @@ -666,14 +670,35 @@ void xen_block_dataplane_destroy(XenBlockDataPlane *dataplane) void xen_block_dataplane_stop(XenBlockDataPlane *dataplane) { XenDevice *xendev; + XenBlockRequest *request, *next; if (!dataplane) { return; } + /* We're about to drain the ring. We can cancel the scheduling of any + * bottom half now */ + qemu_bh_cancel(dataplane->bh); + + /* Ensure we have drained the ring */ + aio_context_acquire(dataplane->ctx); + do { + xen_block_handle_requests(dataplane); + } while (dataplane->more_work); + aio_context_release(dataplane->ctx); + + /* Now ensure that all inflight requests are complete */ + while (!QLIST_EMPTY(&dataplane->inflight)) { + QLIST_FOREACH_SAFE(request, &dataplane->inflight, list, next) { + blk_aio_flush(request->dataplane->blk, xen_block_complete_aio, + request); + } + } + xendev = dataplane->xendev; aio_context_acquire(dataplane->ctx); + if (dataplane->event_channel) { /* Only reason for failure is a NULL channel */ xen_device_set_event_channel_context(xendev, dataplane->event_channel, @@ -684,12 +709,6 @@ void xen_block_dataplane_stop(XenBlockDataPlane *dataplane) blk_set_aio_context(dataplane->blk, qemu_get_aio_context(), &error_abort); aio_context_release(dataplane->ctx); - /* - * Now that the context has been moved onto the main thread, cancel - * further processing. - */ - qemu_bh_cancel(dataplane->bh); - if (dataplane->event_channel) { Error *local_err = NULL; -- 2.40.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 0/1] Updated: Ensure PV ring is drained on disconenct 2023-04-20 10:20 ` [PATCH 0/1] Updated: Ensure PV ring is drained on disconenct Mark Syms via @ 2023-05-06 1:13 ` Stefano Stabellini 0 siblings, 0 replies; 4+ messages in thread From: Stefano Stabellini @ 2023-05-06 1:13 UTC (permalink / raw) To: Mark Syms; +Cc: qemu-devel, sstabellini, anthony.perard, paul, xen-devel On Thu, 20 Apr 2023, Mark Syms wrote: > Updated patch to address intermittent SIGSEGV on domain disconnect/shutdown. > > Mark Syms (1): > Ensure the PV ring is drained on disconnect > > hw/block/dataplane/xen-block.c | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > -- > 2.40.0 > > >From 21724baa15a72534d98aa2653e9ec39e83559319 Mon Sep 17 00:00:00 2001 > From: Mark Syms <mark.syms@citrix.com> > Date: Thu, 20 Apr 2023 11:08:34 +0100 > Subject: [PATCH 1/1] Ensure the PV ring is drained on disconnect > > Also ensure all pending AIO is complete. Hi Mark, can you please add more info on the problem you are trying to solve? Also add any stacktrace if you get any due to this error. > Signed-off-by: Mark Syms <mark.syms@citrix.com> > --- > hw/block/dataplane/xen-block.c | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c > index 734da42ea7..d9da4090bf 100644 > --- a/hw/block/dataplane/xen-block.c > +++ b/hw/block/dataplane/xen-block.c > @@ -523,6 +523,10 @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane) > > dataplane->more_work = 0; > > + if (dataplane->sring == 0) { > + return done_something; done_something cannot be changed by now, so I would just do return false; > + } > + > rc = dataplane->rings.common.req_cons; > rp = dataplane->rings.common.sring->req_prod; > xen_rmb(); /* Ensure we see queued requests up to 'rp'. */ > @@ -666,14 +670,35 @@ void xen_block_dataplane_destroy(XenBlockDataPlane *dataplane) > void xen_block_dataplane_stop(XenBlockDataPlane *dataplane) > { > XenDevice *xendev; > + XenBlockRequest *request, *next; > > if (!dataplane) { > return; > } > > + /* We're about to drain the ring. We can cancel the scheduling of any > + * bottom half now */ > + qemu_bh_cancel(dataplane->bh); > + > + /* Ensure we have drained the ring */ > + aio_context_acquire(dataplane->ctx); Would it make sense to move the 2 loops below under the existing aio_context_acquire also below? > + do { > + xen_block_handle_requests(dataplane); > + } while (dataplane->more_work); > + aio_context_release(dataplane->ctx); > + > + /* Now ensure that all inflight requests are complete */ > + while (!QLIST_EMPTY(&dataplane->inflight)) { > + QLIST_FOREACH_SAFE(request, &dataplane->inflight, list, next) { > + blk_aio_flush(request->dataplane->blk, xen_block_complete_aio, > + request); > + } > + } especially because I would think that blk_aio_flush needs to be called with aio_context_acquired ? > xendev = dataplane->xendev; > > aio_context_acquire(dataplane->ctx); > + move the new code here > if (dataplane->event_channel) { > /* Only reason for failure is a NULL channel */ > xen_device_set_event_channel_context(xendev, dataplane->event_channel, > @@ -684,12 +709,6 @@ void xen_block_dataplane_stop(XenBlockDataPlane *dataplane) > blk_set_aio_context(dataplane->blk, qemu_get_aio_context(), &error_abort); > aio_context_release(dataplane->ctx); > > - /* > - * Now that the context has been moved onto the main thread, cancel > - * further processing. > - */ > - qemu_bh_cancel(dataplane->bh); > - > if (dataplane->event_channel) { > Error *local_err = NULL; > > -- > 2.40.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-06 1:14 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-29 10:53 Ensure the PV ring is drained on disconnect Mark Syms via 2023-03-29 10:53 ` [PATCH] " Mark Syms via 2023-04-20 10:20 ` [PATCH 0/1] Updated: Ensure PV ring is drained on disconenct Mark Syms via 2023-05-06 1:13 ` Stefano Stabellini
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).