From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=33674 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P8YaJ-0002FD-Hc for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:21:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P8YZD-0003HY-JV for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:18:56 -0400 Received: from mail-qy0-f180.google.com ([209.85.216.180]:34323) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P8YZD-0003H7-Gd for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:18:55 -0400 Received: by qyk1 with SMTP id 1so2979468qyk.4 for ; Wed, 20 Oct 2010 06:18:54 -0700 (PDT) Message-ID: <4CBEEC3B.9070900@codemonkey.ws> Date: Wed, 20 Oct 2010 08:18:51 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] v6: [PATCH 0/3]: Threadlets: A generic task offloading framework References: <20101019173946.16514.62027.stgit@localhost6.localdomain6> <20101020115744.GA10594@amit-laptop.redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Amit Shah , Arun R Bharadwaj , qemu-devel@nongnu.org On 10/20/2010 07:05 AM, Stefan Hajnoczi wrote: > On Wed, Oct 20, 2010 at 12:57 PM, Amit Shah 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=) >> at /home/amit/src/qemu/exec.c:2936 >> #3 0x00000000004bf9a7 in lduw_phys (addr=) 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). Regards, Anthony Liguori > Stefan > >