Re: [LOCKDEP] 33-rc8 Running aplay with pulse as the default

Ed Tomlinson
2010-02-18T12:25:47Z

On Wednesday 17 February 2010 09:29:37 Takashi Iwai wrote:
> At Wed, 17 Feb 2010 08:16:32 -0500,
> Ed Tomlinson wrote:
> >
> > On Tuesday 16 February 2010 08:37:46 Takashi Iwai wrote:
> > > > Thanks for the patch. It helps in that it eliminates the opps but lockdep still triggers and aplay still fails.
> > > > Here is the new traceback.
> > >
> > > Hmm, fixing this isn't so trivial. The same problem occurs on other
> > > subsystems like NFS over years. And it's still there, AFAIK.
> > > The mmap mutex appears suddenly in the strange code path at close.
> > >
> > > The patch below might fix, but I'm not 100% sure whether this has no
> > > side effect.
> > >
> > > Anyway, I doubt very much it being a regression. There is no change
> > > in ALSA core side, and also in V4L em28xx code. Maybe the lockdep
> > > wasn't triggered by some reason. And, this lockdep warning is almost
> > > harmless...
> >
> > Takashi,
> >
> > The second patch eliminating the lock causes oppes every time (one follows just in case
> > its helpful).
>
> Are you sure? The patch should causes a compile error, so you must have
> patched manually in a wrong place ;)

Yes I am sure. I fixed the compile error the same way it is fixed below.

Ed

> Try the patch below (again untested, though).
>
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c
> index bd78338..c51e623 100644
> --- a/drivers/media/video/em28xx/em28xx-audio.c
> +++ b/drivers/media/video/em28xx/em28xx-audio.c
> @@ -301,23 +301,12 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream)
> /* Sets volume, mute, etc */
>
> dev->mute = 0;
> - mutex_lock(&dev->lock);
> ret = em28xx_audio_analog_set(dev);
> - mutex_unlock(&dev->lock);
> if (ret < 0)
> goto err;
>
> runtime->hw = snd_em28xx_hw_capture;
> - if (dev->alt == 0 && dev->adev.users == 0) {
> - int errCode;
> - dev->alt = 7;
> - errCode = usb_set_interface(dev->udev, 0, 7);
> - dprintk("changing alternate number to 7\n");
> - }
> -
> - mutex_lock(&dev->lock);
> dev->adev.users++;
> - mutex_unlock(&dev->lock);
>
> snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
> dev->adev.capture_pcm_substream = substream;
> @@ -336,7 +325,6 @@ static int snd_em28xx_pcm_close(struct snd_pcm_substream *substream)
> dprintk("closing device\n");
>
> dev->mute = 1;
> - mutex_lock(&dev->lock);
> dev->adev.users--;
> em28xx_audio_analog_set(dev);
> if (substream->runtime->dma_area) {
> @@ -344,7 +332,6 @@ static int snd_em28xx_pcm_close(struct snd_pcm_substream *substream)
> vfree(substream->runtime->dma_area);
> substream->runtime->dma_area = NULL;
> }
> - mutex_unlock(&dev->lock);
>
> return 0;
> }
> @@ -352,6 +339,7 @@ static int snd_em28xx_pcm_close(struct snd_pcm_substream *substream)
> static int snd_em28xx_hw_capture_params(struct snd_pcm_substream *substream,
> struct snd_pcm_hw_params *hw_params)
> {
> + struct em28xx *dev = snd_pcm_substream_chip(substream);
> unsigned int channels, rate, format;
> int ret;
>
> @@ -366,6 +354,14 @@ static int snd_em28xx_hw_capture_params(struct snd_pcm_substream *substream,
> /* TODO: set up em28xx audio chip to deliver the correct audio format,
> current default is 48000hz multiplexed => 96000hz mono
> which shouldn't matter since analogue TV only supports mono */
> +
> + if (dev->alt == 0 && dev->adev.users == 1) {
> + int errCode;
> + dev->alt = 7;
> + errCode = usb_set_interface(dev->udev, 0, 7);
> + dprintk("changing alternate number to 7\n");
> + }
> +
> return 0;
> }
>
>
>