All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Glauber Costa <glommer@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>,
	mtosatti@redhat.com, kvm@vger.kernel.org
Subject: Re: [PATCH kvm-unit-test 2/6] Introduce atomic operations
Date: Sun, 29 Aug 2010 12:39:46 +0300	[thread overview]
Message-ID: <4C7A2AE2.9010800@redhat.com> (raw)
In-Reply-To: <20100827113930.GN2985@mothafucka.localdomain>

  On 08/27/2010 02:39 PM, Glauber Costa wrote:
> On Fri, Aug 27, 2010 at 01:49:20PM +0800, Jason Wang wrote:
>> +u64 atomic64_cmpxchg(atomic64_t *v, u64 old, u64 new)
>> +{
>> +        u64 ret;
>> +        u64 _old = old;
>> +        u64 _new = new;
>> +
>> +        asm volatile("lock cmpxchgq %1,%2"
>> +                     : "=a" (ret)
>> +                     : "r" (_new),
>> +                       "m" (*(volatile long *)&v->counter), "0"(_old)
>> +                     : "memory"
>> +                     );
>> +        return ret;
>> +}
>> +
> This is wrong.
> See http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=commitdiff;h=113fc5a6e8c2288619ff7e8187a6f556b7e0d372
>
> you need to explicitly mark you are changing the memory, here.
>
> Btw, this is mostly header copying, and can miss bug fixes like the one above. Isn't there
> a way to just specify in the test they are needed, and then grab them from linux automatically?

That may pull in dependencies.  I'd prefer using builtins like 
__sync_val_compare_and_swap() (but many atomics are not available).  Is 
there some library we can use for this?

-- 
error compiling committee.c: too many arguments to function


  reply	other threads:[~2010-08-29  9:39 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-27  5:49 [PATCH kvm-unit-test 0/6] Kvmclock test Jason Wang
2010-08-27  5:49 ` [PATCH kvm-unit-test 1/6] Introduce memory barriers Jason Wang
2010-08-27  5:49 ` [PATCH kvm-unit-test 2/6] Introduce atomic operations Jason Wang
2010-08-27 11:39   ` Glauber Costa
2010-08-29  9:39     ` Avi Kivity [this message]
2010-08-27  5:49 ` [PATCH kvm-unit-test 3/6] Export tsc related helpers Jason Wang
2010-08-27  5:49 ` [PATCH kvm-unit-test 4/6] Introduce atol() Jason Wang
2010-08-27  5:49 ` [PATCH kvm-unit-test 5/6] Add a simple kvmclock driver Jason Wang
2010-08-27 11:31   ` Glauber Costa
2010-08-27  5:49 ` [PATCH kvm-unit-test 6/6] Add a test for kvm-clock Jason Wang
2010-08-27 11:27   ` Glauber Costa
2010-08-30  3:07     ` Jason Wang
2010-08-27 11:34   ` Glauber Costa
2010-08-30  3:27     ` Jason Wang
2010-08-28  1:58   ` Zachary Amsden

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=4C7A2AE2.9010800@redhat.com \
    --to=avi@redhat.com \
    --cc=glommer@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@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.