All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Liu Ping Fan" <qemulist@gmail.com>,
	qemu-devel@nongnu.org, kvm@vger.kernel.org,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"Blue Swirl" <blauwirbel@gmail.com>,
	"Avi Kivity" <avi@redhat.com>,
	"Anthony Liguori" <anthony@codemonkey.ws>,
	"Jan Kiszka" <jan.kiszka@siemens.com>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [PATCH 01/15] atomic: introduce atomic operations
Date: Wed, 08 Aug 2012 15:18:22 +0200	[thread overview]
Message-ID: <5022671E.8070602@redhat.com> (raw)
In-Reply-To: <CAJSP0QUiRQrKe7foKK34K812pjSq4vFbB23g8_JsAGzn43N2vA@mail.gmail.com>

Il 08/08/2012 15:09, Stefan Hajnoczi ha scritto:
>> > NAK. We don't want random inline assembly implementations of locking
>> > primitives in QEMU, they are way too hard to keep working with all the
>> > possible host architectures we support. I spent some time a while back
>> > getting rid of the (variously busted) versions we had previously.
>> >
>> > If you absolutely must use atomic ops, use the gcc builtins. For
>> > preference, stick to higher level and less error-prone abstractions.
> We're spoilt for choice here:
> 
> 1. Atomic built-ins from gcc
> 2. glib atomics
> 
> No need to roll our own or copy the implementation from the kernel.

To some extent we need to because:

1. GCC atomics look ugly, :) do not provide rmb/wmb, and in some
versions of GCC mb is known to be (wrongly) a no-op.

2. glib atomics do not provide mb/rmb/wmb either, and
g_atomic_int_get/g_atomic_int_set are inefficient: they add barriers
everywhere, while it is clearer if you put barriers manually, and you
often do not need barriers in the get side.  glib atomics also do not
provide xchg.

I agree however that a small wrapper around GCC atomics is much better
than assembly.  Assembly can be limited to the memory barriers (where we
already have it anyway).

Paolo

WARNING: multiple messages have this Message-ID (diff)
From: Paolo Bonzini <pbonzini@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	kvm@vger.kernel.org, "Jan Kiszka" <jan.kiszka@siemens.com>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"Liu Ping Fan" <qemulist@gmail.com>,
	qemu-devel@nongnu.org, "Blue Swirl" <blauwirbel@gmail.com>,
	"Avi Kivity" <avi@redhat.com>,
	"Anthony Liguori" <anthony@codemonkey.ws>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [PATCH 01/15] atomic: introduce atomic operations
Date: Wed, 08 Aug 2012 15:18:22 +0200	[thread overview]
Message-ID: <5022671E.8070602@redhat.com> (raw)
In-Reply-To: <CAJSP0QUiRQrKe7foKK34K812pjSq4vFbB23g8_JsAGzn43N2vA@mail.gmail.com>

Il 08/08/2012 15:09, Stefan Hajnoczi ha scritto:
>> > NAK. We don't want random inline assembly implementations of locking
>> > primitives in QEMU, they are way too hard to keep working with all the
>> > possible host architectures we support. I spent some time a while back
>> > getting rid of the (variously busted) versions we had previously.
>> >
>> > If you absolutely must use atomic ops, use the gcc builtins. For
>> > preference, stick to higher level and less error-prone abstractions.
> We're spoilt for choice here:
> 
> 1. Atomic built-ins from gcc
> 2. glib atomics
> 
> No need to roll our own or copy the implementation from the kernel.

To some extent we need to because:

1. GCC atomics look ugly, :) do not provide rmb/wmb, and in some
versions of GCC mb is known to be (wrongly) a no-op.

2. glib atomics do not provide mb/rmb/wmb either, and
g_atomic_int_get/g_atomic_int_set are inefficient: they add barriers
everywhere, while it is clearer if you put barriers manually, and you
often do not need barriers in the get side.  glib atomics also do not
provide xchg.

I agree however that a small wrapper around GCC atomics is much better
than assembly.  Assembly can be limited to the memory barriers (where we
already have it anyway).

Paolo

  reply	other threads:[~2012-08-08 13:18 UTC|newest]

Thread overview: 154+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-08  6:25 [PATCH 0/15 v2] prepare unplug out of protection of global lock Liu Ping Fan
2012-08-08  6:25 ` [Qemu-devel] " Liu Ping Fan
2012-08-08  6:25 ` [PATCH 01/15] atomic: introduce atomic operations Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  8:55   ` Paolo Bonzini
2012-08-08  8:55     ` [Qemu-devel] " Paolo Bonzini
2012-08-08  9:02   ` Avi Kivity
2012-08-08  9:02     ` [Qemu-devel] " Avi Kivity
2012-08-08  9:05     ` 陳韋任 (Wei-Ren Chen)
2012-08-08  9:05       ` 陳韋任 (Wei-Ren Chen)
2012-08-08  9:15       ` Avi Kivity
2012-08-08  9:15         ` [Qemu-devel] " Avi Kivity
2012-08-08  9:21   ` Peter Maydell
2012-08-08  9:21     ` Peter Maydell
2012-08-08 13:09     ` Stefan Hajnoczi
2012-08-08 13:09       ` Stefan Hajnoczi
2012-08-08 13:18       ` Paolo Bonzini [this message]
2012-08-08 13:18         ` Paolo Bonzini
2012-08-08 13:32         ` Peter Maydell
2012-08-08 13:32           ` [Qemu-devel] " Peter Maydell
2012-08-08 13:49           ` Paolo Bonzini
2012-08-08 13:49             ` [Qemu-devel] " Paolo Bonzini
2012-08-08 14:00             ` Avi Kivity
2012-08-08 14:00               ` [Qemu-devel] " Avi Kivity
2012-08-08  6:25 ` [PATCH 02/15] qom: using atomic ops to re-implement object_ref Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  6:25 ` [PATCH 03/15] qom: introduce reclaimer to release obj Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:05   ` Avi Kivity
2012-08-08  9:05     ` [Qemu-devel] " Avi Kivity
2012-08-08  9:07     ` Paolo Bonzini
2012-08-08  9:07       ` [Qemu-devel] " Paolo Bonzini
2012-08-08  9:15       ` Avi Kivity
2012-08-08  9:15         ` [Qemu-devel] " Avi Kivity
2012-08-09  7:33         ` liu ping fan
2012-08-09  7:33           ` [Qemu-devel] " liu ping fan
2012-08-09  7:49           ` Paolo Bonzini
2012-08-09  7:49             ` [Qemu-devel] " Paolo Bonzini
2012-08-09  8:18             ` Avi Kivity
2012-08-09  8:18               ` [Qemu-devel] " Avi Kivity
2012-08-10  6:43               ` liu ping fan
2012-08-10  6:43                 ` [Qemu-devel] " liu ping fan
2012-08-08  9:35   ` Paolo Bonzini
2012-08-08  9:35     ` [Qemu-devel] " Paolo Bonzini
2012-08-09  7:38     ` liu ping fan
2012-08-09  7:38       ` [Qemu-devel] " liu ping fan
2012-08-08  6:25 ` [PATCH 04/15] memory: MemoryRegion topology must be stable when updating Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:13   ` Avi Kivity
2012-08-08  9:13     ` [Qemu-devel] " Avi Kivity
2012-08-09  7:28     ` liu ping fan
2012-08-09  7:28       ` [Qemu-devel] " liu ping fan
2012-08-09  8:24       ` Avi Kivity
2012-08-09  8:24         ` [Qemu-devel] " Avi Kivity
2012-08-10  6:44         ` liu ping fan
2012-08-10  6:44           ` [Qemu-devel] " liu ping fan
2012-08-13 18:28       ` Marcelo Tosatti
2012-08-13 18:28         ` [Qemu-devel] " Marcelo Tosatti
2012-08-08 19:17   ` Blue Swirl
2012-08-08 19:17     ` [Qemu-devel] " Blue Swirl
2012-08-09  7:28     ` liu ping fan
2012-08-09  7:28       ` [Qemu-devel] " liu ping fan
2012-08-09 17:09       ` Blue Swirl
2012-08-09 17:09         ` [Qemu-devel] " Blue Swirl
2012-08-08  6:25 ` [PATCH 05/15] memory: introduce life_ops to MemoryRegion Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:18   ` Avi Kivity
2012-08-08  9:18     ` [Qemu-devel] " Avi Kivity
2012-08-08  6:25 ` [PATCH 06/15] memory: use refcnt to manage MemoryRegion Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:20   ` Avi Kivity
2012-08-08  9:20     ` [Qemu-devel] " Avi Kivity
2012-08-09  7:27     ` liu ping fan
2012-08-09  7:27       ` [Qemu-devel] " liu ping fan
2012-08-09  8:38       ` Avi Kivity
2012-08-09  8:38         ` [Qemu-devel] " Avi Kivity
2012-08-10  6:44         ` liu ping fan
2012-08-10  6:44           ` [Qemu-devel] " liu ping fan
2012-08-12  8:43           ` Avi Kivity
2012-08-12  8:43             ` [Qemu-devel] " Avi Kivity
2012-08-08  6:25 ` [PATCH 07/15] memory: inc/dec mr's ref when adding/removing from mem view Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  6:25 ` [PATCH 08/15] memory: introduce PhysMap to present snapshot of toploygy Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:27   ` Avi Kivity
2012-08-08  9:27     ` [Qemu-devel] " Avi Kivity
2012-08-08 19:18   ` Blue Swirl
2012-08-08 19:18     ` [Qemu-devel] " Blue Swirl
2012-08-09  7:29     ` liu ping fan
2012-08-09  7:29       ` [Qemu-devel] " liu ping fan
2012-08-08  6:25 ` [PATCH 09/15] memory: prepare flatview and radix-tree for rcu style access Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:41   ` Avi Kivity
2012-08-08  9:41     ` [Qemu-devel] " Avi Kivity
2012-08-11  1:58     ` liu ping fan
2012-08-11  1:58       ` [Qemu-devel] " liu ping fan
2012-08-11 10:06       ` liu ping fan
2012-08-11 10:06         ` [Qemu-devel] " liu ping fan
2012-08-08 19:23   ` Blue Swirl
2012-08-08 19:23     ` [Qemu-devel] " Blue Swirl
2012-08-09  7:29     ` liu ping fan
2012-08-09  7:29       ` [Qemu-devel] " liu ping fan
2012-08-08  6:25 ` [PATCH 10/15] memory: change tcg related code to using PhysMap Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  6:25 ` [PATCH 11/15] lock: introduce global lock for device tree Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:41   ` Paolo Bonzini
2012-08-08  9:41     ` [Qemu-devel] " Paolo Bonzini
2012-08-09  7:28     ` liu ping fan
2012-08-09  7:28       ` [Qemu-devel] " liu ping fan
2012-08-09  7:41       ` Paolo Bonzini
2012-08-09  7:41         ` [Qemu-devel] " Paolo Bonzini
2012-08-08  9:42   ` Avi Kivity
2012-08-08  9:42     ` [Qemu-devel] " Avi Kivity
2012-08-09  7:27     ` liu ping fan
2012-08-09  7:27       ` [Qemu-devel] " liu ping fan
2012-08-09  8:31       ` Avi Kivity
2012-08-09  8:31         ` [Qemu-devel] " Avi Kivity
2012-08-08  6:25 ` [PATCH 12/15] qdev: using devtree lock to protect device's accessing Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:33   ` Peter Maydell
2012-08-08  9:33     ` [Qemu-devel] " Peter Maydell
2012-08-08  6:25 ` [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:52   ` Paolo Bonzini
2012-08-08  9:52     ` [Qemu-devel] " Paolo Bonzini
2012-08-08 10:07     ` Avi Kivity
2012-08-08 10:07       ` [Qemu-devel] " Avi Kivity
2012-08-09  7:28     ` liu ping fan
2012-08-09  7:28       ` [Qemu-devel] " liu ping fan
2012-08-09  8:00       ` Paolo Bonzini
2012-08-09  8:00         ` [Qemu-devel] " Paolo Bonzini
2012-08-10  6:42         ` liu ping fan
2012-08-10  6:42           ` [Qemu-devel] " liu ping fan
2012-08-13 18:53           ` Marcelo Tosatti
2012-08-13 18:53             ` [Qemu-devel] " Marcelo Tosatti
2012-08-13 18:51         ` Marcelo Tosatti
2012-08-13 18:51           ` [Qemu-devel] " Marcelo Tosatti
2012-08-08  6:25 ` [PATCH 14/15] qom: object_unref call reclaimer Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:40   ` Paolo Bonzini
2012-08-08  9:40     ` [Qemu-devel] " Paolo Bonzini
2012-08-13 18:56   ` Marcelo Tosatti
2012-08-13 18:56     ` [Qemu-devel] " Marcelo Tosatti
2012-08-08  6:25 ` [PATCH 15/15] e1000: using new interface--unmap to unplug Liu Ping Fan
2012-08-08  6:25   ` [Qemu-devel] " Liu Ping Fan
2012-08-08  9:56   ` Paolo Bonzini
2012-08-08  9:56     ` [Qemu-devel] " Paolo Bonzini
2012-08-09  7:28     ` liu ping fan
2012-08-09  7:28       ` [Qemu-devel] " liu ping fan
2012-08-09  7:40       ` Paolo Bonzini
2012-08-09  7:40         ` [Qemu-devel] " Paolo Bonzini
2012-08-10  6:43         ` liu ping fan
2012-08-10  6:43           ` [Qemu-devel] " liu ping fan

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=5022671E.8070602@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=afaerber@suse.de \
    --cc=anthony@codemonkey.ws \
    --cc=avi@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=jan.kiszka@siemens.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemulist@gmail.com \
    --cc=stefanha@gmail.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.