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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
-- 
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux