> Subject: Re: [PATCH] xen: Drop out of coroutine context
> xen_invalidate_map_cache_entry
>
> On Tue, 16 Jan 2024, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng....@nxp.com>
> >
> > xen_invalidate_map_cache_entry is not expected to run in a coroutine.
> > Without this, there is crash:
> >
> > signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
> > threadid=<optimized out>) at pthread_kill.c:78
> > at /usr/src/debug/glibc/2.38+git-r0/sysdeps/posix/raise.c:26
> > fmt=0xffff9e1ca8a8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
> > assertion=assertion@entry=0xaaaae0d25740 "!qemu_in_coroutine()",
> > file=file@entry=0xaaaae0d301a8 "../qemu-xen-dir-remote/block/graph-
> lock.c", line=line@entry=260,
> > function=function@entry=0xaaaae0e522c0 <__PRETTY_FUNCTION__.3>
> "bdrv_graph_rdlock_main_loop") at assert.c:92
> > assertion=assertion@entry=0xaaaae0d25740 "!qemu_in_coroutine()",
> > file=file@entry=0xaaaae0d301a8 "../qemu-xen-dir-remote/block/graph-
> lock.c", line=line@entry=260,
> > function=function@entry=0xaaaae0e522c0 <__PRETTY_FUNCTION__.3>
> "bdrv_graph_rdlock_main_loop") at assert.c:101
> > at ../qemu-xen-dir-remote/block/graph-lock.c:260
> > at /home/Freenix/work/sw-stash/xen/upstream/tools/qemu-xen-dir-
> remote/include/block/graph-lock.h:259
> > host=host@entry=0xffff742c8000, size=size@entry=2097152)
> > at ../qemu-xen-dir-remote/block/io.c:3362
> > host=0xffff742c8000, size=2097152)
> > at ../qemu-xen-dir-remote/block/block-backend.c:2859
> > host=<optimized out>, size=<optimized out>, max_size=<optimized out>)
> > at ../qemu-xen-dir-remote/block/block-ram-registrar.c:33
> > size=2097152, max_size=2097152)
> > at ../qemu-xen-dir-remote/hw/core/numa.c:883
> > buffer=buffer@entry=0xffff743c5000 "")
> > at ../qemu-xen-dir-remote/hw/xen/xen-mapcache.c:475
> > buffer=buffer@entry=0xffff743c5000 "")
> > at ../qemu-xen-dir-remote/hw/xen/xen-mapcache.c:487
> > as=as@entry=0xaaaae1ca3ae8 <address_space_memory>,
> buffer=0xffff743c5000,
> > len=<optimized out>, is_write=is_write@entry=true,
> > access_len=access_len@entry=32768)
> > at ../qemu-xen-dir-remote/system/physmem.c:3199
> > dir=DMA_DIRECTION_FROM_DEVICE, len=<optimized out>,
> > buffer=<optimized out>, as=0xaaaae1ca3ae8 <address_space_memory>)
> > at /home/Freenix/work/sw-stash/xen/upstream/tools/qemu-xen-dir-
> remote/include/sysemu/dma.h:236
> > elem=elem@entry=0xaaaaf620aa30, len=len@entry=32769)
> > at ../qemu-xen-dir-remote/hw/virtio/virtio.c:758
> > elem=elem@entry=0xaaaaf620aa30, len=len@entry=32769,
> idx=idx@entry=0)
> > at ../qemu-xen-dir-remote/hw/virtio/virtio.c:919
> > elem=elem@entry=0xaaaaf620aa30, len=32769)
> > at ../qemu-xen-dir-remote/hw/virtio/virtio.c:994
> > req=req@entry=0xaaaaf620aa30, status=status@entry=0 '\000')
> > at ../qemu-xen-dir-remote/hw/block/virtio-blk.c:67
> > ret=0) at ../qemu-xen-dir-remote/hw/block/virtio-blk.c:136
> > at ../qemu-xen-dir-remote/block/block-backend.c:1559
> > --Type <RET> for more, q to quit, c to continue without paging--
> > at ../qemu-xen-dir-remote/block/block-backend.c:1614
> > i1=<optimized out>) at ../qemu-xen-dir-remote/util/coroutine-
> ucontext.c:177
> > at ../sysdeps/unix/sysv/linux/aarch64/setcontext.S:123
> >
> > Signed-off-by: Peng Fan <peng....@nxp.com>
>
> Hi Peng! Many thanks for the patch and for the investigation!
>
> Only one minor question below
>
>
> > ---
> > hw/xen/xen-mapcache.c | 31 +++++++++++++++++++++++++++++--
> > 1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c index
> > f7d974677d..4e1bb665ee 100644
> > --- a/hw/xen/xen-mapcache.c
> > +++ b/hw/xen/xen-mapcache.c
> > @@ -481,11 +481,38 @@ static void
> xen_invalidate_map_cache_entry_unlocked(uint8_t *buffer)
> > g_free(entry);
> > }
> >
> > -void xen_invalidate_map_cache_entry(uint8_t *buffer)
> > +typedef struct XenMapCacheData {
> > + Coroutine *co;
> > + uint8_t *buffer;
> > + int ret;
>
> Do we need int ret? It doesn't look like we are using it.
Good catch, it is not needed, I will drop it in V2.
Thanks,
Peng.
>
>
> > +} XenMapCacheData;
> > +
> > +static void xen_invalidate_map_cache_entry_bh(void *opaque)
> > {
> > + XenMapCacheData *data = opaque;
> > +
> > mapcache_lock();
> > - xen_invalidate_map_cache_entry_unlocked(buffer);
> > + xen_invalidate_map_cache_entry_unlocked(data->buffer);
> > mapcache_unlock();
> > +
> > + aio_co_wake(data->co);
> > +}
> > +
> > +void coroutine_mixed_fn xen_invalidate_map_cache_entry(uint8_t
> > +*buffer) {
> > + if (qemu_in_coroutine()) {
> > + XenMapCacheData data = {
> > + .co = qemu_coroutine_self(),
> > + .buffer = buffer,
> > + };
> > + aio_bh_schedule_oneshot(qemu_get_current_aio_context(),
> > + xen_invalidate_map_cache_entry_bh, &data);
> > + qemu_coroutine_yield();
> > + } else {
> > + mapcache_lock();
> > + xen_invalidate_map_cache_entry_unlocked(buffer);
> > + mapcache_unlock();
> > + }
> > }
> >
> > void xen_invalidate_map_cache(void)
> > --
> > 2.35.3
> >
