| Message ID | 1421795250-7528-1-git-send-email-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
At Wed, 21 Jan 2015 08:07:30 +0900, Takashi Sakamoto wrote: > > Unplugging devices can cause disorder by releasing streaming resources > before stopping playbacking/capturing substreams. This commit forces > the state of ALSA character devices into disconnect before releasing > the resources. This isn't enough, unfortunately. snd_card_disconnect() itself doesn't sync, merely disconnects and stops the streams, etc, thus the file handles may be still opened. In general, releasing the resources should be done in either in card's private_free or snd_device's free callback where all files have been already released. For example, bebob_remove() should just call snd_card_free_when_closed() (that invokes snd_card_disconnect() internally) and call the reset (kfree() and *_destroy()) in a card's free callback. Takashi > > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> > --- > sound/firewire/bebob/bebob.c | 3 ++- > sound/firewire/fireworks/fireworks.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c > index fc19c99..41eb1ce 100644 > --- a/sound/firewire/bebob/bebob.c > +++ b/sound/firewire/bebob/bebob.c > @@ -306,10 +306,11 @@ static void bebob_remove(struct fw_unit *unit) > if (bebob == NULL) > return; > > + snd_card_disconnect(bebob->card); > + > kfree(bebob->maudio_special_quirk); > > snd_bebob_stream_destroy_duplex(bebob); > - snd_card_disconnect(bebob->card); > snd_card_free_when_closed(bebob->card); > } > > diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c > index 3e2ed8e..21fb50f 100644 > --- a/sound/firewire/fireworks/fireworks.c > +++ b/sound/firewire/fireworks/fireworks.c > @@ -289,10 +289,11 @@ static void efw_remove(struct fw_unit *unit) > { > struct snd_efw *efw = dev_get_drvdata(&unit->device); > > + snd_card_disconnect(efw->card); > + > snd_efw_stream_destroy_duplex(efw); > snd_efw_transaction_remove_instance(efw); > > - snd_card_disconnect(efw->card); > snd_card_free_when_closed(efw->card); > } > > -- > 2.1.0 >
On Jan 21 2015 16:10, Takashi Iwai wrote: > At Wed, 21 Jan 2015 08:07:30 +0900, > Takashi Sakamoto wrote: >> >> Unplugging devices can cause disorder by releasing streaming resources >> before stopping playbacking/capturing substreams. This commit forces >> the state of ALSA character devices into disconnect before releasing >> the resources. > > This isn't enough, unfortunately. snd_card_disconnect() itself > doesn't sync, merely disconnects and stops the streams, etc, thus the > file handles may be still opened. Oh, exactly. As you said, the drivers can't force applications to call snd_pcm_close() even if calling snd_card_disconnect(). As a result, any AMDTP streams are still running. > In general, releasing the resources should be done in either in card's > private_free or snd_device's free callback where all files have been > already released. For example, bebob_remove() should just call > snd_card_free_when_closed() (that invokes snd_card_disconnect() > internally) and call the reset (kfree() and *_destroy()) in a card's > free callback. Thanks for your indication. I should move these releasing code to .private_free callback. Currently I'm bothered by these drivers about kernel oops at unplugging during playbacking/capturing (not always). I guess: 1. unplugging. 2. isochronous communications stops, then the drivers detect packet discontinuity and set XRUN state to PCM substream immediately. 3. PCM applications call .prepare to recover XRUN state. 4. these drivers try transactions in .prepare 5. (Asynchronously) xxx_remove() is called by bus driver and release stream/transaction resources 6. the trial of transaction causes oops. This patch is not a better solution to this, too. Please abandon it... Thanks Takashi Sakamoto >> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> >> --- >> sound/firewire/bebob/bebob.c | 3 ++- >> sound/firewire/fireworks/fireworks.c | 3 ++- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c >> index fc19c99..41eb1ce 100644 >> --- a/sound/firewire/bebob/bebob.c >> +++ b/sound/firewire/bebob/bebob.c >> @@ -306,10 +306,11 @@ static void bebob_remove(struct fw_unit *unit) >> if (bebob == NULL) >> return; >> >> + snd_card_disconnect(bebob->card); >> + >> kfree(bebob->maudio_special_quirk); >> >> snd_bebob_stream_destroy_duplex(bebob); >> - snd_card_disconnect(bebob->card); >> snd_card_free_when_closed(bebob->card); >> } >> >> diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c >> index 3e2ed8e..21fb50f 100644 >> --- a/sound/firewire/fireworks/fireworks.c >> +++ b/sound/firewire/fireworks/fireworks.c >> @@ -289,10 +289,11 @@ static void efw_remove(struct fw_unit *unit) >> { >> struct snd_efw *efw = dev_get_drvdata(&unit->device); >> >> + snd_card_disconnect(efw->card); >> + >> snd_efw_stream_destroy_duplex(efw); >> snd_efw_transaction_remove_instance(efw); >> >> - snd_card_disconnect(efw->card); >> snd_card_free_when_closed(efw->card); >> } >> >> -- >> 2.1.0
diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c index fc19c99..41eb1ce 100644 --- a/sound/firewire/bebob/bebob.c +++ b/sound/firewire/bebob/bebob.c @@ -306,10 +306,11 @@ static void bebob_remove(struct fw_unit *unit) if (bebob == NULL) return; + snd_card_disconnect(bebob->card); + kfree(bebob->maudio_special_quirk); snd_bebob_stream_destroy_duplex(bebob); - snd_card_disconnect(bebob->card); snd_card_free_when_closed(bebob->card); } diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c index 3e2ed8e..21fb50f 100644 --- a/sound/firewire/fireworks/fireworks.c +++ b/sound/firewire/fireworks/fireworks.c @@ -289,10 +289,11 @@ static void efw_remove(struct fw_unit *unit) { struct snd_efw *efw = dev_get_drvdata(&unit->device); + snd_card_disconnect(efw->card); + snd_efw_stream_destroy_duplex(efw); snd_efw_transaction_remove_instance(efw); - snd_card_disconnect(efw->card); snd_card_free_when_closed(efw->card); }
Unplugging devices can cause disorder by releasing streaming resources before stopping playbacking/capturing substreams. This commit forces the state of ALSA character devices into disconnect before releasing the resources. Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- sound/firewire/bebob/bebob.c | 3 ++- sound/firewire/fireworks/fireworks.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)