From mboxrd@z Thu Jan  1 00:00:00 1970
From: pbonzini@redhat.com (Paolo Bonzini)
Date: Fri, 12 Nov 2010 15:39:17 +0100
Subject: [ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or
In-Reply-To: <20101112122939.GA30826@Krystal>
References: <1289552247-18130-1-git-send-email-pbonzini@redhat.com>
	<1289552247-18130-3-git-send-email-pbonzini@redhat.com>
	<20101112120846.GB27929@Krystal> <4CDD2F69.7020407@redhat.com>
	<20101112121900.GA29642@Krystal> <4CDD31E7.1050500@redhat.com>
	<20101112122939.GA30826@Krystal>
Message-ID: <4CDD5195.4010308@redhat.com>

On 11/12/2010 01:29 PM, Mathieu Desnoyers wrote:
> * Paolo Bonzini (pbonzini at redhat.com) wrote:
>> On 11/12/2010 01:19 PM, Mathieu Desnoyers wrote:
>>> * Paolo Bonzini (pbonzini at redhat.com) wrote:
>>>> Return nothing can be useful because it can be optimized on x86
>>>> as "lock orl (mem), reg/imm".  However, there are no other
>>>> return-nothing atomic ops in uatomic_*.h so I decided not to
>>>> provide this.
>>>>
>>>
>>> How about we start by implementing uatomic_and/uatomic_or that
>>> return void, and if we ever need
>>> uatomic_return_and/uatomic_return_or (see my other mail), we add
>>> them ?
>>
>> I am using uatomic_or's return value in my call_rcu-with-futex, but
>> I don't need atomic-or-and-exchange really, I can even change it to
>> a load followed by an atomic or (it introduces a race but it is
>> benign).
>
> If the race does not matter, why do we need the new atomic op in the
> first place ? ;)

Say the "correct" code would be

    atomic x = *p, *p |= FLAG1;   # uatomic_xchg_or
    if (x & ~FLAG2)
        futex_wake (...)

I may try to rewrite it like this:

    x = *p;                       # 1
    atomic *p |= FLAG1;           # 2, uatomic_or
    if (x & ~FLAG2)
        futex_wake (...)

This is racy because something can set or reset FLAG2 between 1 and 2. 
Setting it may cause spurious wakeups, which are okay.  If I know that 
nothing can reset FLAG2 between 1 and 2, correctness is preserved.

Instead if I remove atomicity, I get races on read-modify-write 
operations, and that is much more obviously incorrect.

But I really do not like being tricky (futexes are tricky enough on 
their own), and rather than spending a lot of time proving this correct 
I'd probably just use cmpxchg here.

Paolo