Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
On Mon, Apr 03, 2017 at 10:37:42AM +0200, Takashi Iwai wrote: > Ah, I see that snd_soc_unregister_card() has the check of > card->instantiated, so it should be fine to call multiple times. Yeah, we'd run into that often enough that it's worth handling nicely. signature.asc Description: PGP signature
Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
On Fri, Mar 31, 2017 at 09:48:02AM +0200, Takashi Iwai wrote: > Mark Brown wrote: > > flag directly when we see a problem call a function to do it. That way > > if we want to improve things in the future we can do that without having > > to update the callers again. > BTW, ALSA core has snd_card_disconnect() that does this kind of > shut-up from user-space. It was introduced for hot-unplug, but > basically unbinding is the software hot-unplug. So, if ASoC won't > rebind a once-unbound component, you can simply call > snd_card_disconnect() at the component unbinding time to assure that > no further user actions can be done. Ah, that's exactly the sort of improvement I was thinking of! signature.asc Description: PGP signature
Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
On Mon, 03 Apr 2017 10:26:05 +0200, Kuninori Morimoto wrote: > > > Hi Takashi-san > > > > So, we can call snd_soc_unregister_card() whenever CPU/Codec/Platform > > > were unregsiterd. > > > > In theory yes, but you should be careful to do so, e.g. make sure that > > it won't be called again by the removal/unbind of other components / > > drivers. > > > > I suggested snd_card_disconnect() because it doesn't release resources > > by itself, but it just disconnects from the further accesses. So, > > double-free won't happen in this case. It makes the hotunplug safer > > as long as the drivers manage the resource releases properly. > > I had checked many unbind/bind pattern/order on 2nd [RFC] patch which I > posted. > At first, I believe Oops on unbind/bind issue was solved on it. > 2nd, if my understanding was correct, it doesn't have double-free issue, > or something like that. > But, I'm not 100% sure about 2nd, thus it has [RFC] on patch. Ah, I see that snd_soc_unregister_card() has the check of card->instantiated, so it should be fine to call multiple times. thanks, Takashi
Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
Hi Takashi-san > > So, we can call snd_soc_unregister_card() whenever CPU/Codec/Platform > > were unregsiterd. > > In theory yes, but you should be careful to do so, e.g. make sure that > it won't be called again by the removal/unbind of other components / > drivers. > > I suggested snd_card_disconnect() because it doesn't release resources > by itself, but it just disconnects from the further accesses. So, > double-free won't happen in this case. It makes the hotunplug safer > as long as the drivers manage the resource releases properly. I had checked many unbind/bind pattern/order on 2nd [RFC] patch which I posted. At first, I believe Oops on unbind/bind issue was solved on it. 2nd, if my understanding was correct, it doesn't have double-free issue, or something like that. But, I'm not 100% sure about 2nd, thus it has [RFC] on patch. Best regards --- Kuninori Morimoto
Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
On Mon, 03 Apr 2017 08:29:34 +0200, Kuninori Morimoto wrote: > > > Hi Takashi-san > > > > I think this is a good direction to at least start to mitigate these > > > problems (which we really should be doing) and hopefully make it easier > > > to do further improvements in future. There's obviously more places > > > where we should be checking the flag (controls for example) but they can > > > be added later. One thing I would like to see is instead of setting the > > > flag directly when we see a problem call a function to do it. That way > > > if we want to improve things in the future we can do that without having > > > to update the callers again. > > > > BTW, ALSA core has snd_card_disconnect() that does this kind of > > shut-up from user-space. It was introduced for hot-unplug, but > > basically unbinding is the software hot-unplug. So, if ASoC won't > > rebind a once-unbound component, you can simply call > > snd_card_disconnect() at the component unbinding time to assure that > > no further user actions can be done. > > Thanks. I checked about snd_card_disconnect(), and it will be called > from snd_card_free(). And it will be called from snd_soc_unregister_card() Yes, snd_card_free() assures the disconnection at first, syncs the all settled down, then releases the resources. > So, we can call snd_soc_unregister_card() whenever CPU/Codec/Platform > were unregsiterd. In theory yes, but you should be careful to do so, e.g. make sure that it won't be called again by the removal/unbind of other components / drivers. I suggested snd_card_disconnect() because it doesn't release resources by itself, but it just disconnects from the further accesses. So, double-free won't happen in this case. It makes the hotunplug safer as long as the drivers manage the resource releases properly. Takashi > This method also solve random unbind/bind Oops too. > Here, random unbind/bind example is that > expected correct operation is unbind all CPU/Codec/Platfrom/Card, > and then, bind all CPU/Codec/Platfrom/Card again. > (here unbind order can be random) > But this case, we will get Oops if unbind Codec -> bind Codec -> unbind Card. > Using snd_soc_unregister_card() can solve this issue too. > > Best regards > --- > Kuninori Morimoto >
Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
Hi Takashi-san > > I think this is a good direction to at least start to mitigate these > > problems (which we really should be doing) and hopefully make it easier > > to do further improvements in future. There's obviously more places > > where we should be checking the flag (controls for example) but they can > > be added later. One thing I would like to see is instead of setting the > > flag directly when we see a problem call a function to do it. That way > > if we want to improve things in the future we can do that without having > > to update the callers again. > > BTW, ALSA core has snd_card_disconnect() that does this kind of > shut-up from user-space. It was introduced for hot-unplug, but > basically unbinding is the software hot-unplug. So, if ASoC won't > rebind a once-unbound component, you can simply call > snd_card_disconnect() at the component unbinding time to assure that > no further user actions can be done. Thanks. I checked about snd_card_disconnect(), and it will be called from snd_card_free(). And it will be called from snd_soc_unregister_card() So, we can call snd_soc_unregister_card() whenever CPU/Codec/Platform were unregsiterd. This method also solve random unbind/bind Oops too. Here, random unbind/bind example is that expected correct operation is unbind all CPU/Codec/Platfrom/Card, and then, bind all CPU/Codec/Platfrom/Card again. (here unbind order can be random) But this case, we will get Oops if unbind Codec -> bind Codec -> unbind Card. Using snd_soc_unregister_card() can solve this issue too. Best regards --- Kuninori Morimoto
Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
On Thu, 30 Mar 2017 23:53:34 +0200, Mark Brown wrote: > > On Wed, Mar 29, 2017 at 02:45:37AM +, Kuninori Morimoto wrote: > > > To avoid Sound Card crash in (2) case, what we can do now is, add dirty > > flag on Sound Card, and avoid to open Sound Card. > > This patch solved this issue. > > I think this is a good direction to at least start to mitigate these > problems (which we really should be doing) and hopefully make it easier > to do further improvements in future. There's obviously more places > where we should be checking the flag (controls for example) but they can > be added later. One thing I would like to see is instead of setting the > flag directly when we see a problem call a function to do it. That way > if we want to improve things in the future we can do that without having > to update the callers again. BTW, ALSA core has snd_card_disconnect() that does this kind of shut-up from user-space. It was introduced for hot-unplug, but basically unbinding is the software hot-unplug. So, if ASoC won't rebind a once-unbound component, you can simply call snd_card_disconnect() at the component unbinding time to assure that no further user actions can be done. Takashi
