Re: [Qemu-devel] [PATCH V3 01/11] atomic: introduce atomic operations

[Jamie Lokier]

liu ping fan wrote:
> >> +static inline void atomic_set(Atomic *v, int i)
> >> +{
> >> +    v->counter = i;
> >> +}

Hi,

When running on ARM Linux kernels prior to 2.6.32, userspace
atomic_set() needs to use "clrex" or "strex" too.

See Linux commit 200b812d, "Clear the exclusive monitor when returning
from an exception".

You can see ARM's atomic_set() used to use "strex", and warns it's
important.  The kernel patch allows atomic_set() to be simplified, and
that includes for userspace, by putting clrex/strex in the exception
return path instead.

However, someone may run QEMU on a kernel before 2.6.32, which isn't
that old.  (E.g. my phone is running 2.6.28).

Otherwise you can have this situation:

    Initially: a = 0.

    Thread
          atomic_inc(&a, 1)
          = ldrex, add, [strex interrupted]

                                 Interrupted by signal handler
                                      atomic_set(&a, 3)
                                      = str
                                 Signal return

    Resume thread
          = strex (succeeds because CPU-local exclusive-flag still set)

    Result: a = 1, should be impossible when the signal triggered, and
            information about the signal is lost.

A more realistic example would use atomic_compare_exchange(), to
atomic-read-and-clear, atomic-read-and-dec-if-not-zero a variable set
in a signal handler, however I've used atomic_inc() to illustrate
because that's in your patch.

Best,
-- Jamie