[pulseaudio-discuss] Atomic operations on ARM

[Lennart Poettering]

On Fri, 01.02.08 15:22, Jyri Sarha (lepbtetfmvbz at spammotel.com) wrote:

Hi!

> The strexeq-instruction has two conditions the equality to the old value 
> and the exclusiveness of the operation (e.g. if the value in memory was 
> tampered between the operations). The operation fails if either of
> these conditions fail, e.g. the value in memory is unchanged. So it is 
> possible that the old-value-condition is met, but the exclusiveness- 
> condition fails, but even the tampered memory value would meet the 
> old-value-condition.

But are both these failures detected and is it possible to tell them
apart? If so it would only be natural to call these instructions in a
loop. I mean, we're not really doing wait-free stuff here anyway,
normal lock-free stuff is all I am in for.

This loop should normally not spin for more than one or maybe two
times, so I don't see much problem in doing this.

> The above applies also to kernel helper implementation of atomic exchange 
> for ARM6 and above.
> 
> Because of the above problem (I suspect) this assertion in pulsecore/async.c 
> fails sometimes under heavy load:
>     /* Guaranteed to succeed if we only have a single reader */
>     pa_assert_se(pa_atomic_ptr_cmpxchg(&cells[idx], ret, NULL));
> 
> The assertion failure has happened with the both kernel helper and inline asm 
> versions (they are identical in ARM6 environment anyway). The failures
> are not very common thou.

Hmm, this queue should only have a single writer and a single reader
thread. Thus it seems that a write (push) access to the queue causes
this read (pop) to fail. Which is a big prolem I would say.

> The atomic compare and exchange can also be written in a way that it retries
> the operation if the exclusiveness-condition fails but the equality-condition
> was ok, which would resemble real atomicity more. The inline assembler version
> would then look like this:

Yes, this looks like what I want. Why do you say "resemble more" and
not "is identical to"? The only drawback is that we cannot do
wait-free any more, only normal lock-free, right? Or did I miss
anything?

> A similar kind of external loop can also be added to kernel helper
> function, but if the kernel helper in fact makes a systemcall it is
> unnecessary. I wonder if all this is worth the trouble. 

All code I saw that makes use of the kernel helper function calls it
in a loop. (At least that's what I remember)

> So what should be done?
> 
> 1. Change the above line in pulsecore/async.c to use pa_atomic_store
> instead and try to look if there are other similar places.
> 
> 2. Write loops like above to ARM specific implementations atomic
> compare and exchange.

I'd certainly vote for #2. I see no real drawbacks on this. 

> Any way I'll produce a proper ARM atomic ops patch as soon as I am happy 
> with it. However it may take a while because I am still only learning
> the autoconf magic and I have some other tasks I should take care of too.

Every patch greatly appreciated!

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net         ICQ# 11060553
http://0pointer.net/lennart/           GnuPG 0x1A015CC4