On Thu, 2009-10-01 at 13:01 +0200, Nurkkala Eero.An (EXT-Offcode/Oulu) wrote: > On Thu, 2009-10-01 at 12:37 +0200, ext Mark Brown wrote: > > On Thu, Oct 01, 2009 at 09:17:36AM +0300, ext-eero.nurkkala@xxxxxxxxx wrote: > > > From: Eero Nurkkala <ext-eero.nurkkala@xxxxxxxxx> > > > > > codec->mutex seems required if widget->* values > > > are altered. Moreover, snd_soc_dapm_put_volsw() > > > may alter widget->saved_value. dapm_set_pga() > > > uses widget->saved_value in a for loop which now > > > has a distant chance of getting out of control. > > > > Hrm. This doesn't look right. dapm_set_pga() is only called from > > within the DAPM power updates so we should've taken the codec mutex much > > further up the stack otherwise all the DAPM list walking and updating > > will have issues. Was this just from code inspection or are you running > > into actual issues? > > No, codex mutex is not taken further up the stack. > I ran lockdep for the case also. I also placed the mutexes > so that they're always taken in the same function. > > I still think the described scenario can happen. Or could > you point where the mutex is taken earlier? If it was, > I would've deadlocked every time....Maybe I'm missing > some info. > > BTW, what's the reasoning for codec mutex anyway? > > (Yes, it was just code inspection). > > - Eero Actually, in this case: (kernel 2.6.28) snd_soc_dapm_put_volsw() (mutex_lock(&widget->codec->mutex);) -> dapm_mixer_update_power() -> dapm_power_widgets() -> dapm_set_pga() and with this patch it would deadlock. (would try to take the mutex twice). But if it didn't come from snd_soc_dapm_put_volsw(), but instead: close_delayed_work() -> snd_soc_dapm_stream_event() -> dapm_power_widgets() -> dapm_set_pga() then the codec->mutex is not taken, and the bug presented in the patch is out there, no? Eg. if snd_soc_dapm_put_volsw() is called, it can alter widget->saved_value meanwhile damp_set_pga() is being using the same value in a for loop. So yes, my patch makes things worse, but this use of codec mutex doesn't appear very clear to me. (possibly everybody else knows it far better =) - Eero _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
