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)
> > > + kfree(card);
> > > return 0;
> > > }
> > > @@ -539,6 +607,9 @@ int snd_card_free(struct snd_card *card)
> > > DECLARE_COMPLETION_ONSTACK(released);
> > > int ret;
> > > + 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
