On Tue, 17 May 2016, Takashi Iwai wrote:
No, my proposal is to make all snd_atomic_*() NOP unless a configure option is passed.
Ok, I must honestly say I hadn't studied the actual pcm_plugin.c code in great detail before (I didn't create the patch but was in the train of discussion at the time). I had just assumed that the w->begin and w->end variables were some form of counters used outside the atomic functions, but I can see now that they are not.
Looking at it now it appears that all this atomic stuff is trying to accomplish is to avoid the (sole) read in snd_pcm_plugin_status() from happening during one of the many potential writes in the other functions in the file, and the only reason for that in turn seems to be to get the acceses to *pcm->appl.ptr and *pcm->hw.ptr as well as other things needed for the return snd_pcm_status_t* consistent.
But that in turn means that fixing the atomicity of the w->begin and w->end accesses as proposed in the patch just glosses over that particular implementation issue; if indeed something is calling the functions doing the writes concurrently, something else is bound to get screwed up, unless by chance the two calls touch different variables, or the timing is such that two things aren't touched at the same time.
Right now it feels a bit uncomfortable to fix it in this way. It's just luck if it doesn't die somewhere else. Or am I missing something (highly likely)?
/Ricard