All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Stefan Hajnoczi <stefanha@gmail.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>,
	Paolo Bonzini <pbonzini@redhat.com>,
	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: Tue, 13 Aug 2013 16:13:03 +0200	[thread overview]
Message-ID: <520A3EEF.5050509@siemens.com> (raw)
In-Reply-To: <20130813134559.GA29990@stefanha-thinkpad.redhat.com>

On 2013-08-13 15:45, Stefan Hajnoczi wrote:
> On Tue, Aug 13, 2013 at 09:56:17AM +0200, Jan Kiszka wrote:
>> in the attempt to use Alex' ppoll-based timer rework for decoupled,
>> real-time capable timer device models I'm now scratching my head over
>> the aio_poll interface. I'm looking at dataplane/virtio-blk.c, just finding
>>
>> static void *data_plane_thread(void *opaque)
>> {
>>     VirtIOBlockDataPlane *s = opaque;
>>
>>     do {
>>         aio_poll(s->ctx, true);
>>     } while (!s->stopping || s->num_reqs > 0);
>>     return NULL;
>> }
>>
>> wondering where the locking is. Or doesn't this use need any at all? Are
>> all data structures that this thread accesses exclusively used by it, or
>> are they all accessed in a lock-less way?
> 
> Most of the data structures in dataplane upstream are not shared.
> Virtio, virtio-blk, and Linux AIO raw file I/O are duplicated for
> dataplane and do not rely on QEMU infrastructure.
> 
> I've been working on undoing this duplication over the past months but
> upstream QEMU still mostly does not share data structures and therefore
> does not need much synchronization.  For the crude synchronization that
> we do need we simply start/stop the dataplane thread.
> 
>> Our iothread mainloop more or less open-codes aio_poll and is, thus,
>> able to drop its lock before falling asleep while still holding it
>> during event dispatching. Obviously, I need the same when processing
>> timer lists of an AioContext, protecting them against concurrent
>> modifications over VCPUs or other threads. So I'm thinking of adding a
>> block notification callback to aio_poll, to be called before/after
>> qemu_poll_ns so that any locks can be dropped / reacquired as needed. Or
>> am I missing some magic interface / pattern?
> 
> Upstream dataplane does not use timers, so the code there cannot serve
> as an example.
> 
> If you combine Alex Bligh, Ping Fan, and my latest timer series, you get
> support for QEMUTimer in AioContexts where qemu_timer_mod_ns() and
> qemu_timer_del() are thread-safe.  vm_clock (without icount) and
> rt_clock are thread-safe clock sources.

To which series of yours and Ping Fan are you referring? [1] and [2]?

> 
> This should make timers usable in another thread for clock device
> emulation if only your iothread uses the AioContext and its timers
> (besides the thread-safe mod/del interfaces).

As argued in the other thread, I don't think we need (and want) locking
in the timer subsystem, rather push this to its users. But I'll look
again at your patches, if they are also usable.

> 
> The details depend on your device, do you have a git repo I can look at
> to understand your device model?

Pushed my hacks here:

git://git.kiszka.org/qemu.git queues/rt.new3

Jan

[1] http://thread.gmane.org/gmane.comp.emulators.qemu/227590
[2] http://thread.gmane.org/gmane.comp.emulators.qemu/226369

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

  reply	other threads:[~2013-08-13 14:14 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 [this message]
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
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=520A3EEF.5050509@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=alex@alex.org.uk \
    --cc=aliguori@us.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=morita.kazutaka@lab.ntt.co.jp \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemulist@gmail.com \
    --cc=rth@twiddle.net \
    --cc=stefanha@gmail.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.