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

[Ricard Wanderlof]

On Fri, 13 May 2016, Takashi Iwai wrote:

> > > Yeah, that's why I stated the primary question.  The atomic operation
> > > there is mostly cosmetic, not really helpful.  In the whole other
> > > operation, we don't guarantee the thread safety at all.  So,
> > > implementing the atomic op only in a very small part of the whole code
> > > appears like a superfluous optimization to me.
> > 
> > The question is, if we decide to remove this atomic stuff, how do we 
> > verify that it actually causes no problems, on all architectures, etc?
> 
> It's not what one can show in 100%, as you know.  And, it's the reason
> why the code is still left over years...

We have come up upon this problem in conjunction with GStreamer. It 
appears that GStreamer uses one thread for the streaming and a separate 
one for the device management (open/close) on the same device handle, 
which triggers the problem. So it appears that GStreamer falsly is 
assuming some form of thread safety here.

The atomic stuff is so specific to a couple of files, it seems as if it 
was added for a specific purpose, such as this particular case. In the 
other hand, looking at the git history of include/iatomic.h, it's gone 
through a number of cleanups over the years, lately mostly to remove 
originally buggy code, and what is remaining I guess like you say is just 
what nobody dared remove.

On the other hand, as the code stands, I can't really understand how it 
enforces atomicity. The read and write memory barriers just guarantee 
ordering, but there's nothing to stop a context switch from occurring in 
the middle of a ++ operation. wmb() and rmb() might influence timing 
though so that code at least might behave differently with those 
operations in place.

> IMO, we just need to make a clear statement about the thread safety of
> alsa-lib.  For a single threaded op, it can't give any regression, of
> course.

Just removing the memory barriers should have absolutely no effect in the 
single threaded case I would have thought, especially since there are no 
I/O or other hardware related things (such as DMA descriptors) being poked 
at.

/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