On Thursday 04 November 2010 20:08:58 ext Mark Brown wrote:
Right, that's what I was thinking - I'm mostly concerned that we need to have the locking further up the stack rather than further down. This feels like a band aid which fixes the symptom (the list pointers getting corrupted) but it smells like we may have something further up the stack which can still create issues. For example, if both DAPM and something else manipulate the register map simultaneously we could get R/M/W issues.
Well, I think if we want to have one place to use a lock, this is the highest we can go. But I did figured out the reason for the crash. In my machine driver I had a kcontrol (ENUM_EXT) for muting the speaker. In the set callback I used snd_soc_dapm_enable_pin/snd_soc_dapm_disable_pin + snd_soc_dapm_sync. This is the place where it is crashing, since dapm_sync calls dapm_power_widgets directly without locking. All other places, when dapm_power_widgets called are protected by codec->mutex so those paths are safe. By replacing the custom kcontrol/callbacks with SOC_DAPM_PIN_SWITCH() the problem is gone. So it seams that my case can be solved easily with the SOC_DAPM_PIN_SWITCH(), but the race remains, and other setups might fail under some rare scenario
We can not add the codec->mutex to the snd_soc_dapm_sync, since it is called from soc_probe_dai_link, which is part of the initialization, and the codec-
mutex has been taken up in the chain.
It might be a good idea to go through the machine drivers, and check for snd_soc_dapm_sync usage.