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