qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	liu ping fan <qemulist@gmail.com>, Alex Bligh <alex@alex.org.uk>,
	MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] Using aio_poll for timer carrier threads
Date: Mon, 19 Aug 2013 16:09:48 +0200	[thread overview]
Message-ID: <5212272C.1020008@redhat.com> (raw)
In-Reply-To: <5212206B.3040507@siemens.com>

Il 19/08/2013 15:40, Jan Kiszka ha scritto:
> On 2013-08-19 15:21, Paolo Bonzini wrote:
>> Il 13/08/2013 16:54, Jan Kiszka ha scritto:
>>>>> Using an AioContext lock for timers is somewhat complicated for lock
>>>>> ordering, because context A could try to modify a timer from context B,
>>>>> at the same time when context B is modifying a timer from context A.
>>>>> This would cause a deadlock.
>>> That's like MMIO access on device A triggers MMIO access on B and vice
>>> versa - why should we need this, so why should we support this? I think
>>> the typical case is that timers (and their lists) and data structures
>>> they access have a fairly close relation, thus can reuse the same lock.
>>
>> Yes, that's true.  Still it would have to be documented, and using
>> too-coarse locks risks having many BQLs, which multiplies the complexity
>> (fine-grained locking at least keeps critical sections small and limits
>> the amount of nested locking).
> 
> As this lock does not require taking other locks while holding it, it
> should actually be fine.

Using an AioContext lock would be more dangerous, though.  The risk is
mostly that someone is holding _another_ AioContext lock while calling a
timer function.

For MMIO, the rule would be that all address_space_rw/map/unmap must be
done with no lock taken.

>> I like Stefan's patches to make the timer list thread-safe, especially
>> if we can optimize it (with RCU?) to make the read side lockless.
> 
> What is a pure read-side in that context? Checks if some timer is
> expired?

Yes.  It is actually quite common, as Stefan pointed out elsewhere to
Ping Fan, as it happens at least once in each iteration of the loop.

The read side doesn't really need to look beyond the head of the list.

> Given that RCU write sides are heavier than plain mutexes and
> many typical accesses (mod, del, expire) involve writing, such an
> optimization may also be counterproductive.

This would have to be a creative use of RCU, with no "copy" in it...
basically you can use RCU only as a substitute for reference counting.
call_rcu/synchronize_rcu is not done on every write operation, all we
need to do is ensuring that devices free timers only after an RCU
critical section.  We should do the same for memory regions, see my
patches to split exitfn and instance_finalize; I'll go more in depth in
my KVM Forum 2013.

Lockless algorithms (including RCU) are somewhat appealing because you
do not have to worry about locking, but of course all warnings about
increased complexity and premature optimization apply.

Paolo

  reply	other threads:[~2013-08-19 14:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-13  7:56 [Qemu-devel] Using aio_poll for timer carrier threads Jan Kiszka
2013-08-13 13:45 ` Stefan Hajnoczi
2013-08-13 14:13   ` Jan Kiszka
2013-08-14  0:48     ` liu ping fan
2013-08-14  8:52     ` Stefan Hajnoczi
2013-08-14  9:05       ` Jan Kiszka
2013-08-14 11:35         ` Stefan Hajnoczi
2013-08-14 12:32     ` Jan Kiszka
2013-08-19 12:00       ` Paolo Bonzini
2013-08-13 14:45 ` Paolo Bonzini
2013-08-13 14:54   ` Jan Kiszka
2013-08-19 13:21     ` Paolo Bonzini
2013-08-19 13:40       ` Jan Kiszka
2013-08-19 14:09         ` Paolo Bonzini [this message]
2013-08-19 13:58       ` Alex Bligh

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=5212272C.1020008@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=alex@alex.org.uk \
    --cc=aliguori@us.ibm.com \
    --cc=jan.kiszka@siemens.com \
    --cc=kwolf@redhat.com \
    --cc=morita.kazutaka@lab.ntt.co.jp \
    --cc=qemu-devel@nongnu.org \
    --cc=qemulist@gmail.com \
    --cc=rth@twiddle.net \
    --cc=stefanha@redhat.com \
    /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).