sound driver (opti) spinlock bug, 2.6.7-rc3

sound driver (opti) spinlock bug, 2.6.7-rc3

[David Ford]

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

pnp: the driver 'opti9xx' has been registered
pnp: match found with the PnP device '01:01.01' and the driver 'opti9xx'
pnp: match found with the PnP device '01:01.03' and the driver 'opti9xx'
sound/isa/opti9xx/opti92x-ad1848.c:428: 
spin_lock(sound/isa/opti9xx/opti92x-ad1848.c:cf40eea0) already locked by 
sound/isa/opti9xx/opti92x-ad1848.c/604
sound/isa/opti9xx/opti92x-ad1848.c:610: 
spin_unlock(sound/isa/opti9xx/opti92x-ad1848.c:cf40eea0) not locked
ALSA sound/isa/opti9xx/opti92x-ad1848.c:2174: no OPL device at 0x380-0x383
ALSA device list:
  #0: OPTi 82C931, 82C931 at 0x608, irq 10, dma 0&1
sound/isa/opti9xx/opti92x-ad1848.c:879: 
spin_lock(sound/isa/opti9xx/opti92x-ad1848.c:cf407fbc) already locked by 
sound/isa/opti9xx/opti92x-ad1848.c/1035
sound/isa/opti9xx/opti92x-ad1848.c:772: 
spin_lock(sound/isa/opti9xx/opti92x-ad1848.c:cf407fbc) already locked by 
sound/isa/opti9xx/opti92x-ad1848.c/879
sound/isa/opti9xx/opti92x-ad1848.c:888: 
spin_unlock(sound/isa/opti9xx/opti92x-ad1848.c:cf407fbc) not locked
sound/isa/opti9xx/opti92x-ad1848.c:1055: 
spin_unlock(sound/isa/opti9xx/opti92x-ad1848.c:cf407fbc) not locked
sound/isa/opti9xx/opti92x-ad1848.c:772: 
spin_lock(sound/isa/opti9xx/opti92x-ad1848.c:cf407fbc) already locked by 
sound/isa/opti9xx/opti92x-ad1848.c/939


[-- Attachment #2: david+challenge-response.vcf --]
[-- Type: text/x-vcard, Size: 183 bytes --]

begin:vcard
fn:David Ford
n:Ford;David
email;internet:david@blue-labs.org
title:Industrial Geek
tel;home:Ask please
tel;cell:(203) 650-3611
x-mozilla-html:TRUE
version:2.1
end:vcard

Re: sound driver (opti) spinlock bug, 2.6.7-rc3

[Andrew Morton]

David Ford <david+challenge-response@blue-labs.org> wrote:
>
> sound/isa/opti9xx/opti92x-ad1848.c:428: 
>  spin_lock(sound/isa/opti9xx/opti92x-ad1848.c:cf40eea0) already locked by 
>  sound/isa/opti9xx/opti92x-ad1848.c/604

Like this?

The locking in there seems fairly optimistic - looks like someone did a
cli() translation on it.

--- 25/sound/isa/opti9xx/opti92x-ad1848.c~opti92x-ad1848-locking-fix	2004-06-12 17:09:38.699325296 -0700
+++ 25-akpm/sound/isa/opti9xx/opti92x-ad1848.c	2004-06-12 17:16:38.029577408 -0700
@@ -601,13 +601,11 @@ __skip_base:
 	dma_bits |= 0x04;
 #endif	/* CS4231 || OPTi93X */
 
-	spin_lock_irqsave(&chip->lock, flags);
 #ifndef OPTi93X
-	 outb(irq_bits << 3 | dma_bits, chip->wss_base);
+	outb(irq_bits << 3 | dma_bits, chip->wss_base);
 #else /* OPTi93X */
 	snd_opti9xx_write(chip, OPTi9XX_MC_REG(3), (irq_bits << 3 | dma_bits));
 #endif /* OPTi93X */
-	spin_unlock_irqrestore(&chip->lock, flags);
 
 __skip_resources:
 	if (chip->hardware > OPTi9XX_HW_82C928) {
_

Re: sound driver (opti) spinlock bug, 2.6.7-rc3

[Takashi Iwai]

At Sat, 12 Jun 2004 17:19:47 -0700,
Andrew Morton wrote:
> 
> David Ford <david+challenge-response@blue-labs.org> wrote:
> >
> > sound/isa/opti9xx/opti92x-ad1848.c:428: 
> >  spin_lock(sound/isa/opti9xx/opti92x-ad1848.c:cf40eea0) already locked by 
> >  sound/isa/opti9xx/opti92x-ad1848.c/604
> 
> Like this?
> 
> The locking in there seems fairly optimistic - looks like someone did a
> cli() translation on it.

Oh my.  Apparently this driver has been never used/tested on
SMP/preempt practically since long ago.

I checked the code quickly now, and it Looks like it has other
deadlocks.  Could you try the patch below?


Takashi

--- linux/sound/isa/opti9xx/opti92x-ad1848.c	23 Apr 2004 15:01:17 -0000	1.40
+++ linux/sound/isa/opti9xx/opti92x-ad1848.c	14 Jun 2004 17:18:09 -0000
@@ -474,7 +474,6 @@ static int __devinit snd_opti9xx_configu
 	unsigned char dma_bits;
 	unsigned char mpu_port_bits = 0;
 	unsigned char mpu_irq_bits;
-	unsigned long flags;
 
 	switch (chip->hardware) {
 #ifndef OPTi93X
@@ -601,13 +600,11 @@ __skip_base:
 	dma_bits |= 0x04;
 #endif	/* CS4231 || OPTi93X */
 
-	spin_lock_irqsave(&chip->lock, flags);
 #ifndef OPTi93X
 	 outb(irq_bits << 3 | dma_bits, chip->wss_base);
 #else /* OPTi93X */
 	snd_opti9xx_write(chip, OPTi9XX_MC_REG(3), (irq_bits << 3 | dma_bits));
 #endif /* OPTi93X */
-	spin_unlock_irqrestore(&chip->lock, flags);
 
 __skip_resources:
 	if (chip->hardware > OPTi9XX_HW_82C928) {
@@ -767,15 +764,10 @@ static void snd_opti93x_mce_down(opti93x
 
 static void snd_opti93x_mute(opti93x_t *chip, int mute)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&chip->lock, flags);
-
 	mute = mute ? 1 : 0;
-	if (chip->mute == mute) {
-		spin_unlock_irqrestore(&chip->lock, flags);
+	if (chip->mute == mute)
 		return;
-	}
+
 	chip->mute = mute;
 
 	snd_opti93x_mute_reg(chip, OPTi93X_CD_LEFT_INPUT, mute);
@@ -800,8 +792,6 @@ static void snd_opti93x_mute(opti93x_t *
 	snd_opti93x_mute_reg(chip, OPTi93X_MIC_RIGHT_INPUT, mute);
 	snd_opti93x_mute_reg(chip, OPTi93X_OUT_LEFT, mute);
 	snd_opti93x_mute_reg(chip, OPTi93X_OUT_RIGHT, mute);
-
-	spin_unlock_irqrestore(&chip->lock, flags);
 }
 
 
@@ -873,10 +863,8 @@ static unsigned char snd_opti93x_get_for
 
 static void snd_opti93x_playback_format(opti93x_t *chip, unsigned char fmt)
 {
-	unsigned long flags;
 	unsigned char mask;
 
-	spin_lock_irqsave(&chip->lock, flags);
 	snd_opti93x_mute(chip, 1);
 
 	snd_opti93x_mce_up(chip);
@@ -885,14 +873,10 @@ static void snd_opti93x_playback_format(
 	snd_opti93x_mce_down(chip);
 
 	snd_opti93x_mute(chip, 0);
-	spin_unlock_irqrestore(&chip->lock, flags);
 }
 
 static void snd_opti93x_capture_format(opti93x_t *chip, unsigned char fmt)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&chip->lock, flags);
 	snd_opti93x_mute(chip, 1);
 
 	snd_opti93x_mce_up(chip);
@@ -904,7 +888,6 @@ static void snd_opti93x_capture_format(o
 	snd_opti93x_mce_down(chip);
 
 	snd_opti93x_mute(chip, 0);
-	spin_unlock_irqrestore(&chip->lock, flags);
 }