Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-15 Thread Mark Brown
On Thu, Jan 15, 2015 at 01:40:49PM +0900, jiwang wrote:

> Can we have a more generic fix to this issue?
> Or shall we set owner field for all machine drivers?

Ideally we should do both.


signature.asc
Description: Digital signature


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-15 Thread Mark Brown
On Thu, Jan 15, 2015 at 01:40:49PM +0900, jiwang wrote:

 Can we have a more generic fix to this issue?
 Or shall we set owner field for all machine drivers?

Ideally we should do both.


signature.asc
Description: Digital signature


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 16:34:15 +,
Mark Brown wrote:
> 
> On Wed, Jan 14, 2015 at 02:01:33PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > > Above all, disallowing the module unload while using is the common
> > > > behavior of any other drivers.  Why do we have to be a rebel against
> > > > all civil manner? :)
> 
> > > That's not true for everything
> 
> > Hmm, which driver does behave so intentionally?  I'm interested in the
> > supposed reason behind it.
> 
> Relatively few of the subsystems in drivers have references to
> module_get().

Time flys...  At the time of Linux 2.2 kernels, it was fairly common
to run a regular auto-cleanup of unused modules via cron running
"rmmod -a". Thus, each driver was mandated to deal with the module
refcount while used, or set it to -1 to avoid the auto-unload
permanently (like ipv6).

> > > and for ASoC I'd tend to assume that the
> > > user knows what they're doing and has a good reason for it; it's
> > > certainly something that can be helpful in development.
> 
> > The module unload is never considered to be equivalent with hot
> > unplug  It's more than that.
> 
> I'm not sure that's the case from a user perspective.

Unloading a module means to remove the functionality.  Unbinding is to
remove a device aka hotunplug.  Conceptually totally different.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Thu, 15 Jan 2015 13:40:49 +0900,
jiwang wrote:
> 
> Hi
> On 01/14/2015 10:43 PM, Fabio Estevam wrote:
> > On Wed, Jan 14, 2015 at 10:57 AM, Lars-Peter Clausen  
> > wrote:
> >
> >> My personal opinion on this is that disallowing module removal while a
> >> driver registered by the module when is in use, while there is no technical
> >> reason to do so, is a anti-feature. Whether in ALSA or elsewhere.
> >>
> >> But looking at the source it seems that this is a core feature of ALSA and
> >> at least for the card module itself it will do the ref-counting when a
> >> stream is started/stopped. And we even support setting the owner of a card
> >> in ASoC. It's just that pretty much no ASoC card driver bothers to set the
> >> owner field in the snd_soc_card struct. So this particular problem can be
> >> fixed by updating the imx-wm8962 driver to set the owner field.
> > Thanks, Lars_Peter. This fixes the issue:
> >
> > root@freescale /$ modprobe -r snd_soc_imx_wm8962
> > modprobe: can't unload module snd_soc_imx_wm8962: Resource temporarily
> > unavailable
> >
> > Will send a patch with your suggestion soon.
> I think by set owner field in imx_wm8962 machine driver can fix the 
> crash I saw on sabreSD board,
> but as this is a generic issue which I suppose should exist on other 
> boards with different
> machine drivers.
> 
> Can we have a more generic fix to this issue?
> Or shall we set owner field for all machine drivers?

This is two folds.  The lack of card's owner field is a bug of each
driver.  Another is the missing of a hotplug-safe cleanup in ASoC
core.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread jiwang

Hi
On 01/14/2015 10:43 PM, Fabio Estevam wrote:

On Wed, Jan 14, 2015 at 10:57 AM, Lars-Peter Clausen  wrote:


My personal opinion on this is that disallowing module removal while a
driver registered by the module when is in use, while there is no technical
reason to do so, is a anti-feature. Whether in ALSA or elsewhere.

But looking at the source it seems that this is a core feature of ALSA and
at least for the card module itself it will do the ref-counting when a
stream is started/stopped. And we even support setting the owner of a card
in ASoC. It's just that pretty much no ASoC card driver bothers to set the
owner field in the snd_soc_card struct. So this particular problem can be
fixed by updating the imx-wm8962 driver to set the owner field.

Thanks, Lars_Peter. This fixes the issue:

root@freescale /$ modprobe -r snd_soc_imx_wm8962
modprobe: can't unload module snd_soc_imx_wm8962: Resource temporarily
unavailable

Will send a patch with your suggestion soon.
I think by set owner field in imx_wm8962 machine driver can fix the 
crash I saw on sabreSD board,
but as this is a generic issue which I suppose should exist on other 
boards with different

machine drivers.

Can we have a more generic fix to this issue?
Or shall we set owner field for all machine drivers?


Thanks,
Jiada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Mark Brown
On Wed, Jan 14, 2015 at 02:01:33PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > > Above all, disallowing the module unload while using is the common
> > > behavior of any other drivers.  Why do we have to be a rebel against
> > > all civil manner? :)

> > That's not true for everything

> Hmm, which driver does behave so intentionally?  I'm interested in the
> supposed reason behind it.

Relatively few of the subsystems in drivers have references to
module_get().

> > and for ASoC I'd tend to assume that the
> > user knows what they're doing and has a good reason for it; it's
> > certainly something that can be helpful in development.

> The module unload is never considered to be equivalent with hot
> unplug  It's more than that.

I'm not sure that's the case from a user perspective.


signature.asc
Description: Digital signature


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Fabio Estevam
On Wed, Jan 14, 2015 at 10:57 AM, Lars-Peter Clausen  wrote:

> My personal opinion on this is that disallowing module removal while a
> driver registered by the module when is in use, while there is no technical
> reason to do so, is a anti-feature. Whether in ALSA or elsewhere.
>
> But looking at the source it seems that this is a core feature of ALSA and
> at least for the card module itself it will do the ref-counting when a
> stream is started/stopped. And we even support setting the owner of a card
> in ASoC. It's just that pretty much no ASoC card driver bothers to set the
> owner field in the snd_soc_card struct. So this particular problem can be
> fixed by updating the imx-wm8962 driver to set the owner field.

Thanks, Lars_Peter. This fixes the issue:

root@freescale /$ modprobe -r snd_soc_imx_wm8962
modprobe: can't unload module snd_soc_imx_wm8962: Resource temporarily
unavailable

Will send a patch with your suggestion soon.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 13:57:03 +0100,
Lars-Peter Clausen wrote:
> 
> On 01/14/2015 01:02 PM, Mark Brown wrote:
> [...]
> >>> I don't think that we need to prevent module unload when a stream is 
> >>> active.
> >>>   From a framework point of view is not different from hot-unplug. I don't
> >>> see a reason why we'd jump through hoops to actively forbid removing the
> >>> module once it works just fine.
> >
> >> Well, the module unload means a more drastic cleanup.  Even if you
> >> unbind, the code and data are still there while module unload may
> >> clean them up all.
> >
> >> Above all, disallowing the module unload while using is the common
> >> behavior of any other drivers.  Why do we have to be a rebel against
> >> all civil manner? :)
> >
> > That's not true for everything and for ASoC I'd tend to assume that the
> > user knows what they're doing and has a good reason for it; it's
> > certainly something that can be helpful in development.
> 
> 
> My personal opinion on this is that disallowing module removal while a 
> driver registered by the module when is in use, while there is no technical 
> reason to do so, is a anti-feature. Whether in ALSA or elsewhere.
> 
> But looking at the source it seems that this is a core feature of ALSA and 
> at least for the card module itself it will do the ref-counting when a 
> stream is started/stopped. And we even support setting the owner of a card 
> in ASoC. It's just that pretty much no ASoC card driver bothers to set the 
> owner field in the snd_soc_card struct. So this particular problem can be 
> fixed by updating the imx-wm8962 driver to set the owner field.

Right, and that's what I wanted to hear.  My concern was about the
missing piece in the existing core part.

The rest of hotplug fix is of course a thing to be done in anyway.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 12:02:28 +,
Mark Brown wrote:
> 
> On Wed, Jan 14, 2015 at 11:50:48AM +0100, Takashi Iwai wrote:
> > Lars-Peter Clausen wrote:
> 
> > > > OK, so it's not about active stream.  From the reporter's description,
> > > > I supposed that the module gets unloaded while playing a stream, which
> > > > shouldn't be allowed.
> 
> > > Well one of the ways to trigger this is to remove the module while the 
> > > stream is active. But it is not exclusively a problem module unload 
> > > problem. 
> > > E.g. the same happens if you hot-unplug the ASoC card.
> 
> > Yes, unbinding can trigger a similar problem, ends up the same bad
> > code path.
> 
> Right, which was my point - we need to be able to cope with the driver
> being removed while in use, unbind is just one path where this could
> happen and the issue isn't specific to unbind.
> 
> > > I don't think that we need to prevent module unload when a stream is 
> > > active. 
> > >  From a framework point of view is not different from hot-unplug. I don't 
> > > see a reason why we'd jump through hoops to actively forbid removing the 
> > > module once it works just fine.
> 
> > Well, the module unload means a more drastic cleanup.  Even if you
> > unbind, the code and data are still there while module unload may
> > clean them up all.
> 
> > Above all, disallowing the module unload while using is the common
> > behavior of any other drivers.  Why do we have to be a rebel against
> > all civil manner? :)
> 
> That's not true for everything

Hmm, which driver does behave so intentionally?  I'm interested in the
supposed reason behind it.

> and for ASoC I'd tend to assume that the
> user knows what they're doing and has a good reason for it; it's
> certainly something that can be helpful in development.

The module unload is never considered to be equivalent with hot
unplug  It's more than that.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Lars-Peter Clausen

On 01/14/2015 01:02 PM, Mark Brown wrote:
[...]

I don't think that we need to prevent module unload when a stream is active.
  From a framework point of view is not different from hot-unplug. I don't
see a reason why we'd jump through hoops to actively forbid removing the
module once it works just fine.



Well, the module unload means a more drastic cleanup.  Even if you
unbind, the code and data are still there while module unload may
clean them up all.



Above all, disallowing the module unload while using is the common
behavior of any other drivers.  Why do we have to be a rebel against
all civil manner? :)


That's not true for everything and for ASoC I'd tend to assume that the
user knows what they're doing and has a good reason for it; it's
certainly something that can be helpful in development.



My personal opinion on this is that disallowing module removal while a 
driver registered by the module when is in use, while there is no technical 
reason to do so, is a anti-feature. Whether in ALSA or elsewhere.


But looking at the source it seems that this is a core feature of ALSA and 
at least for the card module itself it will do the ref-counting when a 
stream is started/stopped. And we even support setting the owner of a card 
in ASoC. It's just that pretty much no ASoC card driver bothers to set the 
owner field in the snd_soc_card struct. So this particular problem can be 
fixed by updating the imx-wm8962 driver to set the owner field.


- Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Mark Brown
On Wed, Jan 14, 2015 at 11:50:48AM +0100, Takashi Iwai wrote:
> Lars-Peter Clausen wrote:

> > > OK, so it's not about active stream.  From the reporter's description,
> > > I supposed that the module gets unloaded while playing a stream, which
> > > shouldn't be allowed.

> > Well one of the ways to trigger this is to remove the module while the 
> > stream is active. But it is not exclusively a problem module unload 
> > problem. 
> > E.g. the same happens if you hot-unplug the ASoC card.

> Yes, unbinding can trigger a similar problem, ends up the same bad
> code path.

Right, which was my point - we need to be able to cope with the driver
being removed while in use, unbind is just one path where this could
happen and the issue isn't specific to unbind.

> > I don't think that we need to prevent module unload when a stream is 
> > active. 
> >  From a framework point of view is not different from hot-unplug. I don't 
> > see a reason why we'd jump through hoops to actively forbid removing the 
> > module once it works just fine.

> Well, the module unload means a more drastic cleanup.  Even if you
> unbind, the code and data are still there while module unload may
> clean them up all.

> Above all, disallowing the module unload while using is the common
> behavior of any other drivers.  Why do we have to be a rebel against
> all civil manner? :)

That's not true for everything and for ASoC I'd tend to assume that the
user knows what they're doing and has a good reason for it; it's
certainly something that can be helpful in development.


signature.asc
Description: Digital signature


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 11:00:47 +0100,
Lars-Peter Clausen wrote:
> 
> On 01/14/2015 09:47 AM, Takashi Iwai wrote:
> > At Wed, 14 Jan 2015 09:15:36 +0100,
> > Lars-Peter Clausen wrote:
> >>
> >> On 01/14/2015 08:43 AM, Takashi Iwai wrote:
> >>> At Tue, 13 Jan 2015 21:54:12 +,
> >>> Mark Brown wrote:
> 
>  On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:
> > Wang, Jiada (ESD) wrote:
> 
> >> I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, 
> >> wm8962 codec and SSI CPU DAI,
> 
> >> I got Kernel crash when unloading audio drivers (playback stream is 
> >> active)
> >> modprobe -r snd_soc_imx_wm8962
> >> modprobe -r snd_soc_fsl_ssi
> >> modprobe -r snd_soc_wm8962
> 
> > The root problem is that you can unload the module while playing.
> > The corresponding module refcounts should have been increased during
> > used.
> 
> > Do we miss [try_]module_get() somewhere in ASoC?
> 
>  That doesn't help, users can still forcibly unbind the driver at runtime
>  without loading the module - and there's always the potential for
>  actually hotpluggable hardware.  The teardown paths should be able to
>  cope somewhat gracefully.
> >>>
> >>> The module refcount has to be handled while being used for stopping
> >>> module unload.  That's irrelevant from the dynamic unbinding support
> >>> itself.  Of course, the module refcount doesn't save the world, but
> >>> it's the right fix for this particular scenario.
> >>
> >> Refcounting won't help in this case. The issue is caused by a delayed work
> >> item that gets launched when the PCM stream is stopped. So if you decrease
> >> the refcount when the stream is stopped you still have a window where it is
> >> possible to remove the module while the work is still being scheduled.
> >
> > OK, so it's not about active stream.  From the reporter's description,
> > I supposed that the module gets unloaded while playing a stream, which
> > shouldn't be allowed.
> 
> Well one of the ways to trigger this is to remove the module while the 
> stream is active. But it is not exclusively a problem module unload problem. 
> E.g. the same happens if you hot-unplug the ASoC card.

Yes, unbinding can trigger a similar problem, ends up the same bad
code path.

> I don't think that we need to prevent module unload when a stream is active. 
>  From a framework point of view is not different from hot-unplug. I don't 
> see a reason why we'd jump through hoops to actively forbid removing the 
> module once it works just fine.

Well, the module unload means a more drastic cleanup.  Even if you
unbind, the code and data are still there while module unload may
clean them up all.

Above all, disallowing the module unload while using is the common
behavior of any other drivers.  Why do we have to be a rebel against
all civil manner? :)


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Lars-Peter Clausen

On 01/14/2015 09:47 AM, Takashi Iwai wrote:

At Wed, 14 Jan 2015 09:15:36 +0100,
Lars-Peter Clausen wrote:


On 01/14/2015 08:43 AM, Takashi Iwai wrote:

At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:


On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:

Wang, Jiada (ESD) wrote:



I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, wm8962 
codec and SSI CPU DAI,



I got Kernel crash when unloading audio drivers (playback stream is active)
modprobe -r snd_soc_imx_wm8962
modprobe -r snd_soc_fsl_ssi
modprobe -r snd_soc_wm8962



The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.



Do we miss [try_]module_get() somewhere in ASoC?


That doesn't help, users can still forcibly unbind the driver at runtime
without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Refcounting won't help in this case. The issue is caused by a delayed work
item that gets launched when the PCM stream is stopped. So if you decrease
the refcount when the stream is stopped you still have a window where it is
possible to remove the module while the work is still being scheduled.


OK, so it's not about active stream.  From the reporter's description,
I supposed that the module gets unloaded while playing a stream, which
shouldn't be allowed.


Well one of the ways to trigger this is to remove the module while the 
stream is active. But it is not exclusively a problem module unload problem. 
E.g. the same happens if you hot-unplug the ASoC card.


I don't think that we need to prevent module unload when a stream is active. 
From a framework point of view is not different from hot-unplug. I don't 
see a reason why we'd jump through hoops to actively forbid removing the 
module once it works just fine.


- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Lars-Peter Clausen

On 01/14/2015 09:25 AM, jiwang wrote:

Hi

On 01/14/2015 05:15 PM, Lars-Peter Clausen wrote:

On 01/14/2015 08:43 AM, Takashi Iwai wrote:

At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:


On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:

Wang, Jiada (ESD) wrote:



I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver,
wm8962 codec and SSI CPU DAI,



I got Kernel crash when unloading audio drivers (playback stream is
active)
modprobe -r snd_soc_imx_wm8962
modprobe -r snd_soc_fsl_ssi
modprobe -r snd_soc_wm8962



The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.



Do we miss [try_]module_get() somewhere in ASoC?


That doesn't help, users can still forcibly unbind the driver at runtime
without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Refcounting won't help in this case. The issue is caused by a delayed work
item that gets launched when the PCM stream is stopped. So if you decrease
the refcount when the stream is stopped you still have a window where it
is possible to remove the module while the work is still being scheduled.

And while we do flush the scheduled work when we remove the ASoC card this
is done before snd_card_free() is called. So when snd_card_free() is
called it gets re-scheduled again. I think the correct fix is to add a
snd_card_disconnect() at the very top of soc_cleanup_card_resources().


when stream is active, snd_card_disconnect() will trigger pcm_close() be
executed by another thread,
we can't ensure the pcm_close() is executed before the rest of
soc_cleanup_card_resources().


Hm right, because that only gets called once the userspace application 
finally closes the PCM device. Takashi approach with moving things to the 
card_free callback might work better.


- Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 09:15:36 +0100,
Lars-Peter Clausen wrote:
> 
> On 01/14/2015 08:43 AM, Takashi Iwai wrote:
> > At Tue, 13 Jan 2015 21:54:12 +,
> > Mark Brown wrote:
> >>
> >> On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:
> >>> Wang, Jiada (ESD) wrote:
> >>
>  I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, 
>  wm8962 codec and SSI CPU DAI,
> >>
>  I got Kernel crash when unloading audio drivers (playback stream is 
>  active)
>  modprobe -r snd_soc_imx_wm8962
>  modprobe -r snd_soc_fsl_ssi
>  modprobe -r snd_soc_wm8962
> >>
> >>> The root problem is that you can unload the module while playing.
> >>> The corresponding module refcounts should have been increased during
> >>> used.
> >>
> >>> Do we miss [try_]module_get() somewhere in ASoC?
> >>
> >> That doesn't help, users can still forcibly unbind the driver at runtime
> >> without loading the module - and there's always the potential for
> >> actually hotpluggable hardware.  The teardown paths should be able to
> >> cope somewhat gracefully.
> >
> > The module refcount has to be handled while being used for stopping
> > module unload.  That's irrelevant from the dynamic unbinding support
> > itself.  Of course, the module refcount doesn't save the world, but
> > it's the right fix for this particular scenario.
> 
> Refcounting won't help in this case. The issue is caused by a delayed work 
> item that gets launched when the PCM stream is stopped. So if you decrease 
> the refcount when the stream is stopped you still have a window where it is 
> possible to remove the module while the work is still being scheduled.

OK, so it's not about active stream.  From the reporter's description,
I supposed that the module gets unloaded while playing a stream, which
shouldn't be allowed.

> And while we do flush the scheduled work when we remove the ASoC card this 
> is done before snd_card_free() is called. So when snd_card_free() is called 
> it gets re-scheduled again. I think the correct fix is to add a 
> snd_card_disconnect() at the very top of soc_cleanup_card_resources().

Or move the most code of soc_cleanup_card_resources() to
card->private_free or such to be called from snd_card_free(), and
snd_soc_unregister_card() just needs to call snd_card_free().
This will trigger the disconnection, settle down the device usages
then release the soc resources gracefully.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread jiwang

Hi

On 01/14/2015 05:15 PM, Lars-Peter Clausen wrote:

On 01/14/2015 08:43 AM, Takashi Iwai wrote:

At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:


On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:

Wang, Jiada (ESD) wrote:


I am using i.MX6Q sabreSD board, which have imx_wm892 machine 
driver, wm8962 codec and SSI CPU DAI,


I got Kernel crash when unloading audio drivers (playback stream 
is active)

modprobe -r snd_soc_imx_wm8962
modprobe -r snd_soc_fsl_ssi
modprobe -r snd_soc_wm8962



The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.



Do we miss [try_]module_get() somewhere in ASoC?


That doesn't help, users can still forcibly unbind the driver at 
runtime

without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Refcounting won't help in this case. The issue is caused by a delayed 
work item that gets launched when the PCM stream is stopped. So if you 
decrease the refcount when the stream is stopped you still have a 
window where it is possible to remove the module while the work is 
still being scheduled.


And while we do flush the scheduled work when we remove the ASoC card 
this is done before snd_card_free() is called. So when snd_card_free() 
is called it gets re-scheduled again. I think the correct fix is to 
add a snd_card_disconnect() at the very top of 
soc_cleanup_card_resources().


when stream is active, snd_card_disconnect() will trigger pcm_close() be 
executed by another thread,
we can't ensure the pcm_close() is executed before the rest of 
soc_cleanup_card_resources().


- Jiada


- Lars


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Lars-Peter Clausen

On 01/14/2015 08:43 AM, Takashi Iwai wrote:

At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:


On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:

Wang, Jiada (ESD) wrote:



I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, wm8962 
codec and SSI CPU DAI,



I got Kernel crash when unloading audio drivers (playback stream is active)
modprobe -r snd_soc_imx_wm8962
modprobe -r snd_soc_fsl_ssi
modprobe -r snd_soc_wm8962



The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.



Do we miss [try_]module_get() somewhere in ASoC?


That doesn't help, users can still forcibly unbind the driver at runtime
without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Refcounting won't help in this case. The issue is caused by a delayed work 
item that gets launched when the PCM stream is stopped. So if you decrease 
the refcount when the stream is stopped you still have a window where it is 
possible to remove the module while the work is still being scheduled.


And while we do flush the scheduled work when we remove the ASoC card this 
is done before snd_card_free() is called. So when snd_card_free() is called 
it gets re-scheduled again. I think the correct fix is to add a 
snd_card_disconnect() at the very top of soc_cleanup_card_resources().


- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Thu, 15 Jan 2015 13:40:49 +0900,
jiwang wrote:
 
 Hi
 On 01/14/2015 10:43 PM, Fabio Estevam wrote:
  On Wed, Jan 14, 2015 at 10:57 AM, Lars-Peter Clausen l...@metafoo.de 
  wrote:
 
  My personal opinion on this is that disallowing module removal while a
  driver registered by the module when is in use, while there is no technical
  reason to do so, is a anti-feature. Whether in ALSA or elsewhere.
 
  But looking at the source it seems that this is a core feature of ALSA and
  at least for the card module itself it will do the ref-counting when a
  stream is started/stopped. And we even support setting the owner of a card
  in ASoC. It's just that pretty much no ASoC card driver bothers to set the
  owner field in the snd_soc_card struct. So this particular problem can be
  fixed by updating the imx-wm8962 driver to set the owner field.
  Thanks, Lars_Peter. This fixes the issue:
 
  root@freescale /$ modprobe -r snd_soc_imx_wm8962
  modprobe: can't unload module snd_soc_imx_wm8962: Resource temporarily
  unavailable
 
  Will send a patch with your suggestion soon.
 I think by set owner field in imx_wm8962 machine driver can fix the 
 crash I saw on sabreSD board,
 but as this is a generic issue which I suppose should exist on other 
 boards with different
 machine drivers.
 
 Can we have a more generic fix to this issue?
 Or shall we set owner field for all machine drivers?

This is two folds.  The lack of card's owner field is a bug of each
driver.  Another is the missing of a hotplug-safe cleanup in ASoC
core.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread jiwang

Hi
On 01/14/2015 10:43 PM, Fabio Estevam wrote:

On Wed, Jan 14, 2015 at 10:57 AM, Lars-Peter Clausen l...@metafoo.de wrote:


My personal opinion on this is that disallowing module removal while a
driver registered by the module when is in use, while there is no technical
reason to do so, is a anti-feature. Whether in ALSA or elsewhere.

But looking at the source it seems that this is a core feature of ALSA and
at least for the card module itself it will do the ref-counting when a
stream is started/stopped. And we even support setting the owner of a card
in ASoC. It's just that pretty much no ASoC card driver bothers to set the
owner field in the snd_soc_card struct. So this particular problem can be
fixed by updating the imx-wm8962 driver to set the owner field.

Thanks, Lars_Peter. This fixes the issue:

root@freescale /$ modprobe -r snd_soc_imx_wm8962
modprobe: can't unload module snd_soc_imx_wm8962: Resource temporarily
unavailable

Will send a patch with your suggestion soon.
I think by set owner field in imx_wm8962 machine driver can fix the 
crash I saw on sabreSD board,
but as this is a generic issue which I suppose should exist on other 
boards with different

machine drivers.

Can we have a more generic fix to this issue?
Or shall we set owner field for all machine drivers?


Thanks,
Jiada
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 16:34:15 +,
Mark Brown wrote:
 
 On Wed, Jan 14, 2015 at 02:01:33PM +0100, Takashi Iwai wrote:
  Mark Brown wrote:
 
Above all, disallowing the module unload while using is the common
behavior of any other drivers.  Why do we have to be a rebel against
all civil manner? :)
 
   That's not true for everything
 
  Hmm, which driver does behave so intentionally?  I'm interested in the
  supposed reason behind it.
 
 Relatively few of the subsystems in drivers have references to
 module_get().

Time flys...  At the time of Linux 2.2 kernels, it was fairly common
to run a regular auto-cleanup of unused modules via cron running
rmmod -a. Thus, each driver was mandated to deal with the module
refcount while used, or set it to -1 to avoid the auto-unload
permanently (like ipv6).

   and for ASoC I'd tend to assume that the
   user knows what they're doing and has a good reason for it; it's
   certainly something that can be helpful in development.
 
  The module unload is never considered to be equivalent with hot
  unplug  It's more than that.
 
 I'm not sure that's the case from a user perspective.

Unloading a module means to remove the functionality.  Unbinding is to
remove a device aka hotunplug.  Conceptually totally different.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Mark Brown
On Wed, Jan 14, 2015 at 02:01:33PM +0100, Takashi Iwai wrote:
 Mark Brown wrote:

   Above all, disallowing the module unload while using is the common
   behavior of any other drivers.  Why do we have to be a rebel against
   all civil manner? :)

  That's not true for everything

 Hmm, which driver does behave so intentionally?  I'm interested in the
 supposed reason behind it.

Relatively few of the subsystems in drivers have references to
module_get().

  and for ASoC I'd tend to assume that the
  user knows what they're doing and has a good reason for it; it's
  certainly something that can be helpful in development.

 The module unload is never considered to be equivalent with hot
 unplug  It's more than that.

I'm not sure that's the case from a user perspective.


signature.asc
Description: Digital signature


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Lars-Peter Clausen

On 01/14/2015 08:43 AM, Takashi Iwai wrote:

At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:


On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:

Wang, Jiada (ESD) wrote:



I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, wm8962 
codec and SSI CPU DAI,



I got Kernel crash when unloading audio drivers (playback stream is active)
modprobe -r snd_soc_imx_wm8962
modprobe -r snd_soc_fsl_ssi
modprobe -r snd_soc_wm8962



The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.



Do we miss [try_]module_get() somewhere in ASoC?


That doesn't help, users can still forcibly unbind the driver at runtime
without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Refcounting won't help in this case. The issue is caused by a delayed work 
item that gets launched when the PCM stream is stopped. So if you decrease 
the refcount when the stream is stopped you still have a window where it is 
possible to remove the module while the work is still being scheduled.


And while we do flush the scheduled work when we remove the ASoC card this 
is done before snd_card_free() is called. So when snd_card_free() is called 
it gets re-scheduled again. I think the correct fix is to add a 
snd_card_disconnect() at the very top of soc_cleanup_card_resources().


- Lars
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread jiwang

Hi

On 01/14/2015 05:15 PM, Lars-Peter Clausen wrote:

On 01/14/2015 08:43 AM, Takashi Iwai wrote:

At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:


On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:

Wang, Jiada (ESD) wrote:


I am using i.MX6Q sabreSD board, which have imx_wm892 machine 
driver, wm8962 codec and SSI CPU DAI,


I got Kernel crash when unloading audio drivers (playback stream 
is active)

modprobe -r snd_soc_imx_wm8962
modprobe -r snd_soc_fsl_ssi
modprobe -r snd_soc_wm8962



The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.



Do we miss [try_]module_get() somewhere in ASoC?


That doesn't help, users can still forcibly unbind the driver at 
runtime

without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Refcounting won't help in this case. The issue is caused by a delayed 
work item that gets launched when the PCM stream is stopped. So if you 
decrease the refcount when the stream is stopped you still have a 
window where it is possible to remove the module while the work is 
still being scheduled.


And while we do flush the scheduled work when we remove the ASoC card 
this is done before snd_card_free() is called. So when snd_card_free() 
is called it gets re-scheduled again. I think the correct fix is to 
add a snd_card_disconnect() at the very top of 
soc_cleanup_card_resources().


when stream is active, snd_card_disconnect() will trigger pcm_close() be 
executed by another thread,
we can't ensure the pcm_close() is executed before the rest of 
soc_cleanup_card_resources().


- Jiada


- Lars


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 09:15:36 +0100,
Lars-Peter Clausen wrote:
 
 On 01/14/2015 08:43 AM, Takashi Iwai wrote:
  At Tue, 13 Jan 2015 21:54:12 +,
  Mark Brown wrote:
 
  On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:
  Wang, Jiada (ESD) wrote:
 
  I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, 
  wm8962 codec and SSI CPU DAI,
 
  I got Kernel crash when unloading audio drivers (playback stream is 
  active)
  modprobe -r snd_soc_imx_wm8962
  modprobe -r snd_soc_fsl_ssi
  modprobe -r snd_soc_wm8962
 
  The root problem is that you can unload the module while playing.
  The corresponding module refcounts should have been increased during
  used.
 
  Do we miss [try_]module_get() somewhere in ASoC?
 
  That doesn't help, users can still forcibly unbind the driver at runtime
  without loading the module - and there's always the potential for
  actually hotpluggable hardware.  The teardown paths should be able to
  cope somewhat gracefully.
 
  The module refcount has to be handled while being used for stopping
  module unload.  That's irrelevant from the dynamic unbinding support
  itself.  Of course, the module refcount doesn't save the world, but
  it's the right fix for this particular scenario.
 
 Refcounting won't help in this case. The issue is caused by a delayed work 
 item that gets launched when the PCM stream is stopped. So if you decrease 
 the refcount when the stream is stopped you still have a window where it is 
 possible to remove the module while the work is still being scheduled.

OK, so it's not about active stream.  From the reporter's description,
I supposed that the module gets unloaded while playing a stream, which
shouldn't be allowed.

 And while we do flush the scheduled work when we remove the ASoC card this 
 is done before snd_card_free() is called. So when snd_card_free() is called 
 it gets re-scheduled again. I think the correct fix is to add a 
 snd_card_disconnect() at the very top of soc_cleanup_card_resources().

Or move the most code of soc_cleanup_card_resources() to
card-private_free or such to be called from snd_card_free(), and
snd_soc_unregister_card() just needs to call snd_card_free().
This will trigger the disconnection, settle down the device usages
then release the soc resources gracefully.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Lars-Peter Clausen

On 01/14/2015 09:25 AM, jiwang wrote:

Hi

On 01/14/2015 05:15 PM, Lars-Peter Clausen wrote:

On 01/14/2015 08:43 AM, Takashi Iwai wrote:

At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:


On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:

Wang, Jiada (ESD) wrote:



I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver,
wm8962 codec and SSI CPU DAI,



I got Kernel crash when unloading audio drivers (playback stream is
active)
modprobe -r snd_soc_imx_wm8962
modprobe -r snd_soc_fsl_ssi
modprobe -r snd_soc_wm8962



The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.



Do we miss [try_]module_get() somewhere in ASoC?


That doesn't help, users can still forcibly unbind the driver at runtime
without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Refcounting won't help in this case. The issue is caused by a delayed work
item that gets launched when the PCM stream is stopped. So if you decrease
the refcount when the stream is stopped you still have a window where it
is possible to remove the module while the work is still being scheduled.

And while we do flush the scheduled work when we remove the ASoC card this
is done before snd_card_free() is called. So when snd_card_free() is
called it gets re-scheduled again. I think the correct fix is to add a
snd_card_disconnect() at the very top of soc_cleanup_card_resources().


when stream is active, snd_card_disconnect() will trigger pcm_close() be
executed by another thread,
we can't ensure the pcm_close() is executed before the rest of
soc_cleanup_card_resources().


Hm right, because that only gets called once the userspace application 
finally closes the PCM device. Takashi approach with moving things to the 
card_free callback might work better.


- Lars

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 13:57:03 +0100,
Lars-Peter Clausen wrote:
 
 On 01/14/2015 01:02 PM, Mark Brown wrote:
 [...]
  I don't think that we need to prevent module unload when a stream is 
  active.
From a framework point of view is not different from hot-unplug. I don't
  see a reason why we'd jump through hoops to actively forbid removing the
  module once it works just fine.
 
  Well, the module unload means a more drastic cleanup.  Even if you
  unbind, the code and data are still there while module unload may
  clean them up all.
 
  Above all, disallowing the module unload while using is the common
  behavior of any other drivers.  Why do we have to be a rebel against
  all civil manner? :)
 
  That's not true for everything and for ASoC I'd tend to assume that the
  user knows what they're doing and has a good reason for it; it's
  certainly something that can be helpful in development.
 
 
 My personal opinion on this is that disallowing module removal while a 
 driver registered by the module when is in use, while there is no technical 
 reason to do so, is a anti-feature. Whether in ALSA or elsewhere.
 
 But looking at the source it seems that this is a core feature of ALSA and 
 at least for the card module itself it will do the ref-counting when a 
 stream is started/stopped. And we even support setting the owner of a card 
 in ASoC. It's just that pretty much no ASoC card driver bothers to set the 
 owner field in the snd_soc_card struct. So this particular problem can be 
 fixed by updating the imx-wm8962 driver to set the owner field.

Right, and that's what I wanted to hear.  My concern was about the
missing piece in the existing core part.

The rest of hotplug fix is of course a thing to be done in anyway.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Lars-Peter Clausen

On 01/14/2015 09:47 AM, Takashi Iwai wrote:

At Wed, 14 Jan 2015 09:15:36 +0100,
Lars-Peter Clausen wrote:


On 01/14/2015 08:43 AM, Takashi Iwai wrote:

At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:


On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:

Wang, Jiada (ESD) wrote:



I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, wm8962 
codec and SSI CPU DAI,



I got Kernel crash when unloading audio drivers (playback stream is active)
modprobe -r snd_soc_imx_wm8962
modprobe -r snd_soc_fsl_ssi
modprobe -r snd_soc_wm8962



The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.



Do we miss [try_]module_get() somewhere in ASoC?


That doesn't help, users can still forcibly unbind the driver at runtime
without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Refcounting won't help in this case. The issue is caused by a delayed work
item that gets launched when the PCM stream is stopped. So if you decrease
the refcount when the stream is stopped you still have a window where it is
possible to remove the module while the work is still being scheduled.


OK, so it's not about active stream.  From the reporter's description,
I supposed that the module gets unloaded while playing a stream, which
shouldn't be allowed.


Well one of the ways to trigger this is to remove the module while the 
stream is active. But it is not exclusively a problem module unload problem. 
E.g. the same happens if you hot-unplug the ASoC card.


I don't think that we need to prevent module unload when a stream is active. 
From a framework point of view is not different from hot-unplug. I don't 
see a reason why we'd jump through hoops to actively forbid removing the 
module once it works just fine.


- Lars
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Fabio Estevam
On Wed, Jan 14, 2015 at 10:57 AM, Lars-Peter Clausen l...@metafoo.de wrote:

 My personal opinion on this is that disallowing module removal while a
 driver registered by the module when is in use, while there is no technical
 reason to do so, is a anti-feature. Whether in ALSA or elsewhere.

 But looking at the source it seems that this is a core feature of ALSA and
 at least for the card module itself it will do the ref-counting when a
 stream is started/stopped. And we even support setting the owner of a card
 in ASoC. It's just that pretty much no ASoC card driver bothers to set the
 owner field in the snd_soc_card struct. So this particular problem can be
 fixed by updating the imx-wm8962 driver to set the owner field.

Thanks, Lars_Peter. This fixes the issue:

root@freescale /$ modprobe -r snd_soc_imx_wm8962
modprobe: can't unload module snd_soc_imx_wm8962: Resource temporarily
unavailable

Will send a patch with your suggestion soon.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 11:00:47 +0100,
Lars-Peter Clausen wrote:
 
 On 01/14/2015 09:47 AM, Takashi Iwai wrote:
  At Wed, 14 Jan 2015 09:15:36 +0100,
  Lars-Peter Clausen wrote:
 
  On 01/14/2015 08:43 AM, Takashi Iwai wrote:
  At Tue, 13 Jan 2015 21:54:12 +,
  Mark Brown wrote:
 
  On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:
  Wang, Jiada (ESD) wrote:
 
  I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, 
  wm8962 codec and SSI CPU DAI,
 
  I got Kernel crash when unloading audio drivers (playback stream is 
  active)
  modprobe -r snd_soc_imx_wm8962
  modprobe -r snd_soc_fsl_ssi
  modprobe -r snd_soc_wm8962
 
  The root problem is that you can unload the module while playing.
  The corresponding module refcounts should have been increased during
  used.
 
  Do we miss [try_]module_get() somewhere in ASoC?
 
  That doesn't help, users can still forcibly unbind the driver at runtime
  without loading the module - and there's always the potential for
  actually hotpluggable hardware.  The teardown paths should be able to
  cope somewhat gracefully.
 
  The module refcount has to be handled while being used for stopping
  module unload.  That's irrelevant from the dynamic unbinding support
  itself.  Of course, the module refcount doesn't save the world, but
  it's the right fix for this particular scenario.
 
  Refcounting won't help in this case. The issue is caused by a delayed work
  item that gets launched when the PCM stream is stopped. So if you decrease
  the refcount when the stream is stopped you still have a window where it is
  possible to remove the module while the work is still being scheduled.
 
  OK, so it's not about active stream.  From the reporter's description,
  I supposed that the module gets unloaded while playing a stream, which
  shouldn't be allowed.
 
 Well one of the ways to trigger this is to remove the module while the 
 stream is active. But it is not exclusively a problem module unload problem. 
 E.g. the same happens if you hot-unplug the ASoC card.

Yes, unbinding can trigger a similar problem, ends up the same bad
code path.

 I don't think that we need to prevent module unload when a stream is active. 
  From a framework point of view is not different from hot-unplug. I don't 
 see a reason why we'd jump through hoops to actively forbid removing the 
 module once it works just fine.

Well, the module unload means a more drastic cleanup.  Even if you
unbind, the code and data are still there while module unload may
clean them up all.

Above all, disallowing the module unload while using is the common
behavior of any other drivers.  Why do we have to be a rebel against
all civil manner? :)


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Mark Brown
On Wed, Jan 14, 2015 at 11:50:48AM +0100, Takashi Iwai wrote:
 Lars-Peter Clausen wrote:

   OK, so it's not about active stream.  From the reporter's description,
   I supposed that the module gets unloaded while playing a stream, which
   shouldn't be allowed.

  Well one of the ways to trigger this is to remove the module while the 
  stream is active. But it is not exclusively a problem module unload 
  problem. 
  E.g. the same happens if you hot-unplug the ASoC card.

 Yes, unbinding can trigger a similar problem, ends up the same bad
 code path.

Right, which was my point - we need to be able to cope with the driver
being removed while in use, unbind is just one path where this could
happen and the issue isn't specific to unbind.

  I don't think that we need to prevent module unload when a stream is 
  active. 
   From a framework point of view is not different from hot-unplug. I don't 
  see a reason why we'd jump through hoops to actively forbid removing the 
  module once it works just fine.

 Well, the module unload means a more drastic cleanup.  Even if you
 unbind, the code and data are still there while module unload may
 clean them up all.

 Above all, disallowing the module unload while using is the common
 behavior of any other drivers.  Why do we have to be a rebel against
 all civil manner? :)

That's not true for everything and for ASoC I'd tend to assume that the
user knows what they're doing and has a good reason for it; it's
certainly something that can be helpful in development.


signature.asc
Description: Digital signature


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Lars-Peter Clausen

On 01/14/2015 01:02 PM, Mark Brown wrote:
[...]

I don't think that we need to prevent module unload when a stream is active.
  From a framework point of view is not different from hot-unplug. I don't
see a reason why we'd jump through hoops to actively forbid removing the
module once it works just fine.



Well, the module unload means a more drastic cleanup.  Even if you
unbind, the code and data are still there while module unload may
clean them up all.



Above all, disallowing the module unload while using is the common
behavior of any other drivers.  Why do we have to be a rebel against
all civil manner? :)


That's not true for everything and for ASoC I'd tend to assume that the
user knows what they're doing and has a good reason for it; it's
certainly something that can be helpful in development.



My personal opinion on this is that disallowing module removal while a 
driver registered by the module when is in use, while there is no technical 
reason to do so, is a anti-feature. Whether in ALSA or elsewhere.


But looking at the source it seems that this is a core feature of ALSA and 
at least for the card module itself it will do the ref-counting when a 
stream is started/stopped. And we even support setting the owner of a card 
in ASoC. It's just that pretty much no ASoC card driver bothers to set the 
owner field in the snd_soc_card struct. So this particular problem can be 
fixed by updating the imx-wm8962 driver to set the owner field.


- Lars

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-14 Thread Takashi Iwai
At Wed, 14 Jan 2015 12:02:28 +,
Mark Brown wrote:
 
 On Wed, Jan 14, 2015 at 11:50:48AM +0100, Takashi Iwai wrote:
  Lars-Peter Clausen wrote:
 
OK, so it's not about active stream.  From the reporter's description,
I supposed that the module gets unloaded while playing a stream, which
shouldn't be allowed.
 
   Well one of the ways to trigger this is to remove the module while the 
   stream is active. But it is not exclusively a problem module unload 
   problem. 
   E.g. the same happens if you hot-unplug the ASoC card.
 
  Yes, unbinding can trigger a similar problem, ends up the same bad
  code path.
 
 Right, which was my point - we need to be able to cope with the driver
 being removed while in use, unbind is just one path where this could
 happen and the issue isn't specific to unbind.
 
   I don't think that we need to prevent module unload when a stream is 
   active. 
From a framework point of view is not different from hot-unplug. I don't 
   see a reason why we'd jump through hoops to actively forbid removing the 
   module once it works just fine.
 
  Well, the module unload means a more drastic cleanup.  Even if you
  unbind, the code and data are still there while module unload may
  clean them up all.
 
  Above all, disallowing the module unload while using is the common
  behavior of any other drivers.  Why do we have to be a rebel against
  all civil manner? :)
 
 That's not true for everything

Hmm, which driver does behave so intentionally?  I'm interested in the
supposed reason behind it.

 and for ASoC I'd tend to assume that the
 user knows what they're doing and has a good reason for it; it's
 certainly something that can be helpful in development.

The module unload is never considered to be equivalent with hot
unplug  It's more than that.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-13 Thread Takashi Iwai
At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:
> 
> On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:
> > Wang, Jiada (ESD) wrote:
> 
> > > I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, 
> > > wm8962 codec and SSI CPU DAI,
> 
> > > I got Kernel crash when unloading audio drivers (playback stream is 
> > > active)
> > > modprobe -r snd_soc_imx_wm8962
> > > modprobe -r snd_soc_fsl_ssi
> > > modprobe -r snd_soc_wm8962
> 
> > The root problem is that you can unload the module while playing.
> > The corresponding module refcounts should have been increased during
> > used.
> 
> > Do we miss [try_]module_get() somewhere in ASoC?
> 
> That doesn't help, users can still forcibly unbind the driver at runtime
> without loading the module - and there's always the potential for
> actually hotpluggable hardware.  The teardown paths should be able to
> cope somewhat gracefully.

The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-13 Thread Mark Brown
On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:
> Wang, Jiada (ESD) wrote:

> > I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, 
> > wm8962 codec and SSI CPU DAI,

> > I got Kernel crash when unloading audio drivers (playback stream is active)
> > modprobe -r snd_soc_imx_wm8962
> > modprobe -r snd_soc_fsl_ssi
> > modprobe -r snd_soc_wm8962

> The root problem is that you can unload the module while playing.
> The corresponding module refcounts should have been increased during
> used.

> Do we miss [try_]module_get() somewhere in ASoC?

That doesn't help, users can still forcibly unbind the driver at runtime
without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


signature.asc
Description: Digital signature


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-13 Thread Takashi Iwai
At Fri, 9 Jan 2015 11:39:32 +,
Wang, Jiada (ESD) wrote:
> 
> Hi Community
> 
> I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, wm8962 
> codec and SSI CPU DAI,
> 
> I got Kernel crash when unloading audio drivers (playback stream is active)
> modprobe -r snd_soc_imx_wm8962
> modprobe -r snd_soc_fsl_ssi
> modprobe -r snd_soc_wm8962
[snip]

The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.

Do we miss [try_]module_get() somewhere in ASoC?


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-13 Thread Takashi Iwai
At Fri, 9 Jan 2015 11:39:32 +,
Wang, Jiada (ESD) wrote:
 
 Hi Community
 
 I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, wm8962 
 codec and SSI CPU DAI,
 
 I got Kernel crash when unloading audio drivers (playback stream is active)
 modprobe -r snd_soc_imx_wm8962
 modprobe -r snd_soc_fsl_ssi
 modprobe -r snd_soc_wm8962
[snip]

The root problem is that you can unload the module while playing.
The corresponding module refcounts should have been increased during
used.

Do we miss [try_]module_get() somewhere in ASoC?


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-13 Thread Takashi Iwai
At Tue, 13 Jan 2015 21:54:12 +,
Mark Brown wrote:
 
 On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:
  Wang, Jiada (ESD) wrote:
 
   I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, 
   wm8962 codec and SSI CPU DAI,
 
   I got Kernel crash when unloading audio drivers (playback stream is 
   active)
   modprobe -r snd_soc_imx_wm8962
   modprobe -r snd_soc_fsl_ssi
   modprobe -r snd_soc_wm8962
 
  The root problem is that you can unload the module while playing.
  The corresponding module refcounts should have been increased during
  used.
 
  Do we miss [try_]module_get() somewhere in ASoC?
 
 That doesn't help, users can still forcibly unbind the driver at runtime
 without loading the module - and there's always the potential for
 actually hotpluggable hardware.  The teardown paths should be able to
 cope somewhat gracefully.

The module refcount has to be handled while being used for stopping
module unload.  That's irrelevant from the dynamic unbinding support
itself.  Of course, the module refcount doesn't save the world, but
it's the right fix for this particular scenario.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] unload Audio drivers while playback stream is active case kernel crash

2015-01-13 Thread Mark Brown
On Tue, Jan 13, 2015 at 06:24:44PM +0100, Takashi Iwai wrote:
 Wang, Jiada (ESD) wrote:

  I am using i.MX6Q sabreSD board, which have imx_wm892 machine driver, 
  wm8962 codec and SSI CPU DAI,

  I got Kernel crash when unloading audio drivers (playback stream is active)
  modprobe -r snd_soc_imx_wm8962
  modprobe -r snd_soc_fsl_ssi
  modprobe -r snd_soc_wm8962

 The root problem is that you can unload the module while playing.
 The corresponding module refcounts should have been increased during
 used.

 Do we miss [try_]module_get() somewhere in ASoC?

That doesn't help, users can still forcibly unbind the driver at runtime
without loading the module - and there's always the potential for
actually hotpluggable hardware.  The teardown paths should be able to
cope somewhat gracefully.


signature.asc
Description: Digital signature