All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: liu ping fan <qemulist@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Stefan Hajnoczi <stefanha@gmail.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel@nongnu.org, Avi Kivity <avi@redhat.com>,
	Anthony Liguori <anthony@codemonkey.ws>,
	Jan Kiszka <jan.kiszka@siemens.com>
Subject: Re: [Qemu-devel] [PATCH v6 1/8] atomic: introduce atomic operations
Date: Thu, 15 Nov 2012 12:24:09 +0100	[thread overview]
Message-ID: <50A4D0D9.1060801@redhat.com> (raw)
In-Reply-To: <CAJnKYQ=S=y2_-Z=K9M1uV1MjgKO48q8HAJnAOsdU4gNZjym7iA@mail.gmail.com>

Il 15/11/2012 08:47, liu ping fan ha scritto:
>>>> RCU prototype required pointer-sized access, which you cannot make type-
>>> But I think that your RCU prototype should rely on atomic of CPU, not
>>> gcc‘s atomic.
>>
>> What's the difference?  gcc's atomic produces the same instructions as
>> hand-written assembly (or should).
>>
> Probably I made a mistake here, in vhost,  log =
> __sync_fetch_and_and(from, 0)  is used to fetch 64bits atomically in
> the case  32bits qemu running on 64bits linux.  Right?   But how can
> we read 32bits twice in atomic?

You can use cmpxchg8b.

>>> Otherwise, it could be slow (I guess something like spinlock there).
>>
>> Not sure what you mean.  I'm using two things: 1) write barriers; 2)
>> atomic_xchg of a pointer for fast, wait-free enqueuing of call_rcu
>> callbacks.
>
> Got your main idea. Will go through your prototype. Just one more
> question, why here atomic_xchg needed?  Could not the sequence "read
> old pointer, set new pointer" satisfy your requirement?

No, it would be racy.  Say you have this:

     wrong                     right
  -----------------------------------------
     ref(new)                  ref(new)
     old = tail                old = new
     tail = new                xchg(&old, &tail)
     old->next = new           old->next = new
     up(&sem)                  up(&sem)

where sem and tail are global, while old and new are local.  There can
be any number of producers as in the above scheme, and one consumer.  In
the consumer, iteration of the list starts from the second element (the
first element is dummy):

    dummy = new Node;
    tail = dummy;
    for(;;) {
        down(&sem);
        node = dummy->next;
        unref(dummy);
        visit node;
        dummy = node;
    }

Then the invariants are:

- the number of elements N in the list (starting from the dummy node and
following ->next) is always >1.  Because of this, you never have to
touch head when adding an element.

- the number of excess signals in the semaphore sem is always <= N-1.
Because of this, it doesn't matter that there is an instant in time
where tail is not reachable from head.  The element is really in the
list (as far as the consumer goes) only after the semaphore is signaled.

In the wrong version, two threads could read the same pointer:

    x = tail                                  |
                         y = tail             |
    tail = new_x                              |
                         tail = new_y         |
    x->next = new_x                           |   old_tail->next = new_x
    up(&sem)                                  |
                         y->next = new_y      |   old_tail->next = new_x
                         up(&sem)             |

No node points to new_x, so you have 2 nodes reachable from the head
(the dummy node, and new_y) with 2 signals on the semaphore,
contradicting the second invariant.

This instead works:

    x = new_x                                 |
                         y = new_y            |
    xchg(&x, &tail)                           |   tail = x, x = old_tail
                         xchg(&y, &tail)      |   tail = y, y = x
    x->next = new_x                           |   old_tail->next = new_x
    up(&sem)                                  |
                         y->next = new_y      |   x->next = new_y
                         up(&sem)             |

because you have 3 nodes and 2 signals.

Paolo

  reply	other threads:[~2012-11-15 11:24 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-05  5:38 [Qemu-devel] [PATCH v6 0/8] push mmio dispatch out of big lock Liu Ping Fan
2012-11-05  5:38 ` [Qemu-devel] [PATCH v6 1/8] atomic: introduce atomic operations Liu Ping Fan
2012-11-12  9:54   ` Paolo Bonzini
2012-11-13  6:48     ` liu ping fan
2012-11-13 10:11       ` Paolo Bonzini
2012-11-14  9:38         ` liu ping fan
2012-11-14  9:47           ` Paolo Bonzini
2012-11-15  7:47             ` liu ping fan
2012-11-15 11:24               ` Paolo Bonzini [this message]
2012-11-16  0:03               ` Richard Henderson
2012-11-21  5:58                 ` liu ping fan
2012-11-18 10:04               ` Avi Kivity
2012-11-21  5:57                 ` liu ping fan
2012-11-13 10:11       ` Paolo Bonzini
2012-11-05  5:38 ` [Qemu-devel] [PATCH v6 2/8] qom: apply atomic on object's refcount Liu Ping Fan
2012-11-05  5:38 ` [Qemu-devel] [PATCH v6 3/8] hotplug: introduce qdev_unplug_complete() to remove device from views Liu Ping Fan
2012-11-12  9:27   ` Paolo Bonzini
2012-11-13  6:12     ` liu ping fan
2012-11-05  5:38 ` [Qemu-devel] [PATCH v6 4/8] pci: remove pci device from mem view when unplug Liu Ping Fan
2012-11-05  5:38 ` [Qemu-devel] [PATCH v6 5/8] memory: introduce local lock for address space Liu Ping Fan
2012-11-05  5:38 ` [Qemu-devel] [PATCH v6 6/8] memory: make mmio dispatch able to be out of biglock Liu Ping Fan
2012-11-05  6:45   ` Jan Kiszka
2012-11-05  5:38 ` [Qemu-devel] [PATCH v6 7/8] memory: introduce tls context to trace nested mmio request issue Liu Ping Fan
2012-11-05  6:57   ` Jan Kiszka
2012-11-05  5:38 ` [Qemu-devel] [PATCH v6 8/8] vcpu: push mmio dispatcher out of big lock Liu Ping Fan
2012-11-05  7:00 ` [Qemu-devel] [PATCH v6 0/8] push mmio dispatch " Jan Kiszka
2012-11-09  6:23   ` liu ping fan
2012-11-09  8:15     ` Jan Kiszka

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=50A4D0D9.1060801@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=avi@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --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.