On (Wed) Oct 20 2010 [08:18:51], Anthony Liguori wrote:
> On 10/20/2010 07:05 AM, Stefan Hajnoczi wrote:
> >On Wed, Oct 20, 2010 at 12:57 PM, Amit Shah<amit.s...@redhat.com> wrote:
> >>On (Tue) Oct 19 2010 [23:12:20], Arun R Bharadwaj wrote:
> >>>Hi,
> >>>
> >>>This is the v6 of the patch-series to have a generic asynchronous task
> >>>offloading framework (called threadlets) within qemu.
> >>>
> >>>Request to consider pulling this series as discussed during the
> >>>Qemu-devel call.
> >>I tried this out with virtio-serial (patch below). Have a couple of
> >>things to note:
> >>
> >>- Guests get a SIGUSR2 on startup sometimes. This doesn't happen with
> >> qemu.git, so looks like it's introduced by this patchset.
> >>
> >>- After running some tests, I get an abort. I still have to look at
> >> what's causing it, but doesn't look like it's related to virtio-serial
> >> code.
> >>
> >>Program received signal SIGABRT, Aborted.
> >>0x0000003dc76329a5 in raise () from /lib64/libc.so.6
> >>Missing separate debuginfos, use: debuginfo-install
> >>SDL-1.2.14-8.fc13.x86_64 glibc-2.12.1-2.x86_64
> >>libX11-1.3.1-3.fc13.x86_64 libXau-1.0.5-1.fc12.x86_64
> >>libpng-1.2.44-1.fc13.x86_64 libxcb-1.5-1.fc13.x86_64
> >>ncurses-libs-5.7-7.20100130.fc13.x86_64 zlib-1.2.3-23.fc12.x86_64
> >>(gdb) bt
> >>#0 0x0000003dc76329a5 in raise () from /lib64/libc.so.6
> >>#1 0x0000003dc7634185 in abort () from /lib64/libc.so.6
> >>#2 0x00000000004bf829 in qemu_get_ram_ptr (addr=<value optimized out>)
> >>at /home/amit/src/qemu/exec.c:2936
> >>#3 0x00000000004bf9a7 in lduw_phys (addr=<value optimized out>) at
> >>/home/amit/src/qemu/exec.c:3836
> >>#4 0x0000000000557c90 in vring_avail_idx (vq=0x17b9320, idx=1333) at
> >>/home/amit/src/qemu/hw/virtio.c:133
> >>#5 virtqueue_num_heads (vq=0x17b9320, idx=1333) at
> >>/home/amit/src/qemu/hw/virtio.c:252
> >>#6 0x0000000000557e5e in virtqueue_avail_bytes (vq=0x17b9320,
> >>in_bytes=4096, out_bytes=0) at /home/amit/src/qemu/hw/virtio.c:311
> >>
> >>- I'm using a threadlet to queue up several work items which are to be
> >> processed in a fifo order. There's no cancel function for a threadlet
> >> that either processes all work and then quits the thread or just
> >> cancels all pending work and quits.
> >>
> >> Amit
> >>
> >>
> >>diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> >>index 74ba5ec..caaafbe 100644
> >>--- a/hw/virtio-serial-bus.c
> >>+++ b/hw/virtio-serial-bus.c
> >>@@ -51,6 +51,14 @@ struct VirtIOSerial {
> >> struct virtio_console_config config;
> >> };
> >>
> >>+typedef struct VirtIOSerialWork {
> >>+ ThreadletWork work;
> >>+ VirtIOSerialPort *port;
> >>+ VirtQueue *vq;
> >>+ VirtIODevice *vdev;
> >>+ int discard;
> >>+} VirtIOSerialWork;
> >>+
> >> static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
> >> {
> >> VirtIOSerialPort *port;
> >>@@ -113,10 +121,20 @@ static size_t write_to_port(VirtIOSerialPort *port,
> >> return offset;
> >> }
> >>
> >>-static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
> >>- VirtIODevice *vdev, bool discard)
> >>+static void async_flush_queued_data(ThreadletWork *work)
> >> {
> >>+ VirtIOSerialPort *port;
> >>+ VirtIOSerialWork *vs_work;
> >>+ VirtQueue *vq;
> >>+ VirtIODevice *vdev;
> >> VirtQueueElement elem;
> >>+ int discard;
> >>+
> >>+ vs_work = DO_UPCAST(VirtIOSerialWork, work, work);
> >>+ port = vs_work->port;
> >>+ vq = vs_work->vq;
> >>+ vdev = vs_work->vdev;
> >>+ discard = vs_work->discard;
> >>
> >> assert(port || discard);
> >> assert(virtio_queue_ready(vq));
> >You cannot access guest memory using QEMU RAM functions (or use the
> >virtqueue_pop() function which uses them) from a thread without taking
> >the QEMU global mutex.
> >
> >The abort stack trace is a result of accessing guest RAM from two
> >threads simultaneously.
> >
> >In general it is not safe to use QEMU functions from a thread unless
> >they are explicitly written to work outside the QEMU global mutex.
> >Most functions assume the global mutex, which serializes I/O thread
> >and vcpu changes to global state, is held.
>
> Yes, threadlets are only meant to be used to make synchronous system
> calls asynchronous. They are not meant to add parallelism to QEMU
> (yet).
Yes -- I realised that. (But I don't see why the virtio rings get
modified elsewhere other than the only function I push/pop from above.)
Anyway, just one question as I've still not read the code: does a
running work item in a threadlet block migration? Do the remaining work
items in a threadlet get migrated fine?
Amit
