On Thu, 4 Jun 2009, Jaroslav Kysela wrote:
On Wed, 3 Jun 2009, Takashi Iwai wrote:
At Wed, 3 Jun 2009 20:43:29 +0100, Mark Brown wrote:
The introduction of snd_card_set_id() added a lock on the card list to the old choose_default_id() function when using it to implement the new API call. This lock is needed to allow us to walk the list and check to see if our new name is a duplicate. Unfortunately this causes a lockup when called from snd_card_register() (in cases where no ID is supplied for the card) since the card list is already locked there.
Oh, thanks for catching this. Obviously I haven't tested the latest code enough...
Yes, thanks.
Fix this fairly hideously by factoring out the implementation and using a flag to indicate if the lock should be held. A better fix would probably be to refactor snd_card_register() to move the _set_id() outside the locking region but I can't immediately see anything I can convince myself is safe.
Agreed, there can be a cleaner solution but your fix is definitely safe. So I applied it as a quick fix.
The patch has a unlock problem in 'if (snd_cards[i] && !strcmp(snd_cards[i]->id, id)) {' block.
Anyway, here is next cleanup which will remove locking outside the internal id function and fixes also next possible race in unique id allocation:
http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=13d55b5cb27a3c...
Oops. There was wrong "if (card->id[0] != '\0')" check so the call in snd_card_register() will not work as expected. This commit is final:
http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=c5313f60c4a50d...
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.