qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <aliguori@linux.vnet.ibm.com>
To: Avi Kivity <avi@redhat.com>
Cc: ericvh@gmail.com, jvrao <jvrao@linux.vnet.ibm.com>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU
Date: Mon, 29 Mar 2010 16:23:20 -0500	[thread overview]
Message-ID: <4BB11A48.4060805@linux.vnet.ibm.com> (raw)
In-Reply-To: <4BB1136B.6050506@redhat.com>

On 03/29/2010 03:54 PM, Avi Kivity wrote:
> On 03/29/2010 11:42 PM, Anthony Liguori wrote:
>>>> For individual device models or host services, I think (3) is 
>>>> probably the worst model overall.  I personally think that (1) is 
>>>> better in the long run but ultimately would need an existence proof 
>>>> to compare against (2).  (2) looks appealing until you actually try 
>>>> to have the device handle multiple requests at a time.
>>>
>>> Sooner or later nature and the ever more complicated code will force 
>>> us towards (3).  As an example, we've observed live migration to 
>>> throttle vcpus when sending a large guest's zeroed memory over; the 
>>> bandwidth control doesn't kick in since zero pages are compressed, 
>>> so the iothread spends large amounts of time reading memory.
>>
>>
>> Making things re-entrant is different than (3) in my mind.
>>
>> There's no reason that VCPU threads should run in lock-step with live 
>> migration during the live phase.  Making device models re-entrant and 
>> making live migration depend not depend on the big global lock is a 
>> good thing to do.
>
> It's not sufficient.  If you have a single thread that runs both live 
> migrations and timers, then timers will be backlogged behind live 
> migration, or you'll have to yield often.  This is regardless of the 
> locking model (and of course having threads without fixing the locking 
> is insufficient as well, live migration accesses guest memory so it 
> needs the big qemu lock).

But what's the solution?  Sending every timer in a separate thread?  
We'll hit the same problem if we implement an arbitrary limit to number 
of threads.

>> What I'm skeptical of, is whether converting virtio-9p or qcow2 to 
>> handle each request in a separate thread is really going to improve 
>> things. 
>
> Currently qcow2 isn't even fullly asynchronous, so it can't fail to 
> improve things.

Unless it introduces more data corruptions which is my concern with any 
significant change to qcow2.

>> The VNC server is another area that I think multithreading would be a 
>> bad idea.
>
> If the vnc server is stuffing a few megabytes of screen into a socket, 
> then timers will be delayed behind it, unless you litter the code with 
> calls to bottom halves.  Even worse if it does complicated compression 
> and encryption.

Sticking the VNC server in it's own thread would be fine.  Trying to 
make the VNC server multithreaded though would be problematic.

Basically, sticking isolated components in a single thread should be 
pretty reasonable.

>>>
>>> But if those system calls are blocking, you need a thread?
>>
>> You can dispatch just the system call to a thread pool.  The 
>> advantage of doing that is that you don't need to worry about locking 
>> since the system calls are not (usually) handling shared state.
>
> There is always implied shared state.  If you're doing direct guest 
> memory access, you need to lock memory against hotunplug, or the 
> syscall will end up writing into freed memory.  If the device can be 
> hotunplugged, you need to make sure all threads have returned before 
> unplugging it.

There are other ways to handle hot unplug (like reference counting) that 
avoid this problem.

Ultimately, this comes down to a question of lock granularity and thread 
granularity.  I don't think it's a good idea to start with the 
assumption that we want extremely fine granularity.  There's certainly 
very low hanging fruit with respect to threading.

>>> On a philosophical note, threads may be easier to model complex 
>>> hardware that includes a processor, for example our scsi card (and 
>>> how about using tcg as a jit to boost it :)
>>
>> Yeah, it's hard to argue that script evaluation shouldn't be done in 
>> a thread.  But that doesn't prevent me from being very cautious about 
>> how and where we use threading :-)
>
> Caution where threads are involved is a good thing.  They are 
> inevitable however, IMO.

We already are using threads so they aren't just inevitable, they're 
reality.  I still don't think using threads would significantly simplify 
virtio-9p.

Regards,

Anthony Liguori

  reply	other threads:[~2010-03-29 21:23 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-25 16:43 [Qemu-devel] [PATCH -V3 00/32] virtio-9p: paravirtual file system passthrough Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 01/32] vitio-9p: Add a virtio 9p device to qemu Aneesh Kumar K.V
2010-03-25 21:04   ` Anthony Liguori
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 02/32] vrtio-9p: Implement P9_TVERSION for 9P Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 03/32] virtio-9p: Implement P9_TATTACH Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 04/32] virtio-9p: Implement P9_TSTAT Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 05/32] virtio-9p: Implement P9_TWALK Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 06/32] virtio-9p: Implement P9_TOPEN Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 07/32] virtio-9p: Implement P9_TREAD Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 08/32] virtio-9p: Implement P9_TCLUNK Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE Aneesh Kumar K.V
2010-03-29  6:36   ` [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU jvrao
2010-03-29 15:00     ` Anthony Liguori
2010-03-29 20:31       ` Avi Kivity
2010-03-29 20:42         ` Anthony Liguori
2010-03-29 20:54           ` Avi Kivity
2010-03-29 21:23             ` Anthony Liguori [this message]
2010-03-30 10:24               ` Avi Kivity
2010-03-30 13:13                 ` Anthony Liguori
2010-03-30 13:28                   ` Avi Kivity
2010-03-30 13:54                     ` Anthony Liguori
2010-03-30 14:03                       ` Avi Kivity
2010-03-30 14:07                         ` Anthony Liguori
2010-03-30 14:23                           ` Avi Kivity
2010-03-30 14:59                             ` Anthony Liguori
2010-03-29 21:17           ` jvrao
2010-03-30 10:28             ` Avi Kivity
2010-04-01 13:14           ` Paul Brook
2010-04-01 14:34             ` Avi Kivity
2010-04-01 15:30               ` Paul Brook
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 10/32] virtio-9p: Implement P9_TCREATE Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 11/32] virtio-9p: Implement P9_TWSTAT Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 12/32] virtio-9p: Implement P9_TREMOVE Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 13/32] virtio-9p: Implement P9_TFLUSH Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 14/32] virtio-9p: Add multiple mount point support Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 15/32] virtio-9p: Use little endian format on virtio Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 16/32] virtio-9p: Add support for hardlink Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 17/32] Implement sync support in 9p server Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 18/32] virtio-9p: Fix sg usage in the code Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 19/32] virtio-9p: Get the correct count values from the pdu Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 20/32] virtio-9p: Remove BUG_ON and add proper error handling Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 21/32] virtio-9p: Remove unnecessary definition of fid Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 22/32] virtio-9p: Update existing fid path on rename Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 23/32] vritio-9p: Fix chmod bug with directory Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 24/32] qemu-malloc: Add qemu_asprintf Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 25/32] virtio-9p: Move V9fs File system specific options to a separate header file Aneesh Kumar K.V
2010-03-29  0:52   ` jvrao
2010-03-29  1:09   ` jvrao
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 26/32] virtio-9p: Create a commandline option -fsdev Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 27/32] virtio-9p: Create qemu_fsdev_opts Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 28/32] virtio-9p: Handle the fsdev command line options Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 29/32] virtio-9p: Decouple share_path details from virtio-9p-dev Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 30/32] virtio-9p: Create a syntactic shortcut for the file-system pass-thru Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 31/32] virtio-9p: Return proper errors from create paths Aneesh Kumar K.V
2010-03-25 16:43 ` [Qemu-devel] [PATCH -V3 32/32] virtio-9p: Handle unknown 9P protocol versions as per the standards Aneesh Kumar K.V

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BB11A48.4060805@linux.vnet.ibm.com \
    --to=aliguori@linux.vnet.ibm.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=avi@redhat.com \
    --cc=ericvh@gmail.com \
    --cc=jvrao@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).