On Tue, 17 May 2016, Takashi Iwai wrote:
There is a manual lock loop with snd_atomic_read_wait(), as found in pcm_plugin.c. So, removing the stuff may potentially damage some apps that wrongly assume the thread safety.
Yes, I think could be true. Certainly GStreamer seems to assume this.
Now I've been considering this issue again, I'm inclined to suggest: let's begin with deprecating the atomic stuff there, but opt-in via via a configure option for a while, in case someone still really needs it.
By making it optional, it's fine to take your patch and cleanup the old code. Then we don't have to think of the compatibility with the old toolchain so much.
I'm trying to consider how this would be done in practice. Basically, add a configuration option like '--with-atomic' with the codec in iatomic.h looking something like:
static __inline__ void snd_atomic_write_begin(snd_atomic_write_t *w) { + #ifdef USE_ATOMIC + __atomic_add_fetch(&w->begin, 1,__ATOMIC_SEQ_CST); + #else - w->begin++; + #endif - wmb(); }
Thus removing the (useless?) wmb() in any case, and going to w->begin++ in the non-atomic case, and __atomic_add_fetch() in the atomic case?
However, this does mean that those that need/expect atomic operations must explicitly add a new configuration option, but perhaps that is ok, as one could argue that the applications that need it are buggy to start with.
Also, it would prohibit building the building of alsa-lib for such applications with gcc < 4.7 (when __atomic_add_fetch was introduced), but perhaps that's more of an incentive than anything else. :-)
/Ricard