On Tue, 13 Jul 2021 18:05:34 +0200, Takashi Iwai wrote:
On Tue, 13 Jul 2021 17:23:02 +0200, Amadeusz SX2awiX4ski wrote:
On 7/13/2021 4:28 PM, Takashi Iwai wrote:
/**
- snd_card_ref - Get the card object from the index
@@ -481,6 +547,7 @@ EXPORT_SYMBOL_GPL(snd_card_disconnect_sync); static int snd_card_do_free(struct snd_card *card) {
- card->releasing = true; #if IS_ENABLED(CONFIG_SND_MIXER_OSS) if (snd_mixer_oss_notify_callback) snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_FREE);
@@ -498,7 +565,8 @@ static int snd_card_do_free(struct snd_card *card) #endif if (card->release_completion) complete(card->release_completion);
- kfree(card);
- if (!card->managed)
return 0; } @@ -539,6 +607,9 @@ int snd_card_free(struct snd_card *card) DECLARE_COMPLETION_ONSTACK(released); int ret;kfree(card);
- if (card->releasing)
return 0;
"card->releasing" use feels bit racy to me... something like below would break it?
thread1 thread2 snd_card_free() if(card->releasing) == false thread1 goes sleep snd_card_do_free() card->releasing = true run until the end thread1 resume continues with trying to release
It's a destructor and can't be called in parallel.
That is, what the code above cares is the case where snd_card_free() is called explicitly even if the card is created with devres. So the check of card->releasing could be __snd_card_release() instead in snd_card_free(), too.
Takashi