On Tue, 17 May 2016 17:28:43 +0200, Ricard Wanderlof wrote: > > > 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)? Right, the current atomic ops usage is only covering the very light locking scheme that might casually work for some cases. But it never works for heavy use, and a more proper way of concurrent accesses would be needed in anyway. That's the reason I proposed to drop off these. This makes things looking safe although they are not really usable. But how dangerous is this move? I'm not 100% sure so far... Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
