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

[Ed Tomlinson]

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;
>  }
>  
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/