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

From: Takashi Iwai
Date: Wed Feb 17 2010 - 09:29:45 EST


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 ;)

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/