[alsa-devel] [PATCH] alsa-lib:Make snd_atomic_write_* truly atomic

[Ricard Wanderlof]

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
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30